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

Previous Next

Package: guix-patches;

Reported by: Sarah Morgensen <iskarian <at> mgsn.dev>

Date: Sun, 15 Aug 2021 23:17:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 50072 <at> debbugs.gnu.org
Subject: [bug#50072] [PATCH WIP 0/4] Add upstream updater for git-fetch origins.
Date: Mon, 06 Sep 2021 18:59:59 -0700
Hi Ludo,

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

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

Thanks for taking a look!

>
> Sarah Morgensen <iskarian <at> 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




This bug report was last modified 3 years and 132 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.