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

  • Done
  • quality assurance status badge
Details
2 participants
  • Jan (janneke) Nieuwenhuizen
  • Ludovic Courtès
Owner
unassigned
Submitted by
Jan (janneke) Nieuwenhuizen
Severity
normal
J
J
Jan (janneke) Nieuwenhuizen wrote on 29 Jun 2020 15:55
(address . guix-patches@gnu.org)
20200629135559.12696-1-janneke@gnu.org
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
J
J
Jan (janneke) Nieuwenhuizen wrote on 29 Jun 2020 15:58
[PATCH 1/3] system: 'read-boot-parameters' fixes for multiboot.
(address . 42122@debbugs.gnu.org)
20200629135817.12784-1-janneke@gnu.org
* 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(-)

Toggle diff (23 lines)
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
J
J
Jan (janneke) Nieuwenhuizen wrote on 29 Jun 2020 15:58
[PATCH 2/3] services: system-service-type: Add entries support for the Hurd.
(address . 42122@debbugs.gnu.org)
20200629135817.12784-2-janneke@gnu.org
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(-)

Toggle diff (103 lines)
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
J
J
Jan (janneke) Nieuwenhuizen wrote on 29 Jun 2020 15:58
[PATCH 3/3] guix system: "describe" displays multiboot info.
(address . 42122@debbugs.gnu.org)
20200629135817.12784-3-janneke@gnu.org
* guix/scripts/system.scm (display-system-generation): Display
multiboot-modules commands if set.
---
guix/scripts/system.scm | 5 +++++
1 file changed, 5 insertions(+)

Toggle diff (26 lines)
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
L
L
Ludovic Courtès wrote on 2 Jul 2020 23:42
Re: [bug#42122] [PATCH 1/3] system: 'read-boot-parameters' fixes for multiboot.
(name . Jan (janneke) Nieuwenhuizen)(address . janneke@gnu.org)(address . 42122@debbugs.gnu.org)
87v9j5pp99.fsf@gnu.org
Hi!

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

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

s/unset/missing/, right?

Toggle quote (17 lines)
> 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.

Toggle quote (6 lines)
> - (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!
L
L
Ludovic Courtès wrote on 2 Jul 2020 23:50
Re: [bug#42122] [PATCH 2/3] services: system-service-type: Add entries support for the Hurd.
(name . Jan (janneke) Nieuwenhuizen)(address . janneke@gnu.org)(address . 42122@debbugs.gnu.org)
87r1ttpowh.fsf@gnu.org
"Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:

Toggle quote (8 lines)
> 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.

Toggle quote (2 lines)
> + (let ((entries (operating-system-directory-base-entries os)))
> + (list (service system-service-type entries)
^
One missing space for indentation. :-)
L
L
Ludovic Courtès wrote on 2 Jul 2020 23:51
Re: [bug#42122] [PATCH 3/3] guix system: "describe" displays multiboot info.
(name . Jan (janneke) Nieuwenhuizen)(address . janneke@gnu.org)(address . 42122@debbugs.gnu.org)
87mu4hpoub.fsf@gnu.org
"Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:

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

[...]

Toggle quote (4 lines)
> + (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’.
J
J
Jan Nieuwenhuizen wrote on 3 Jul 2020 09:42
Re: [bug#42122] [PATCH 1/3] system: 'read-boot-parameters' fixes for multiboot.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 42122@debbugs.gnu.org)
878sg12gfo.fsf@gnu.org
Ludovic Courtès writes:

Hi!

Toggle quote (6 lines)
> "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.

Toggle quote (19 lines)
>> 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.

Toggle quote (9 lines)
>> - (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.

Toggle quote (2 lines)
> Otherwise LGTM!

Janneke

--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com| Avatar® http://AvatarAcademy.com
J
J
Jan Nieuwenhuizen wrote on 3 Jul 2020 09:42
Re: [bug#42122] [PATCH 2/3] services: system-service-type: Add entries support for the Hurd.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 42122@debbugs.gnu.org)
877dvl2gf8.fsf@gnu.org
Ludovic Courtès writes:

Toggle quote (12 lines)
> "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...

Toggle quote (5 lines)
>> + (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
J
J
Jan Nieuwenhuizen wrote on 3 Jul 2020 09:42
Re: [bug#42122] [PATCH 3/3] guix system: "describe" displays multiboot info.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 42122-done@debbugs.gnu.org)
875zb52get.fsf@gnu.org
Ludovic Courtès writes:

Hi!

Toggle quote (20 lines)
> "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
Closed
?