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

From: zimoun <[email-masked]>
Date: Fri, 17 Sep 2021 19:31:55 +0200

Hi Ludo,

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.  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