GNU bug report logs - #75981
[PATCH (WIP) v1 0/4] Add 'guix fork'.

Previous Next

Full log

View this message in rfc822 format

From: 45mg <45mg.writes <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 45mg <45mg.writes <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Nicolas Graves <ngraves <at> ngraves.fr>, Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>, Tomas Volf <~@wolfsden.cz>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 75981 <at> debbugs.gnu.org, Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>, Attila Lendvai <attila <at> lendvai.name>, Ludovic Courtès <ludo <at> gnu.org>
Subject: [bug#75981] [PATCH (WIP) v1.5 1/4] Add 'guix fork create'.
Date: Sun, 23 Feb 2025 13:27:58 +0000

Hi Maxim,

I've implemented most of your feedback in the v2 - at least, the
feedback that still applies, since v2 is a pretty big refactor. I'm
replying here to the things I /didn't/ address, to explain why.

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

>> +
>> +;;;
>> +;;; Helper prodecures.
>> +;;;
>> +
>> +(define (fingerprint->key-file-name fingerprint)
>> +  (let* ((listing (invoke/stdout "gpg" "--list-key" "--with-colons" fingerprint))
>> +         (uid (chain-cut listing
>> +                           (string-split <> #\newline)
>> +                           (filter (cut string-prefix? "uid:" <>) <>)
>> +                           first
>
> If there are no key for FINGERPRINT, `first' will fail with a cryptic
> error here.  It should ideally throw a useful exception.

I tested it, and it doesn't, because 'invoke/stdout' has error handling:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (fingerprint->key-file-name "3CE464558A84FDC69DB40CFB090B11993D9AEBB5")  ;Ludo's key, which I have since I needed it to verify the Guix installer .iso
$1 = "ludo-3D9AEBB5.key"
scheme@(guile-user)> (fingerprint->key-file-name "abcdef")
gpg: error reading key: No public key
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Wrong type (expecting exact integer): #<&invoke-error program: "gpg" arguments: ("--list-key" "--with-colons" "abcdef") exit-status: 2 term-signal: #f stop-signal: #f>
--8<---------------cut here---------------end--------------->8---

I think this sufficiently indicates what the error is.

> We prefer to use guile-git throughout Guix, as it has a proper Scheme
> interface.  Have you tried using it instead of shelling out to git?
> Perhaps it was missing some features you needed?

Yes, IIRC renaming remotes wasn't supported. But the main reason was
that I wanted to implement a --dry-run option that would print the
commands to be executed, which would help clarify what the script is
doing. I've done that in v2.

>> +      (unless use-existing?
>> +        (info (G_ "Cloning from upstream ~a...~%") upstream)
>> +        (invoke "git" "clone" upstream directory))
>
> Why not using the above defined invoke-git here?

'invoke-git' acts on the git repository in 'directory'. Here, we are
creating that repository.

>>  
>> @@ -1193,6 +1200,60 @@ (define-syntax current-source-directory
>>            ;; raising an error would upset Geiser users
>>            #f))))))
>>  
>> +
>> +;;;
>> +;;; Higher-order functions.
>> +;;;
>> +
>> +(define-syntax chain-cut
>> +  (lambda (x)
>> +    "Apply each successive form to the result of evaluating the previous one.
>> +Before applying, expand each form (op ...) to (cut op ...).
>> +
>> +Examples:
>> +
>> +    (chain-cut '(1 2 3) cdr car)
>> + => (car (cdr '(1 2 3)))
>> +
>> +    (chain-cut 2 (- 3 <>) 1+)
>> + => (1+ ((cut - 3 <>) 2))
>> + => (1+ (- 3 2))
>> +"
>> +    (syntax-case x ()
>> +      ((chain-cut init op) (identifier? #'op)
>> +       #'(op init))
>> +      ((chain-cut init (op ...))
>> +       #'((cut op ...) init))
>> +      ((chain-cut init op op* ...) (identifier? #'op)
>> +       #'(chain-cut (op init) op* ...))
>> +      ((chain-cut init (op ...) op* ...)
>> +       #'(chain-cut ((cut op ...) init) op* ...)))))
>
> I'm not 100% convince on the above, as it seems it leads to bunching a
> whole lot of procedures together and not paying attention to potential
> exceptions/errors returned.  But maybe that's OK if the whole form is
> wrapped in an error handler.

That's a fair point, but I think we're always going to have this
tradeoff when we do a lot of function composition. For example:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ((compose car car cdr) '(0 (1)))
$1 = 1
scheme@(guile-user)> ((compose car car cdr) '(0 1))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure car: Wrong type (expecting pair): 1
--8<---------------cut here---------------end--------------->8---
Here it's not immediately clear which 'car' failed.

Ideally, we'd be able to make Guile give us a proper backtrace showing
that the error originates in 'chain-cut'... but given my experience with
Guile backtraces I doubt that's even possible :_)

> That's it!  This adding a whole new command line, and to get everyone
> aware, I think going through the new GCD (Guix Common Document/RFC)
> process is warranted before it is to be accepted/included in
> Guix.
>
> -- 
> Thanks,
> Maxim

Previous Next

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