Subject: Re: bug#44187: Channel clones lack SWH fallback

From: Ludovic Courtès <[email-masked]>
Date: Sat, 18 Sep 2021 12:05:56 +0200

Hi!

zimoun <[email-masked]> skribis:

> The patch LGTM although there is a redundancy, from my understanding.
>
> On Fri, 10 Sep 2021 at 16:34, Ludovic Courtès <[email-masked]> 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’.