Hi Ludo,
The patch LGTM although there is a redundancy, from my understanding.
On Fri, 10 Sep 2021 at 16:34, Ludovic Courtès <l...@gnu.org> wrote:
> @@ -694,7 +714,15 @@ wait until it becomes available, which could take
> several minutes."
> (format log-port "SWH: found revision ~a with directory at '~a'~%"
> (revision-id revision)
> (swh-url (revision-directory-url revision)))
> - (swh-download-directory (revision-directory revision) output
> - #:log-port log-port))
> + (swh-download-archive (match archive-type
> + ('flat
> + (string-append
> + "swh:1:dir:" (revision-directory revision)))
> + ('git-bare
> + (string-append
> + "swh:1:rev:" (revision-id revision))))
Here the ’swid’ depends on the ’archive-type’…
> + output
> + #:archive-type archive-type
…which is also passed. Then this is propagated. For instance,
’swh-download-directory’:
> +(define* (swh-download-directory id output
> + #:key (log-port (current-error-port)))
> + "Download from Software Heritage the directory with the given ID, and
> +unpack it to OUTPUT. Return #t on success and #f on failure."
> + (swh-download-archive (string-append "swh:1:dir:" id) output
> + #:archive-type 'flat
> + #:log-port log-port))
> +
Does it make sense to pass this ’swhid’ equal to ’swh:1:rev’ with the
’flat’ archive-type? Another instance is,
> + (match (vault-fetch swhid
> + #:archive-type archive-type
> + #:log-port log-port)
and from my understanding, again ’swhid’ depends on ’archive-type’.
Therefore, it prone error. The best seems to pass ’(archive-type
. swhid)’ and pattern-match on that. Yeah, it potentially breaks the
public API… but there is no claim about stability (and I am not
convinced this (guix swh) module is used outside Guix :-)).
Cheers,
simon
