[PATCH 0/2] Add channels field to guix-configuration

OpenSubmitted by Brice Waegeneire.
Details
2 participants
  • Brice Waegeneire
  • Ludovic Courtès
Owner
unassigned
Severity
normal
B
B
Brice Waegeneire wrote on 17 Jul 2021 22:58
(address . guix-patches@gnu.org)
20210717205819.380-1-brice@waegenei.re
This patchset brings the same feature as the authorized-keys but for channels;
allowing to sepcify the default channels an operating-system uses. Allowing
an operating-system declaration to be self-contained in regards to channels

Brice Waegeneire (2):
services: guix: Use "match-record" in activation.
sevices: guix: Add channels field.

doc/guix.texi | 14 +++++++++-
gnu/services/base.scm | 65 +++++++++++++++++++++++++++++++++----------
2 files changed, 64 insertions(+), 15 deletions(-)

--
2.32.0
B
B
Brice Waegeneire wrote on 17 Jul 2021 23:04
[PATCH 1/2] services: guix: Use "match-record" in activation.
(address . guix-patches@gnu.org)
20210717210424.1921-1-brice@waegenei.re
It's more explicit to specify used fields instead of depending on their
position.

* gnu/services/base.scm (guix-activation): Replace "match" with
"match-record".
---
gnu/services/base.scm | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

Toggle diff (50 lines)
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index ab3e441a7b..e206bea5f0 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -12,7 +12,7 @@
 ;;; Copyright © 2019 John Soo <jsoo1@asu.edu>
 ;;; Copyright © 2019 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian@pelzflorian.de>
-;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
 ;;; Copyright © 2021 qblade <qblade@protonmail.com>
 ;;; Copyright © 2021 Hui Lu <luhuins@163.com>
 ;;;
@@ -1700,21 +1700,21 @@ proxy of 'guix-daemon'...~%")
 
 (define (guix-activation config)
   "Return the activation gexp for CONFIG."
-  (match config
-    (($ <guix-configuration> guix build-group build-accounts authorize-key? keys)
-     ;; Assume that the store has BUILD-GROUP as its group.  We could
-     ;; otherwise call 'chown' here, but the problem is that on a COW overlayfs,
-     ;; chown leads to an entire copy of the tree, which is a bad idea.
+  (match-record config <guix-configuration>
+    (guix authorize-key? authorized-keys)
+    #~(begin
+        ;; Assume that the store has BUILD-GROUP as its group.  We could
+        ;; otherwise call 'chown' here, but the problem is that on a COW overlayfs,
+        ;; chown leads to an entire copy of the tree, which is a bad idea.
 
-     ;; Generate a key pair and optionally authorize substitute server keys.
-     #~(begin
-         (unless (file-exists? "/etc/guix/signing-key.pub")
-           (system* #$(file-append guix "/bin/guix") "archive"
-                    "--generate-key"))
+        ;; Generate a key pair and optionally authorize substitute server keys.
+        (unless (file-exists? "/etc/guix/signing-key.pub")
+          (system* #$(file-append guix "/bin/guix") "archive"
+                   "--generate-key"))
 
-         #$(if authorize-key?
-               (substitute-key-authorization keys guix)
-               #~#f)))))
+        #$(if authorize-key?
+              (substitute-key-authorization authorized-keys guix)
+              #~#f))))
 
 (define* (references-file item #:optional (name "references"))
   "Return a file that contains the list of references of ITEM."
-- 
2.32.0
B
B
Brice Waegeneire wrote on 17 Jul 2021 23:04
[PATCH 2/2] services: guix: Add channels field.
(address . guix-patches@gnu.org)
20210717210424.1921-2-brice@waegenei.re
* doc/guix.texi (Channels): Specify that '/etc/guix/channels.scm'
contains channels configuration.
(Base Services): Document 'guix-configuration-channels' field.
* gnu/services/base.scm (setup-channels): New procedure.
(guix-configuration): Add channels field.
(guix-activation): Use 'setup-channels' procedure.
---
doc/guix.texi | 14 +++++++++++++-
gnu/services/base.scm | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 51 insertions(+), 2 deletions(-)

Toggle diff (119 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index cca46218f2..c930530228 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -5001,7 +5001,7 @@ $ wget -O - \
 Guix and its package collection are updated by running @command{guix pull}
 (@pxref{Invoking guix pull}).  By default @command{guix pull} downloads and
 deploys Guix itself from the official GNU@tie{}Guix repository.  This can be
-customized by defining @dfn{channels} in the
+customized by defining @dfn{channels} in @file{/etc/guix/channels.scm} and
 @file{~/.config/guix/channels.scm} file.  A channel specifies a URL and branch
 of a Git repository to be deployed, and @command{guix pull} can be instructed
 to pull from one or more channels.  In other words, channels can be used
@@ -15549,6 +15549,18 @@ This example assumes that the file @file{./guix.example.org-key.pub}
 contains the public key that @code{guix.example.org} uses to sign
 substitutes.
 
+@item @code{channels} (default: @code{'()})
+List of system channels to use, it populates
+@file{/etc/guix/channels.scm}.
+
+@quotation Note
+When booting or reconfiguring to a system where @code{channels}
+is not null, the existing @file{/etc/guix/channels.scm} file is backed up as
+@file{/etc/guix/channels.scm.bak} if it was determined to be a manually modified
+file.  This is to facilitate migration from earlier versions, which
+allowed for in-place modifications to @file{/etc/guix/channels.scm}.
+@end quotation
+
 @item @code{max-silent-time} (default: @code{0})
 @itemx @code{timeout} (default: @code{0})
 The number of seconds of silence and the number of seconds of activity,
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index e206bea5f0..db63eb540b 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -58,6 +58,7 @@
   #:use-module (gnu packages terminals)
   #:use-module ((gnu build file-systems)
                 #:select (mount-flags->bit-mask))
+  #:use-module (guix channels)
   #:use-module (guix gexp)
   #:use-module (guix records)
   #:use-module (guix modules)
@@ -66,6 +67,7 @@
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 pretty-print)
   #:re-export (user-processes-service-type        ;backwards compatibility
                %default-substitute-urls)
   #:export (fstab-service-type
@@ -1502,6 +1504,35 @@ archive' public keys, with GUIX."
         ;; Installed the declared ACL.
         (symlink #+default-acl "/etc/guix/acl"))))
 
+;; FIXME Does this gexp should be build before boot, such as
+;; substitute-key-authorization does?
+(define (setup-channels channels)
+  "Return a gexp with code to setup CHANNELS, a list of channels"
+  (define channels-file
+    (plain-file "channels.scm"
+                (with-output-to-string
+                  (lambda _
+                    (pretty-print (map channel->code
+                                       channels))))))
+
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+
+        ;; If channels.scm already exists, move it out of the way. Create a
+        ;; backup if it's a regular file: it's likely that the user
+        ;; manually defined it.
+        (if (file-exists? "/etc/guix/channels.scm")
+            (if (and (symbolic-link? "/etc/guix/channels.scm")
+                     (store-file-name? (readlink "/etc/guix/channels.scm")))
+                (delete-file "/etc/guix/channels.scm")
+                (rename-file "/etc/guix/channels.scm"
+                             "/etc/guix/channels.scm.bak"))
+            (mkdir-p "/etc/guix"))
+
+        ;; Installed the declared channels.
+        (symlink #+channels-file "/etc/guix/channels.scm"))))
+
 (define %default-authorized-guix-keys
   ;; List of authorized substitute keys.
   (list (file-append guix "/share/guix/berlin.guix.gnu.org.pub")
@@ -1524,6 +1555,8 @@ archive' public keys, with GUIX."
                     (default #t))
   (substitute-urls  guix-configuration-substitute-urls ;list of strings
                     (default %default-substitute-urls))
+  (channels         guix-configuration-channels ;list of channels
+                    (default '()))
   (chroot-directories guix-configuration-chroot-directories ;list of file-like/strings
                       (default '()))
   (max-silent-time  guix-configuration-max-silent-time ;integer
@@ -1701,7 +1734,7 @@ proxy of 'guix-daemon'...~%")
 (define (guix-activation config)
   "Return the activation gexp for CONFIG."
   (match-record config <guix-configuration>
-    (guix authorize-key? authorized-keys)
+    (guix authorize-key? authorized-keys channels)
     #~(begin
         ;; Assume that the store has BUILD-GROUP as its group.  We could
         ;; otherwise call 'chown' here, but the problem is that on a COW overlayfs,
@@ -1714,6 +1747,10 @@ proxy of 'guix-daemon'...~%")
 
         #$(if authorize-key?
               (substitute-key-authorization authorized-keys guix)
+              #~#f)
+
+        #$(if (not (null? channels))
+              (setup-channels channels)
               #~#f))))
 
 (define* (references-file item #:optional (name "references"))
-- 
2.32.0
L
L
Ludovic Courtès wrote on 21 Jul 2021 23:47
Re: bug#49610: [PATCH 0/2] Add channels field to guix-configuration
(name . Brice Waegeneire)(address . brice@waegenei.re)(address . 49610@debbugs.gnu.org)
87wnpje3vr.fsf_-_@gnu.org
Hello,

Brice Waegeneire <brice@waegenei.re> skribis:

Toggle quote (6 lines)
> It's more explicit to specify used fields instead of depending on their
> position.
>
> * gnu/services/base.scm (guix-activation): Replace "match" with
> "match-record".

LGTM!

Ludo’.
L
L
Ludovic Courtès wrote on 21 Jul 2021 23:53
(name . Brice Waegeneire)(address . brice@waegenei.re)(address . 49610@debbugs.gnu.org)
87sg07e3kv.fsf_-_@gnu.org
Brice Waegeneire <brice@waegenei.re> skribis:

Toggle quote (7 lines)
> * doc/guix.texi (Channels): Specify that '/etc/guix/channels.scm'
> contains channels configuration.
> (Base Services): Document 'guix-configuration-channels' field.
> * gnu/services/base.scm (setup-channels): New procedure.
> (guix-configuration): Add channels field.
> (guix-activation): Use 'setup-channels' procedure.

[...]

Toggle quote (4 lines)
> +@item @code{channels} (default: @code{'()})
> +List of system channels to use, it populates
> +@file{/etc/guix/channels.scm}.

What about:

List of channels to be used by @command{guix pull}, by default.
Channels listed here are written to @file{/etc/guix/channels.scm}.

?

Toggle quote (3 lines)
> +;; FIXME Does this gexp should be build before boot, such as
> +;; substitute-key-authorization does?

There’s a grammatical issue :-), but also I’m not sure: what are you
worried about?

Toggle quote (3 lines)
> +(define (setup-channels channels)
> + "Return a gexp with code to setup CHANNELS, a list of channels"

Missing period. For the name, how about ‘install-channels-file’
instead?

Toggle quote (3 lines)
> + (channels guix-configuration-channels ;list of channels
> + (default '()))

I wonder if it should default to ‘%default-channels’, for consistency
and least-surprise. In practice, it means we’d always end up creating
/etc/guix/channels.scm, but that’s probably OK. (The downside is if we,
Guix devs, choose to change ‘%default-channels’ at some point: users
would be stuck with the value that got written to /etc. That’s a very
hypothetical situation though.)

WDYT?

Toggle quote (4 lines)
> + #$(if (not (null? channels))
> + (setup-channels channels)
> #~#f))))

In that case, we could remove the (null? channels) special case.

Thanks,
Ludo’.
B
B
Brice Waegeneire wrote on 4 Aug 2021 06:42
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 49610@debbugs.gnu.org)
87eeb9izwl.fsf_-_@waegenei.re
Hello Ludo‘,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (10 lines)
> Brice Waegeneire <brice@waegenei.re> skribis:
>
>> It's more explicit to specify used fields instead of depending on their
>> position.
>>
>> * gnu/services/base.scm (guix-activation): Replace "match" with
>> "match-record".
>
> LGTM!

Thanks, pushed as 92605326ae909471d17b0db51504e810989989f8.

Cheers,
- Brice
L
L
Ludovic Courtès wrote on 1 Sep 2021 23:16
(name . Brice Waegeneire)(address . brice@waegenei.re)(address . 49610@debbugs.gnu.org)
877dg06lok.fsf_-_@gnu.org
Hello Brice,

Any update on this one? :-)

Thanks in advance,
Ludo’.
B
B
Brice Waegeneire wrote on 21 Dec 2021 22:00
[PATCH v2] sevices: guix: Add channels field.
(address . ludo@gnu.org)(address . 49610@debbugs.gnu.org)
20211221210042.6302-1-brice@waegenei.re
* doc/guix.texi (Channels): Specify that '/etc/guix/channels.scm'
contains channels configuration.
(Base Services): Document 'guix-configuration-channels' field.
* gnu/services/base.scm (install-channels-file): New procedure.
(guix-configuration): Add channels field.
(guix-activation): Use 'install-channels-file' procedure.
---
doc/guix.texi | 15 ++++++++++++++-
gnu/services/base.scm | 42 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 3 deletions(-)

I've changed the type of the new field from a list to a s-expression, I'm not
sure if it should be a G-exp instead. The documentation of the
'channels' field as been updated as suggested.

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (6 lines)
>> +;; FIXME Does this gexp should be build before boot, such as
>> +;; substitute-key-authorization does?
>
> There’s a grammatical issue :-), but also I’m not sure: what are you
> worried about?

This is related to your commit 8b3ad455be7e8ace35a2eaebf7fffbb611280852, where
you added pre-computation of the ACL to make « [...] the first boot slightly
faster ». Should this be done in this case too?

Toggle quote (10 lines)
>> + (channels guix-configuration-channels ;list of channels
>> + (default '()))
>
> I wonder if it should default to ‘%default-channels’, for consistency
> and least-surprise. In practice, it means we’d always end up creating
> /etc/guix/channels.scm, but that’s probably OK. (The downside is if we,
> Guix devs, choose to change ‘%default-channels’ at some point: users
> would be stuck with the value that got written to /etc. That’s a very
> hypothetical situation though.)

Users would not have been stuck with a stale ‘%default-channels’, even with
the first version of this patch. The issue with using a non null default
value, is the absence of backward compatibility. A user with an already defined
/etc/guix/chanels.scm, would see its custom channels being replaced by the
default one after having reconfigure a system with this patch for the first
time. So I guess I should make further adjustment to the patch



Toggle diff (123 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index a826171f34..5284a69156 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -5001,7 +5001,7 @@ $ wget -O - \
 Guix and its package collection are updated by running @command{guix pull}
 (@pxref{Invoking guix pull}).  By default @command{guix pull} downloads and
 deploys Guix itself from the official GNU@tie{}Guix repository.  This can be
-customized by defining @dfn{channels} in the
+customized by defining @dfn{channels} in @file{/etc/guix/channels.scm} and
 @file{~/.config/guix/channels.scm} file.  A channel specifies a URL and branch
 of a Git repository to be deployed, and @command{guix pull} can be instructed
 to pull from one or more channels.  In other words, channels can be used
@@ -15557,6 +15557,19 @@ This example assumes that the file @file{./guix.example.org-key.pub}
 contains the public key that @code{guix.example.org} uses to sign
 substitutes.
 
+@item @code{channels} (default: @code{'(cons* %default-channels)})
+S-expression producing a list of channels to be used by @command{guix
+pull}, by default.  The S-exp is written to
+@file{/etc/guix/channels.scm}.
+
+@quotation Note
+When booting or reconfiguring to a system where @code{channels}
+is not null, the existing @file{/etc/guix/channels.scm} file is backed up as
+@file{/etc/guix/channels.scm.bak} if it was determined to be a manually modified
+file.  This is to facilitate migration from earlier versions, which
+allowed for in-place modifications to @file{/etc/guix/channels.scm}.
+@end quotation
+
 @item @code{max-silent-time} (default: @code{0})
 @itemx @code{timeout} (default: @code{0})
 The number of seconds of silence and the number of seconds of activity,
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index e206bea5f0..c9823e6d55 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -58,6 +58,7 @@
   #:use-module (gnu packages terminals)
   #:use-module ((gnu build file-systems)
                 #:select (mount-flags->bit-mask))
+  #:use-module (guix channels)
   #:use-module (guix gexp)
   #:use-module (guix records)
   #:use-module (guix modules)
@@ -66,6 +67,7 @@
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 pretty-print)
   #:re-export (user-processes-service-type        ;backwards compatibility
                %default-substitute-urls)
   #:export (fstab-service-type
@@ -1502,6 +1504,39 @@ archive' public keys, with GUIX."
         ;; Installed the declared ACL.
         (symlink #+default-acl "/etc/guix/acl"))))
 
+;; FIXME Does this gexp should be built before boot, such as
+;; substitute-key-authorization does?
+(define (install-channels-file channels)
+  "Return a gexp with code to install a file with CHANNELS, a S-exp returning
+a list of channels."
+  (define channels-file
+    (plain-file "channels.scm"
+                (with-output-to-string
+                  (lambda _
+                    (pretty-print (map (lambda (channel)
+                                         (if (channel? channel)
+                                             (channel->code channel)
+                                             channel))
+                                       channels))))))
+
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+
+        ;; If channels.scm already exists, move it out of the way. Create a
+        ;; backup if it's a regular file: it's likely that the user
+        ;; manually defined it.
+        (if (file-exists? "/etc/guix/channels.scm")
+            (if (and (symbolic-link? "/etc/guix/channels.scm")
+                     (store-file-name? (readlink "/etc/guix/channels.scm")))
+                (delete-file "/etc/guix/channels.scm")
+                (rename-file "/etc/guix/channels.scm"
+                             "/etc/guix/channels.scm.bak"))
+            (mkdir-p "/etc/guix"))
+
+        ;; Installed the declared channels.
+        (symlink #+channels-file "/etc/guix/channels.scm"))))
+
 (define %default-authorized-guix-keys
   ;; List of authorized substitute keys.
   (list (file-append guix "/share/guix/berlin.guix.gnu.org.pub")
@@ -1524,6 +1559,8 @@ archive' public keys, with GUIX."
                     (default #t))
   (substitute-urls  guix-configuration-substitute-urls ;list of strings
                     (default %default-substitute-urls))
+  (channels         guix-configuration-channels ;sexp
+                    (default '(cons* %default-channels)))
   (chroot-directories guix-configuration-chroot-directories ;list of file-like/strings
                       (default '()))
   (max-silent-time  guix-configuration-max-silent-time ;integer
@@ -1701,7 +1738,7 @@ proxy of 'guix-daemon'...~%")
 (define (guix-activation config)
   "Return the activation gexp for CONFIG."
   (match-record config <guix-configuration>
-    (guix authorize-key? authorized-keys)
+    (guix authorize-key? authorized-keys channels)
     #~(begin
         ;; Assume that the store has BUILD-GROUP as its group.  We could
         ;; otherwise call 'chown' here, but the problem is that on a COW overlayfs,
@@ -1714,7 +1751,8 @@ proxy of 'guix-daemon'...~%")
 
         #$(if authorize-key?
               (substitute-key-authorization authorized-keys guix)
-              #~#f))))
+              #~#f)
+        #$(install-channels-file channels))))
 
 (define* (references-file item #:optional (name "references"))
   "Return a file that contains the list of references of ITEM."
-- 
2.32.0
L
L
Ludovic Courtès wrote on 3 Jan 12:32 +0100
(name . Brice Waegeneire)(address . brice@waegenei.re)(address . 49610@debbugs.gnu.org)
87v8z1kqb4.fsf@gnu.org
Hi Brice,

Brice Waegeneire <brice@waegenei.re> skribis:

Toggle quote (15 lines)
> * doc/guix.texi (Channels): Specify that '/etc/guix/channels.scm'
> contains channels configuration.
> (Base Services): Document 'guix-configuration-channels' field.
> * gnu/services/base.scm (install-channels-file): New procedure.
> (guix-configuration): Add channels field.
> (guix-activation): Use 'install-channels-file' procedure.
> ---
> doc/guix.texi | 15 ++++++++++++++-
> gnu/services/base.scm | 42 ++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 54 insertions(+), 3 deletions(-)
>
> I've changed the type of the new field from a list to a s-expression, I'm not
> sure if it should be a G-exp instead. The documentation of the
> 'channels' field as been updated as suggested.

It’s OK to keep it this way.

Toggle quote (12 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>>> +;; FIXME Does this gexp should be build before boot, such as
>>> +;; substitute-key-authorization does?
>>
>> There’s a grammatical issue :-), but also I’m not sure: what are you
>> worried about?
>
> This is related to your commit 8b3ad455be7e8ace35a2eaebf7fffbb611280852, where
> you added pre-computation of the ACL to make « [...] the first boot slightly
> faster ». Should this be done in this case too?

Ah no, commit 8b3ad455be7e8ace35a2eaebf7fffbb611280852 is about
pre-generating /etc/guix/acl. In the case of /etc/guix/channels.scm,
there’s nothing to pre-generate though since we’re just dumping the sexp
as-is to /etc/guix/channels.scm, so I think this comment can be safely
removed.

[...]

Toggle quote (2 lines)
> +@item @code{channels} (default: @code{'(cons* %default-channels)})

I’d make the default #~%default-channels, no need for ‘cons*’.

Toggle quote (8 lines)
> +@quotation Note
> +When booting or reconfiguring to a system where @code{channels}
> +is not null, the existing @file{/etc/guix/channels.scm} file is backed up as
> +@file{/etc/guix/channels.scm.bak} if it was determined to be a manually modified
> +file. This is to facilitate migration from earlier versions, which
> +allowed for in-place modifications to @file{/etc/guix/channels.scm}.
> +@end quotation

“When (…) where @code{channels} is not null” does not match the actual
code, does it?

Otherwise LGTM, thanks!

Ludo’.
?