| Message ID | 20200624125749.10908-1-ludo@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#42030] channels: Error out when the 'guix' channel lacks an introduction. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
Ludovic Courtès <ludo@gnu.org> writes: > * guix/channels.scm (latest-channel-instance): Raise an error instead of > warning when 'guix is unauthenticated. > * tests/channels.scm ("latest-channel-instances, missing introduction for 'guix'"): > New test. The hunk in tests/channels.scm does not apply. :-) LGTM, anyway. I think we should export %guix-channel-introduction, or alternatively add a %default-guix-channel that can be inherited, for easy access in custom channels.scm files. WDYT?
Hi, Marius Bakke <marius@gnu.org> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >> * guix/channels.scm (latest-channel-instance): Raise an error instead of >> warning when 'guix is unauthenticated. >> * tests/channels.scm ("latest-channel-instances, missing introduction for 'guix'"): >> New test. > > The hunk in tests/channels.scm does not apply. :-) LGTM, anyway. Ah, maybe because you were too fast: it depends on a18d02def9862dfb9b7a5e3d5aa3b541b066d198, which I pushed yesterday. > I think we should export %guix-channel-introduction, or alternatively > add a %default-guix-channel that can be inherited, for easy access in > custom channels.scm files. WDYT? The idea was to expose only ‘%default-channels’, which leaves room for the possibility of having multiple channels by default. I’d rather avoid exposing the individual bits as that’s a bit of an additional committment. But note that the introduction of the “official” guix channel is automatically added anyway (commit c3f6f564e909ebefe752d24b325871a4e3a02d40). Thanks for your feedback! Ludo’.
Ludovic Courtès <ludo@gnu.org> skribis: > * guix/channels.scm (latest-channel-instance): Raise an error instead of > warning when 'guix is unauthenticated. > * tests/channels.scm ("latest-channel-instances, missing introduction for 'guix'"): > New test. > --- > guix/channels.scm | 13 ++++++++++--- > tests/channels.scm | 21 +++++++++++++++++++++ > 2 files changed, 31 insertions(+), 3 deletions(-) Pushed as ead5c46147ebf352ad4804d52a766dcf105eda4f. Ludo’.
Hello! Sorry for the late reply. Ludovic Courtès <ludo@gnu.org> writes: >> I think we should export %guix-channel-introduction, or alternatively >> add a %default-guix-channel that can be inherited, for easy access in >> custom channels.scm files. WDYT? > > The idea was to expose only ‘%default-channels’, which leaves room for > the possibility of having multiple channels by default. I’d rather > avoid exposing the individual bits as that’s a bit of an additional > committment. > > But note that the introduction of the “official” guix channel is > automatically added anyway (commit > c3f6f564e909ebefe752d24b325871a4e3a02d40). It is only added for people using the default channel URL, and not for local mirrors that may well be authenticated. E.g. (url "/some/dir") or (url "https://github.com/guix-mirror/guix"). Would it make sense to remove the %default-channel-url check from ensure-default-introductions? I think we can safely assume that a channel named 'guix is _the_ Guix channel regardless of where it came from, and if users need to override the introduction they can just do that. Currently I do this to access the channel introduction, which seems needlessly complicated: --8<---------------cut here---------------start------------->8--- (use-modules (srfi srfi-1)) (let ((guix-channel (find (lambda (channel) (eq? 'guix (channel-name channel))) %default-channels))) (list [other channels omitted] (channel (inherit guix-channel) (url "/home/marius/src/guix")))) --8<---------------cut here---------------end--------------->8---
Hi! Marius Bakke <marius@gnu.org> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >>> I think we should export %guix-channel-introduction, or alternatively >>> add a %default-guix-channel that can be inherited, for easy access in >>> custom channels.scm files. WDYT? >> >> The idea was to expose only ‘%default-channels’, which leaves room for >> the possibility of having multiple channels by default. I’d rather >> avoid exposing the individual bits as that’s a bit of an additional >> committment. >> >> But note that the introduction of the “official” guix channel is >> automatically added anyway (commit >> c3f6f564e909ebefe752d24b325871a4e3a02d40). > > It is only added for people using the default channel URL, and not for > local mirrors that may well be authenticated. E.g. (url "/some/dir") or > (url "https://github.com/guix-mirror/guix"). > > Would it make sense to remove the %default-channel-url check from > ensure-default-introductions? I think we can safely assume that a > channel named 'guix is _the_ Guix channel regardless of where it came > from, and if users need to override the introduction they can just do > that. I don’t think we can do that: if someone who’s not currently a committer wants to publish a fork, they’ll also publish a different introduction, pointing to the first commit where they’re in ‘.guix-authorizations’. > Currently I do this to access the channel introduction, which seems > needlessly complicated: > > (use-modules (srfi srfi-1)) > (let ((guix-channel (find (lambda (channel) > (eq? 'guix (channel-name channel))) > %default-channels))) You can write: (find guix-channel? %default-channels). > (list [other channels omitted] > (channel > (inherit guix-channel) > (url "/home/marius/src/guix")))) I’m hesitant. We can publish ‘%guix-channel-introduction’ if it helps, but it’s a slight maintenance constraint for a slight improvement. :-) WDYT? Ludo’.
Ludovic Courtès <ludo@gnu.org> writes: > Hi! > > Marius Bakke <marius@gnu.org> skribis: > >> Ludovic Courtès <ludo@gnu.org> writes: >> >>>> I think we should export %guix-channel-introduction, or alternatively >>>> add a %default-guix-channel that can be inherited, for easy access in >>>> custom channels.scm files. WDYT? >>> >>> The idea was to expose only ‘%default-channels’, which leaves room for >>> the possibility of having multiple channels by default. I’d rather >>> avoid exposing the individual bits as that’s a bit of an additional >>> committment. >>> >>> But note that the introduction of the “official” guix channel is >>> automatically added anyway (commit >>> c3f6f564e909ebefe752d24b325871a4e3a02d40). >> >> It is only added for people using the default channel URL, and not for >> local mirrors that may well be authenticated. E.g. (url "/some/dir") or >> (url "https://github.com/guix-mirror/guix"). >> >> Would it make sense to remove the %default-channel-url check from >> ensure-default-introductions? I think we can safely assume that a >> channel named 'guix is _the_ Guix channel regardless of where it came >> from, and if users need to override the introduction they can just do >> that. > > I don’t think we can do that: if someone who’s not currently a committer > wants to publish a fork, they’ll also publish a different introduction, > pointing to the first commit where they’re in ‘.guix-authorizations’. Right. >> Currently I do this to access the channel introduction, which seems >> needlessly complicated: >> >> (use-modules (srfi srfi-1)) >> (let ((guix-channel (find (lambda (channel) >> (eq? 'guix (channel-name channel))) >> %default-channels))) > > You can write: (find guix-channel? %default-channels). Ah, much better. >> (list [other channels omitted] >> (channel >> (inherit guix-channel) >> (url "/home/marius/src/guix")))) > > I’m hesitant. We can publish ‘%guix-channel-introduction’ if it helps, > but it’s a slight maintenance constraint for a slight improvement. :-) > WDYT? I'm happy with the 'find guix-channel?' trick for now. Let's see if other users report workflow issues before changing anything. :-)
diff --git a/guix/channels.scm b/guix/channels.scm index 3eec5df883..1016b95045 100644 --- a/guix/channels.scm +++ b/guix/channels.scm @@ -406,9 +406,16 @@ their relation. When AUTHENTICATE? is false, CHANNEL is not authenticated." ;; TODO: Warn for all the channels once the authentication interface ;; is public. (when (guix-channel? channel) - (warning (G_ "channel '~a' lacks an introduction and \ -cannot be authenticated~%") - (channel-name channel)))) + (raise (condition + (&message + (message (format #f (G_ "channel '~a' lacks an \ +introduction and cannot be authenticated~%") + (channel-name channel)))) + (&fix-hint + (hint (G_ "Add the missing introduction to your +channels file to address the issue. Alternatively, you can pass +@option{--disable-authentication}, at the risk of running unauthenticated and +thus potentially malicious code."))))))) (warning (G_ "channel authentication disabled~%"))) (when (guix-channel? channel) diff --git a/tests/channels.scm b/tests/channels.scm index 3a2c1d429b..d7202f8cbf 100644 --- a/tests/channels.scm +++ b/tests/channels.scm @@ -402,6 +402,27 @@ (channel-news-for-commit channel commit5 commit1)) '(#f "tag-for-first-news-entry"))))))) +(unless (which (git-command)) (test-skip 1)) +(test-assert "latest-channel-instances, missing introduction for 'guix'" + (with-temporary-git-repository directory + '((add "a.txt" "A") + (commit "first commit") + (add "b.scm" "#t") + (commit "second commit")) + (with-repository directory repository + (let* ((commit1 (find-commit repository "first")) + (commit2 (find-commit repository "second")) + (channel (channel (url (string-append "file://" directory)) + (name 'guix)))) + + (guard (c ((message-condition? c) + (->bool (string-contains (condition-message c) + "introduction")))) + (with-store store + ;; Attempt a downgrade from NEW to OLD. + (latest-channel-instances store (list channel)) + #f)))))) + (unless (gpg+git-available?) (test-skip 1)) (test-equal "authenticate-channel, wrong first commit signer" #t
* guix/channels.scm (latest-channel-instance): Raise an error instead of warning when 'guix is unauthenticated. * tests/channels.scm ("latest-channel-instances, missing introduction for 'guix'"): New test. --- guix/channels.scm | 13 ++++++++++--- tests/channels.scm | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) Hi! This patch makes it an error to have a 'guix' channel without an introduction. Before that, it was just a warning, which is easily overlooked. (Similarly, wget or your browser stop if they cannot authenticate the host you're connecting to over HTTPS.) Note that when using the "official" 'guix' channel, (guix channels) automatically adds the introduction (see commit c3f6f564e909ebefe752d24b325871a4e3a02d40). It will work similarly for people who maintain forks. Thanks, Ludo'.