Hi! zimoun <zimon.touto...@gmail.com> skribis:
> 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. ‘git-bare’ only makes sense for a revision, not a directory, but I wonder if ‘flat’ can be used for a revision (in which case it’d be equivalent to getting the corresponding directory)? I agree there’s some redundancy between directory/revision and flat/git-bare, but it’s the SWH API that looks like this, so I’d be tempted to just keep it as is. Maybe we could ask for guidance on #swh-devel. Thanks! Ludo’.