channels: ‘latest-channel-instances’ traverses user-provided channels first.

Previously, ‘latest-channel-instances’ would perform a depth-first
traversal of channels.  Since dependencies specified in ‘.guix-channel’
are usually less specific that those provided by the user, this would
lead to the use of instances corresponding to those less specific specs,
which in turn might declare dependencies that do not exist for the more
specific instances.

This commit changes ‘latest-channel-instances’ to perform a
breadth-first traversal, thereby giving user-supplied channels higher
precedence over dependencies found via ‘.guix-channel’.

Fixes <https://issues.guix.gnu.org/68822>.

* guix/channels.scm (latest-channel-instances)[ignore?]: Remove.
[instance-name, same-named?, more-specific?]: New procedures.
Rewrite as a breadth-first traversal using a regular loop.
* tests/channels.scm ("latest-channel-instances reads dependencies from most-specific instance"):
New test.

Change-Id: Iba518145cfd209f04293a56246dbfee3b714650b
This commit is contained in:
Ludovic Courtès 2024-01-31 08:29:30 +01:00 committed by Ludovic Courtès
parent 36d654fa54
commit 323b58ac18
No known key found for this signature in database
GPG key ID: 090B11993D9AEBB5
2 changed files with 118 additions and 63 deletions

View file

@ -34,7 +34,6 @@ (define-module (guix channels)
#:use-module (guix packages) #:use-module (guix packages)
#:use-module (guix progress) #:use-module (guix progress)
#:use-module (guix derivations) #:use-module (guix derivations)
#:use-module (guix combinators)
#:use-module (guix diagnostics) #:use-module (guix diagnostics)
#:use-module (guix sets) #:use-module (guix sets)
#:use-module (guix store) #:use-module (guix store)
@ -510,16 +509,6 @@ (define* (latest-channel-instances store channels
against the newly-fetched instances of CHANNELS, and VALIDATE-PULL is called against the newly-fetched instances of CHANNELS, and VALIDATE-PULL is called
for each channel update and can choose to emit warnings or raise an error, for each channel update and can choose to emit warnings or raise an error,
depending on the policy it implements." depending on the policy it implements."
;; Only process channels that are unique, or that are more specific than a
;; previous channel specification.
(define (ignore? channel others)
(member channel others
(lambda (a b)
(and (eq? (channel-name a) (channel-name b))
(or (channel-commit b)
(not (or (channel-commit a)
(channel-commit b))))))))
(define (current-commit name) (define (current-commit name)
;; Return the current commit for channel NAME. ;; Return the current commit for channel NAME.
(any (lambda (channel) (any (lambda (channel)
@ -527,60 +516,77 @@ (define (current-commit name)
(channel-commit channel))) (channel-commit channel)))
current-channels)) current-channels))
(define instance-name
(compose channel-name channel-instance-channel))
(define (same-named? channel)
(let ((name (channel-name channel)))
(lambda (candidate)
(eq? (channel-name candidate) name))))
(define (more-specific? a b)
;; A is more specific than B if it specifies a commit.
(and (channel-commit a)
(not (channel-commit b))))
(let loop ((channels channels) (let loop ((channels channels)
(previous-channels '())) (previous-channels '())
;; Accumulate a list of instances. A list of processed channels is also (instances '()))
;; accumulated to decide on duplicate channel specifications. (match channels
(define-values (resulting-channels instances) (()
(fold2 (lambda (channel previous-channels instances) (reverse instances))
(if (ignore? channel previous-channels) ((channel . rest)
(values previous-channels instances) (let ((previous (find (same-named? channel) previous-channels)))
(begin ;; If there's already an instance for CHANNEL, keep the most specific
(format (current-error-port) ;; one.
(G_ "Updating channel '~a' from Git repository at '~a'...~%") (if (and previous
(channel-name channel) (not (more-specific? channel previous)))
(channel-url channel)) (loop rest previous-channels instances)
(let* ((current (current-commit (channel-name channel))) (begin
(instance (format (current-error-port)
(latest-channel-instance store channel (G_ "Updating channel '~a' from Git repository at '~a'...~%")
#:authenticate? (channel-name channel)
authenticate? (channel-url channel))
#:validate-pull (let* ((current (current-commit (channel-name channel)))
validate-pull (instance
#:starting-commit (latest-channel-instance store channel
current))) #:authenticate?
(when authenticate? authenticate?
;; CHANNEL is authenticated so we can trust the #:validate-pull
;; primary URL advertised in its metadata and warn validate-pull
;; about possibly stale mirrors. #:starting-commit
(let ((primary-url (channel-instance-primary-url current)))
instance))) (when authenticate?
(unless (or (not primary-url) ;; CHANNEL is authenticated so we can trust the
(channel-commit channel) ;; primary URL advertised in its metadata and warn
(string=? primary-url (channel-url channel))) ;; about possibly stale mirrors.
(warning (G_ "pulled channel '~a' from a mirror \ (let ((primary-url (channel-instance-primary-url
instance)))
(unless (or (not primary-url)
(channel-commit channel)
(string=? primary-url (channel-url channel)))
(warning (G_ "pulled channel '~a' from a mirror \
of ~a, which might be stale~%") of ~a, which might be stale~%")
(channel-name channel) (channel-name channel)
primary-url)))) primary-url))))
(let-values (((new-instances new-channels) ;; Perform a breadth-first traversal with the idea that the
(loop (channel-instance-dependencies instance) ;; user-provided channels may be more specific than what
previous-channels))) ;; '.guix-channel' specifies, and so it is on those instances
(values (append (cons channel new-channels) ;; that 'channel-instance-dependencies' should be called.
previous-channels) (loop (append rest
(append (cons instance new-instances) (channel-instance-dependencies instance))
instances))))))) (cons channel
previous-channels (if previous
'() ;instances (delq previous previous-channels)
channels)) previous-channels))
(cons instance
(let ((instance-name (compose channel-name channel-instance-channel))) (if previous
;; Remove all earlier channel specifications if they are followed by a (remove (lambda (instance)
;; more specific one. (eq? (instance-name instance)
(values (delete-duplicates instances (channel-name channel)))
(lambda (a b) instances)
(eq? (instance-name a) (instance-name b)))) instances)))))))))))
resulting-channels))))
(define* (checkout->channel-instance checkout (define* (checkout->channel-instance checkout
#:key commit #:key commit

View file

@ -1,6 +1,6 @@
;;; GNU Guix --- Functional package management for GNU ;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
;;; Copyright © 2019, 2020, 2022 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2019-2020, 2022, 2024 Ludovic Courtès <ludo@gnu.org>
;;; ;;;
;;; This file is part of GNU Guix. ;;; This file is part of GNU Guix.
;;; ;;;
@ -196,6 +196,55 @@ (define channel-metadata-dependencies
"abc1234"))) "abc1234")))
instances))))))) instances)))))))
(test-equal "latest-channel-instances reads dependencies from most-specific instance"
'(chan1 chan2)
;; Here '.guix-channel' in DIRECTORY2 is less specific than the
;; user-provided channel spec in ONE: the latter specifies a commit. Since
;; the most specific one "wins", the bogus '.guix-channel' file added in
;; DIRECTORY1 as its second commit must not be taken into account.
;; See <https://issues.guix.gnu.org/68822>.
(with-temporary-git-repository directory1
`((add "a.scm" "(define-module (a))")
(commit "first commit")
(add ".guix-channel"
,(object->string
'(channel
(version 0)
(dependencies
;; Attempting to fetch this dependency would fail.
(channel
(name nonexistent-dependency)
(url "http://guix.example.org/does-not-exist.git"))))))
(commit "second commit"))
(with-temporary-git-repository directory2
`((add ".guix-channel"
,(object->string
`(channel (version 0)
(dependencies
(channel
(name chan1)
;; Note: no 'commit' field here.
(url ,(string-append "file://" directory1)))))))
(commit "initial commit"))
(with-repository directory1 repository
(let* ((commit (find-commit repository "first"))
(one (channel
(url (string-append "file://" directory1))
(commit (oid->string (commit-id commit))) ;<- specific
(name 'chan1)))
(two (channel
(url (string-append "file://" directory2))
(name 'chan2))))
(with-store store
(map (compose channel-name channel-instance-channel)
(delete-duplicates
(append (latest-channel-instances store (list one two))
(latest-channel-instances store (list two one)))
(lambda (instance1 instance2)
(string=? (channel-instance-commit instance1)
(channel-instance-commit instance2)))))))))))
(test-equal "latest-channel-instances #:validate-pull" (test-equal "latest-channel-instances #:validate-pull"
'descendant 'descendant