[bug#42122] [PATCH 0/3] Support guix system describe and provenance for the Hurd

[bug#42122] [PATCH 0/3] Support guix system describe and provenance for the Hurd

[Jan (janneke) Nieuwenhuizen]

Hello Guix!

This series supports using "guix system describe" on a Chilhurd, showing
provenance info when the disk-image was built using --save-provenance.

Janneke

Jan (janneke) Nieuwenhuizen (3):
  system: 'read-boot-parameters' fixes for multiboot.
  services: system-service-type: Add entries support for the Hurd.
  guix system: "describe" displays multiboot info.

 gnu/system.scm          | 79 ++++++++++++++++++++++++-----------------
 guix/scripts/system.scm |  5 +++
 2 files changed, 52 insertions(+), 32 deletions(-)

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

[bug#42122] [PATCH 1/3] system: 'read-boot-parameters' fixes for multiboot.

[Jan (janneke) Nieuwenhuizen]

* gnu/system.scm (read-boot-parameters): Allow initrd to be unset.  Return
only value for multiboot-modules instead of (key value).
---
 gnu/system.scm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 44baacee7b..a6a9c958e6 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -351,9 +351,13 @@ file system labels."
          (('initrd ('string-append directory file)) ;the old format
           (string-append directory file))
          (('initrd (? string? file))
-          file)))
+          file)
+         (#f #f)))
 
-      (multiboot-modules (or (assq 'multiboot-modules rest) '()))
+      (multiboot-modules
+       (match (assq 'multiboot-modules rest)
+         ((_ args) args)
+         (#f       '())))
 
       (store-device
        ;; Linux device names like "/dev/sda1" are not suitable GRUB device
-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

[bug#42122] [PATCH 2/3] services: system-service-type: Add entries support for the Hurd.

[Jan (janneke) Nieuwenhuizen]

When creating a disk-image using --save-provenance, "guix system describe"
now works.

* gnu/system.scm (operating-system-directory-base-entries): Add conditional
"hurd" parameter, make "initrd" parameter conditional.
(hurd-default-essential-services): Use them.
(operating-system-boot-parameters-file): Only add 'initrd' when set.
---
 gnu/system.scm | 71 +++++++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 30 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index a6a9c958e6..d4641cc9b0 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -537,22 +537,26 @@ possible (that is if there's a LINUX keyword argument in the build system)."
 value of the SYSTEM-SERVICE-TYPE service."
   (let* ((locale  (operating-system-locale-directory os))
          (kernel  (operating-system-kernel os))
+         (hurd    (operating-system-hurd os))
          (modules (operating-system-kernel-loadable-modules os))
-         (kernel  (profile
-                   (content (packages->manifest
-                             (cons kernel
-                                   (map (lambda (module)
-                                          (if (package? module)
-                                              (package-for-kernel kernel
-                                                                  module)
-                                              module))
-                                        modules))))
-                   (hooks (list linux-module-database))))
-         (initrd  (operating-system-initrd-file os))
+         (kernel  (if hurd
+                      kernel
+                      (profile
+                       (content (packages->manifest
+                                 (cons kernel
+                                       (map (lambda (module)
+                                              (if (package? module)
+                                                  (package-for-kernel kernel
+                                                                      module)
+                                                  module))
+                                            modules))))
+                       (hooks (list linux-module-database)))))
+         (initrd  (and (not hurd) (operating-system-initrd-file os)))
          (params  (operating-system-boot-parameters-file os)))
     `(("kernel" ,kernel)
+      ,@(if hurd `(("hurd" ,hurd)) '())
       ("parameters" ,params)
-      ("initrd" ,initrd)
+      ,@(if initrd `(("initrd" ,initrd)) '())
       ("locale" ,locale))))   ;used by libc
 
 (define (operating-system-default-essential-services os)
@@ -604,23 +608,24 @@ bookkeeping."
                                   (operating-system-firmware os)))))))
 
 (define (hurd-default-essential-services os)
-  (list (service system-service-type '())
-        %boot-service
-        %hurd-startup-service
-        %activation-service
-        %shepherd-root-service
-        (service user-processes-service-type)
-        (account-service (append (operating-system-accounts os)
-                                 (operating-system-groups os))
-                         (operating-system-skeletons os))
-        (root-file-system-service)
-        (service file-system-service-type '())
-        (service fstab-service-type
-                 (filter file-system-needed-for-boot?
-                         (operating-system-file-systems os)))
-        (pam-root-service (operating-system-pam-services os))
-        (operating-system-etc-service os)
-        (service profile-service-type (operating-system-packages os))))
+  (let ((entries (operating-system-directory-base-entries os)))
+   (list (service system-service-type entries)
+         %boot-service
+         %hurd-startup-service
+         %activation-service
+         %shepherd-root-service
+         (service user-processes-service-type)
+         (account-service (append (operating-system-accounts os)
+                                  (operating-system-groups os))
+                          (operating-system-skeletons os))
+         (root-file-system-service)
+         (service file-system-service-type '())
+         (service fstab-service-type
+                  (filter file-system-needed-for-boot?
+                          (operating-system-file-systems os)))
+         (pam-root-service (operating-system-pam-services os))
+         (operating-system-etc-service os)
+         (service profile-service-type (operating-system-packages os)))))
 
 (define* (operating-system-services os)
   "Return all the services of OS, including \"essential\" services."
@@ -1276,7 +1281,13 @@ being stored into the \"parameters\" file)."
                      (kernel #$(boot-parameters-kernel params))
                      (kernel-arguments
                       #$(boot-parameters-kernel-arguments params))
-                     (initrd #$(boot-parameters-initrd params))
+                     #$@(if (boot-parameters-initrd params)
+                            #~((initrd #$(boot-parameters-initrd params)))
+                            #~())
+                     #$@(if (pair? (boot-parameters-multiboot-modules params))
+                            #~((multiboot-modules
+                                #$(boot-parameters-multiboot-modules params)))
+                            #~())
                      (bootloader-name #$(boot-parameters-bootloader-name params))
                      (bootloader-menu-entries
                       #$(map menu-entry->sexp
-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

[bug#42122] [PATCH 3/3] guix system: "describe" displays multiboot info.

[Jan (janneke) Nieuwenhuizen]

* guix/scripts/system.scm (display-system-generation): Display
multiboot-modules commands if set.
---
 guix/scripts/system.scm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index d9cf45da23..7f062452ac 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -480,6 +480,7 @@ list of services."
                             (uuid->string root)
                             root))
            (kernel      (boot-parameters-kernel params))
+           (multiboot-modules (boot-parameters-multiboot-modules params))
            (provenance  (catch 'system-error
                           (lambda ()
                             (call-with-input-file
@@ -509,6 +510,10 @@ list of services."
 
       (format #t (G_ "  kernel: ~a~%") kernel)
 
+      (when (pair? multiboot-modules)
+        (format #t (G_ "  multiboot: ~a~%")
+                (string-join (map car multiboot-modules) "\n    ")))
+
       (match provenance
         (#f #t)
         (('provenance ('version 0)
-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

[bug#42122] [PATCH 1/3] system: 'read-boot-parameters' fixes for multiboot.

[Ludovic Courtès]

Hi!

"Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:

> * gnu/system.scm (read-boot-parameters): Allow initrd to be unset.  Return

s/unset/missing/, right?

> only value for multiboot-modules instead of (key value).
> ---
>  gnu/system.scm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index 44baacee7b..a6a9c958e6 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -351,9 +351,13 @@ file system labels."
>           (('initrd ('string-append directory file)) ;the old format
>            (string-append directory file))
>           (('initrd (? string? file))
> -          file)))
> +          file)
> +         (#f #f)))

OK.

> -      (multiboot-modules (or (assq 'multiboot-modules rest) '()))
> +      (multiboot-modules
> +       (match (assq 'multiboot-modules rest)
> +         ((_ args) args)
> +         (#f       '())))

Since this second hunk is a bug fix, I’d rather make it a separate
commit.

Otherwise LGTM!

[bug#42122] [PATCH 2/3] services: system-service-type: Add entries support for the Hurd.

[Ludovic Courtès]

"Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:

> When creating a disk-image using --save-provenance, "guix system describe"
> now works.
>
> * gnu/system.scm (operating-system-directory-base-entries): Add conditional
> "hurd" parameter, make "initrd" parameter conditional.
> (hurd-default-essential-services): Use them.
> (operating-system-boot-parameters-file): Only add 'initrd' when set.

LGTM!  I hadn’t realized there were no ‘system’ entries.

> +  (let ((entries (operating-system-directory-base-entries os)))
> +   (list (service system-service-type entries)
      ^
One missing space for indentation.  :-)

[bug#42122] [PATCH 3/3] guix system: "describe" displays multiboot info.

[Ludovic Courtès]

"Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:

> * guix/scripts/system.scm (display-system-generation): Display
> multiboot-modules commands if set.

[...]

> +      (when (pair? multiboot-modules)
> +        (format #t (G_ "  multiboot: ~a~%")
> +                (string-join (map car multiboot-modules) "\n    ")))

Rather like:

  (match multiboot-modules
    (() #f)
    (((modules . _) ...)
     … (string-join modules "\n    ") …))

Otherwise LGTM, thank you!

Ludo’.

[bug#42122] [PATCH 1/3] system: 'read-boot-parameters' fixes for multiboot.

[Jan Nieuwenhuizen]

Ludovic Courtès writes:

Hi!

> "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:
>
>> * gnu/system.scm (read-boot-parameters): Allow initrd to be unset.  Return
>
> s/unset/missing/, right?

Yes; that's more clear and what I meant.

>> only value for multiboot-modules instead of (key value).
>> ---
>>  gnu/system.scm | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/gnu/system.scm b/gnu/system.scm
>> index 44baacee7b..a6a9c958e6 100644
>> --- a/gnu/system.scm
>> +++ b/gnu/system.scm
>> @@ -351,9 +351,13 @@ file system labels."
>>           (('initrd ('string-append directory file)) ;the old format
>>            (string-append directory file))
>>           (('initrd (? string? file))
>> -          file)))
>> +          file)
>> +         (#f #f)))
>
> OK.

=> to second patch.

>> -      (multiboot-modules (or (assq 'multiboot-modules rest) '()))
>> +      (multiboot-modules
>> +       (match (assq 'multiboot-modules rest)
>> +         ((_ args) args)
>> +         (#f       '())))
>
> Since this second hunk is a bug fix, I’d rather make it a separate
> commit.

Great, => to first bugfix patch.

> Otherwise LGTM!

Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

[bug#42122] [PATCH 2/3] services: system-service-type: Add entries support for the Hurd.

[Jan Nieuwenhuizen]

Ludovic Courtès writes:

> "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:
>
>> When creating a disk-image using --save-provenance, "guix system describe"
>> now works.
>>
>> * gnu/system.scm (operating-system-directory-base-entries): Add conditional
>> "hurd" parameter, make "initrd" parameter conditional.
>> (hurd-default-essential-services): Use them.
>> (operating-system-boot-parameters-file): Only add 'initrd' when set.
>
> LGTM!  I hadn’t realized there were no ‘system’ entries.

Yeah, I guess they were present once and removed during our cross-build
woes to make a smaller patch series.  Anyway...

>> +  (let ((entries (operating-system-directory-base-entries os)))
>> +   (list (service system-service-type entries)
>       ^
> One missing space for indentation.  :-)

Oh!  Thanks.  Fixed.

Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

bug#42122: [PATCH 3/3] guix system: "describe" displays multiboot info.

[Jan Nieuwenhuizen]

Ludovic Courtès writes:

Hi!

> "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:
>
>> * guix/scripts/system.scm (display-system-generation): Display
>> multiboot-modules commands if set.
>
> [...]
>
>> +      (when (pair? multiboot-modules)
>> +        (format #t (G_ "  multiboot: ~a~%")
>> +                (string-join (map car multiboot-modules) "\n    ")))
>
> Rather like:
>
>   (match multiboot-modules
>     (() #f)
>     (((modules . _) ...)
>      … (string-join modules "\n    ") …))
>
> Otherwise LGTM, thank you!

Thanks, done!

Pushed series to master as 28febfafbb23561624cc5c4ac8ed581f1f867f70

Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com