[bug#50072] [PATCH WIP 0/4] Add upstream updater for git-fetch origins.

[Sarah Morgensen]

Hi Ludo,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Sarah,
>
> I like this patch series.  :-)

Thanks for taking a look!

>
> Sarah Morgensen <iskarian@mgsn.dev> skribis:
>
>> * guix/git-download.scm (checkout-to-store): New procedure.
>> * guix/upstream.scm (guess-version-transform)
>> (package-update/git-fetch): New procedures.
>> (%method-updates): Add GIT-FETCH mapping.
>
> This LGTM.

Thanks.  WDYT about pre-emptively adding support for non-url URIs as
well?  That is,

1. change "urls" in <upstream-source> to "uri"

2. in 'git-fetch'

  a) if the upstream-source-uri is a git-reference, just use it as-is
     rather than guessing the tag

  b) if it's not, return an 'upstream-source' with a git-reference URI 

3. update 'upstream-source-compiler' to work for git-reference URIs.

If there are no objections, I think I'll make those changes and send
that as a proper patch.

>
> Nitpick:
>
>> +(define* (checkout-to-store store ref #:key (log (current-error-port)))
>> +  "Checkout REF to STORE.  Write progress reports to LOG.  RECURSIVE? has 
>> the
>> +same effect as the same-named parameter of 'latest-repository-commit'."
>> +  ;; XXX: (guix git) does not use shallow clones, so this will be slow
>> +  ;; for long-running repositories.
>> +  (match-record ref <git-reference>
>
> [...]
>
>> +  ;; Only use the first element of URLS.
>> +  (match-record source <upstream-source>
>> +    (version urls)
>
> I’d use the record acceesors in this cases rather than ‘match-record’;
> currently ‘match-record’ is not super efficient and I find it slightly
> less readable when you’re just accessing a couple of fields.

Fair.  I got a little excited to discover new syntax :)

--
Sarah