[bug#78580,v4] pull: allow filtering which channels to pull from the CLI
Commit Message
* 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
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’.
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.
@@ -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}
@@ -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