Subject: Re: [bug#78580] [PATCH v4] pull: allow filtering which channels to pull from the CLI

From: Sergio Pastor Pérez <[email-masked]>
Date: Wed, 04 Jun 2025 23:32:07 +0200

Hello Ludo, thanks for reviewing my patch!

Ludovic Courtès <[email-masked]> writes:
>> +The optional @var{channels} argument filters the list of pulled channels
>> +to those whose names match the given list.
>
> This would need to be clarified.  IIUC, what ‘guix pull A’ does is that
> it updates A but keeps the other channels unchanged, right?
>
> There’s a risk with this practice though: channels are typically tested
> against the latest version of each other.  For example, ‘guix-science’
> in continuous integration is built against the latest ‘guix’; if one
> tries to update ‘guix-science’ without updating ‘guix’, there’s a
> gradient of problems that might occur: it might work fine, or
> substitutes might be missing, or some packages from ‘guix-science’ will
> fail to build, or ‘guix pull’ will fail upfront.

Well, since channels are not updated synchronously and atomically, the
same situation can happen by a pull before channels are
synchronized. Anyways, many users maintain their own channel for testing
packages that are not yet ready to be upstreamed. In this cases, you
usually want to update your channel for quick testing, but you don't
necessarily want to pull the latest Guix, possible leaving your without
substitutes.

All in all, I think providing this interface is un upgrade without
downsides. Channels that are pinned in your channel specification will
remain pinned, so it's really one more layer of control for the
user. For example, it that allows the user to circumvent the unpleasant
update to a substituteless channel configuration.

>> +  (display (G_ "Usage: guix pull [OPTION]... [CHANNELS...]
>> +Download and deploy the latest version of Guix.
>
> “Download and deploy the latest version of Guix, possibly limited to CHANNELS.”
>
>> +If CHANNELS are specified, pull only from channels with those names (e.g.:
>> +'guix pull rde nonguix' pulls only from the 'guix', 'rde', and 'nonguix'
>> +channels as defined in your channel configuration).\n"))
>
> I’d drop this paragraph: ‘--help’ is concise by convention and further
> explanations should go to the manual.
>
>> +  (define (unpin-channels channels current-channels names)
>> +    "Unpin CHANNELS whose name symbol is present in NAMES list.
>> +If NAMES is an empty list, don't filter anything.  Warn when a name is not
>> +available in the channels list."
>
> You can turn the docstring into a comment since it’s not a top-level
> procedure.
>
>> +    (if (null? names)
>> +        channels
>> +        (let ((available-names (map channel-name
>> +                                    channels))
>> +              (selected-channels (filter (lambda (ch)
>> +                                           (member (channel-name ch)
>> +                                                   names))
>> +                                         channels)))
>
> As per our coding conventions (info "(guix) Formatting Code"),
> s/ch/channel/
> but you can shorten the other identifiers: ‘names’, ‘selected’.
>
>> +          (for-each (lambda (name)
>> +                      (unless (member name available-names)
>> +                        (warning (G_ "Channel '~a' not present in channel list~%")
>
> “channel '~a~' selected but missing from channel list~%" (messages are lower-case).
>
>> +          (map (lambda (cur-ch)
>
> s/cur-ch/channel/
>
>> +                 (let ((selected-channel (find (lambda (ch)
>> +                                                 (eq? (channel-name ch)
>> +                                                      (channel-name cur-ch)))
>> +                                               selected-channels)))
>> +                   ;; If the user selected this channel. Follow channel file
>                                                           ^
> Should be a comma.

Alright, I've already sent v5[1] of the patch with all your suggestions.

[1] https://issues.guix.gnu.org/78580#13

Best regards,
Sergio.