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

Message ID 47f6b899454a44acc2570a57b5fa6caaf8a47706.1748170502.git.sergio.pastorperez@gmail.com
State New
Headers
Series [bug#78580,v4] pull: allow filtering which channels to pull from the CLI |

Commit Message

Sergio Pastor Pérez May 25, 2025, 10:55 a.m. UTC
* guix/scripts/pull.scm (guix-pull): treat non-prefix CLI arguments as a list
of channels to pull.

Change-Id: I5d08c4b1cc84ab58a9c4e7600eb86468f92d10f0
---
 doc/guix.texi         | 10 ++++++--
 guix/scripts/pull.scm | 53 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 53 insertions(+), 10 deletions(-)


base-commit: 096dedd0bb13523002c814b001429c2f65b6f10d
  

Comments

Ludovic Courtès June 3, 2025, 8:14 a.m. UTC | #1
Hi Sergio,

Sergio Pastor Pérez <sergio.pastorperez@gmail.com> writes:

> * guix/scripts/pull.scm (guix-pull): treat non-prefix CLI arguments as a list
> of channels to pull.
>
> Change-Id: I5d08c4b1cc84ab58a9c4e7600eb86468f92d10f0

Please mention the doc/guix.texi changes.

Some comments:

> +@example
> +guix pull [@var{options}] [@var{CHANNELS}@dots{}]

“channels” (lower-case).

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

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

Thanks,
Ludo’.
  
Sergio Pastor Pérez June 4, 2025, 9:32 p.m. UTC | #2
Hello Ludo, thanks for reviewing my patch!

Ludovic Courtès <ludo@gnu.org> 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.
  

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index e4e2b853f1..26f16cfa7a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -4656,8 +4656,14 @@  Invoking guix pull
 deleting /var/guix/profiles/per-user/charlie/current-guix-1-link
 @end example
 
-The @command{guix pull} command is usually invoked with no arguments,
-but it supports the following options:
+The general syntax is:
+
+@example
+guix pull [@var{options}] [@var{CHANNELS}@dots{}]
+@end example
+
+The optional @var{channels} argument filters the list of pulled channels
+to those whose names match the given list.
 
 @table @code
 @item --url=@var{url}
diff --git a/guix/scripts/pull.scm b/guix/scripts/pull.scm
index 76aed0b5cc..fc6809d68c 100644
--- a/guix/scripts/pull.scm
+++ b/guix/scripts/pull.scm
@@ -81,8 +81,12 @@  (define %default-options
     (validate-pull . ,ensure-forward-channel-update)))
 
 (define (show-help)
-  (display (G_ "Usage: guix pull [OPTION]...
-Download and deploy the latest version of Guix.\n"))
+  (display (G_ "Usage: guix pull [OPTION]... [CHANNELS...]
+Download and deploy the latest version of Guix.
+
+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"))
   (display (G_ "
   -C, --channels=FILE    deploy the channels defined in FILE"))
   (display (G_ "
@@ -839,21 +843,52 @@  (define (validate-cache-directory-ownership)
 (define-command (guix-pull . args)
   (synopsis "pull the latest revision of Guix")
 
-  (define (no-arguments arg _)
-    (leave (G_ "~A: extraneous argument~%") arg))
+  (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."
+    (if (null? names)
+        channels
+        (let ((available-names (map channel-name
+                                    channels))
+              (selected-channels (filter (lambda (ch)
+                                           (member (channel-name ch)
+                                                   names))
+                                         channels)))
+          (for-each (lambda (name)
+                      (unless (member name available-names)
+                        (warning (G_ "Channel '~a' not present in channel list~%")
+                                 name)))
+                    names)
+
+          (map (lambda (cur-ch)
+                 (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
+                   ;; specification. Otherwise, leave the channel pinned as
+                   ;; defined by the current profile.
+                   (or selected-channel
+                       cur-ch)))
+               current-channels))))
 
   (with-error-handling
     (with-git-error-handling
      (let* ((opts         (parse-command-line args %options
-                                              (list %default-options)
-                                              #:argument-handler no-arguments))
+                                              (list %default-options)))
             (substitutes? (assoc-ref opts 'substitutes?))
             (dry-run?     (assoc-ref opts 'dry-run?))
             (profile      (or (assoc-ref opts 'profile) %current-profile))
             (current-channels (profile-channels profile))
             (validate-pull    (assoc-ref opts 'validate-pull))
             (authenticate?    (assoc-ref opts 'authenticate-channels?))
-            (verify-certificate? (assoc-ref opts 'verify-certificate?)))
+            (verify-certificate? (assoc-ref opts 'verify-certificate?))
+            (selected-channels (filter-map
+                                (match-lambda
+                                  (('argument . name) (string->symbol name))
+                                  (_ #f))
+                                opts)))
        (cond
         ((assoc-ref opts 'query)
          (process-query opts profile))
@@ -877,7 +912,9 @@  (define-command (guix-pull . args)
                  (ensure-default-profile)
                  (honor-x509-certificates store)
 
-                 (let* ((channels (channel-list opts))
+                 (let* ((channels (unpin-channels (channel-list opts)
+                                                  current-channels
+                                                  selected-channels))
                         (instances
                          (latest-channel-instances store channels
                                                    #:current-channels