Subject: [bug#48314] [PATCH] Install guix system on Raspberry Pi

From: Maxim Cournoyer <[email-masked]>
Date: Thu, 01 Dec 2022 09:25:29 -0500

Hi Stefan!

Some comments/question for this proposed change.

Stefan <[email-masked]> writes:

[...]

> gnu: linux: Fix the extra-version parameter in make-linux-libre*.

This first commit LGTM.  I'll push it shortly.

[...]

> gnu: bootloader: Rework chaining, add grub-efi-netboot-removable-bootloader.
>
> From: Stefan <[email-masked]>
>
> * doc/guix.texi (Bootloader Configuration): Describe the new
> ‘grub-efi-netboot-removable-bootloader’.  Mention used sub-directories and
> that the UEFI Boot Manager is not modified.  Advice to disable write-access
> over TFTP.
> * gnu/bootloader.scm (efi-bootloader-profile): Allow a list of packages and
> collect everything directly in the profile, avoiding a separate collection
> directory.  Renamed the profile from "bootloader-profile" to
> "efi-bootloader-profile".
> [bootloader-collection]: Renamed to …
> [efi-bootloader-profile-hook]: … this and removed unused modules and the
> creation of the now unneeded collection directory.
> (efi-bootloader-chain): Added packages and disk-image-installer arguments.
> Removed handling of the collection directory, now only calling the given
> installer procedure.
> * gnu/bootloader/grub.scm (make-grub-efi-netboot-installer): New helper.
> (make-grub-configuration): New helper based on (grub-configuration-file).
> Adding grub argument, fixed indentation, removend code to get grub.
> (grub-configuration-file): Now using (make-grub-configuration).
> (grub-efi-configuration-file): New function using (make-grub-configuration).
> Instead of getting the grub-efi package from the bootloader-configuration
> this function refers to the grub-efi package directly.
> (grub-cfg): New variable to replace "/boot/grub/grub.cfg".
> (install-grub-efi-netboot): Removed, the functionality got moved.
> (make-grub-efi-netboot-installer): New helper function to return a customized
> installer for a certain efi-sub-directory.  The installer basically copies
> a pre-installed efi-bootloader-profile, and adds needed symlinks for booting
> over network, or – on an ESP – an intermediate grub-cfg to load the final
> grub-cfg file.
> (grub-bootloader): Now using the grub-cfg variable.
> (grub-efi-bootloader): Now using the grub-cfg variable.  Removed inheritance,
> giving complete set of fields.
> (make-grub-efi-netboot-bootloader): New helper function.
> (grub-efi-netboot-bootloader): Now using the helper.
> (grub-efi-netboot-removable-bootloader): New bootloader using the helper.
> It uses the efi-sub-directory "efi/boot" for removable media.
> * gnu/packages/bootloaders.scm (make-grub-efi-netboot): New function to return
> a grub-efi package pre-installed via grub-mknetdir, customized for an
> efi-sub-directory and able to boot via network and local storage.
>
> The rework allows to use an (efi-bootloader-chain) like this, which is able
> to boot over network or local storage, depending on the symlink-support at
> the bootloader-target:
>
> (operating-system
>  (bootloader
>    (bootloader-configuration
>      (bootloader
>        (efi-bootloader-chain
>          grub-efi-netboot-removable-bootloader
>          #:packages (list my-firmware-package
>                           my-u-boot-package)
>          #:files (list (plain-file "config.txt"
>                                    "kernel=u-boot.bin"))
>          #:hooks my-special-bootloader-profile-manipulator))
>      (target "/booti/efi")
>      …))
>  …)
> )

That's *a lot* of text :-). For the future, some of the things there are
improvements rather than necessary changes it seems, so could have been
split into something different, smaller & easier to review.  I've
standardized to use the imperative tense in the change log message (Added
-> Add for example).

[...]

> +(define (grub-configuration-file config . args)
> +  (let* ((bootloader (bootloader-configuration-bootloader config))
> +         (grub (bootloader-package bootloader)))
> +    (apply make-grub-configuration grub config args)))
> +
> +(define (grub-efi-configuration-file . args)
> +  (apply make-grub-configuration grub-efi args))
> +
> +(define grub-cfg "/boot/grub/grub.cfg")

In GRUB-EFI-CONFIGURATION-FILE above, why do we hard-code grub-efi
instead of retrieving it from config the same as for
GRUB-CONFIGURATION-FILE?  It seems that'd be preferable, as otherwise
someone cannot override GRUB-EFI with their own variant, no?

>  
>  
>  ;;;
> @@ -674,42 +681,31 @@ fi~%"))))
>                                ((target-arm?) "--target=arm-efi"))
>                          "--efi-directory" target-esp)))))
>  
> -(define (install-grub-efi-netboot subdir)
> -  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
> -which is usually efi/Guix or efi/boot."
> -  (let* ((system (string-split (nix-system->gnu-triplet
> -                                (or (%current-target-system)
> -                                    (%current-system)))
> -                               #\-))
> -         (arch (first system))
> -         (boot-efi-link (match system
> -                          ;; These are the supportend systems and the names
> -                          ;; defined by the UEFI standard for removable media.
> -                          (("i686" _ ...)        "/bootia32.efi")
> -                          (("x86_64" _ ...)      "/bootx64.efi")
> -                          (("arm" _ ...)         "/bootarm.efi")
> -                          (("aarch64" _ ...)     "/bootaa64.efi")
> -                          (("riscv" _ ...)       "/bootriscv32.efi")
> -                          (("riscv64" _ ...)     "/bootriscv64.efi")
> -                          ;; Other systems are not supported, although defined.
> -                          ;; (("riscv128" _ ...) "/bootriscv128.efi")
> -                          ;; (("ia64" _ ...)     "/bootia64.efi")
> -                          ((_ ...)               #f)))
> -         (core-efi (string-append
> -                    ;; This is the arch dependent file name of GRUB, e.g.
> -                    ;; i368-efi/core.efi or arm64-efi/core.efi.
> -                    (match arch
> -                      ("i686"    "i386")
> -                      ("aarch64" "arm64")
> -                      ("riscv"   "riscv32")
> -                      (_         arch))
> -                    "-efi/core.efi")))
> -    (with-imported-modules
> -     '((guix build union))
> -     #~(lambda (bootloader target mount-point)
> -         "Install the BOOTLOADER, which must be the package grub, as e.g.
> -bootx64.efi or bootaa64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
> -below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
> +(define* (make-grub-efi-netboot-installer grub-efi grub-cfg subdir)
> +  "Make a bootloader-installer for a grub-efi-netboot bootloader, which expects
> +its files in SUBDIR and its configuration file in GRUB-CFG.
> +
> +As a grub-efi-netboot package is already preinstalled by 'grub-mknetdir', the
> +installer basically copies all files from the bootloader-package (or profile)
> +into the bootloader-target directory.
> +
> +Additionally for network booting over TFTP, two relative symlinks to the store
> +and to the GRUB-CFG file are necessary.  Due to this a TFTP root directory must
> +not be located on a FAT file-system.
> +
> +If the bootloader-target does not support symlinks, then it is assumed to be a
> +kind of EFI System Partition (ESP).  In this case an intermediate configuration
> +file is created with the help of GRUB-EFI to load the GRUB-CFG.
> +
> +The installer is usable for any efi-bootloader-chain, which prepares the
> +bootloader-profile in a way ready for copying.
> +
> +The installer does not manipulate the system's 'UEFI Boot Manager'."
> +  (with-imported-modules '((guix build union))
> +    #~(lambda (bootloader target mount-point)
> +        "Copy the BOOTLOADER, which must be a preinstalled grub-efi-netboot
> +package with a SUBDIR like efi/boot or efi/Guix, below the directory
> +TARGET for the system whose root is mounted at MOUNT-POINT.
>  
>  MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
>  or '/' for other 'guix system' commands.
> @@ -719,17 +715,18 @@ bootloader-configuration in:

I've unified the above docstring as one; otherwise it was mangled with
Scheme and it wouldn't have appeared as a whole in the online
documentation system of Guile.

I've improved the writing a bit (I think!), use gexps in some places,
and other smallish changes that amount to:

--8<---------------cut here---------------start------------->8---
4 files changed, 80 insertions(+), 76 deletions(-)
doc/guix.texi                | 17 +++++++++--------
gnu/bootloader.scm           | 11 +++++------
gnu/bootloader/grub.scm      | 77 ++++++++++++++++++++++++++++++++++++++++-------------------------------------
gnu/packages/bootloaders.scm | 51 ++++++++++++++++++++++++++-------------------------

modified   doc/guix.texi
@@ -38083,17 +38083,18 @@ NFS servers, you also need a properly configured DHCP server to make the booting
 over netboot possible.  For all this we can currently only recommend you to look
 for instructions about @acronym{PXE, Preboot eXecution Environment}.
 
-If a local EFI System Partition (ESP) or a similar partition with a FAT file
-system is mounted in @code{targets}, then symlinks cannot be created.  In this
-case everything will be prepared for booting from local storage, simialar as if
-using @code{grub-efi-bootloader}, with the difference that all GRUB binaries
-reside on @code{targets}, too, like needed for booting over network.
+If a local EFI System Partition (ESP) or a similar partition with a FAT
+file system is mounted in @code{targets}, then symlinks cannot be
+created.  In this case everything will be prepared for booting from
+local storage, matching the behavior of @code{grub-efi-bootloader}, with
+the difference that all GRUB binaries are copied to @code{targets},
+necessary for booting over the network.
 
 @vindex grub-efi-netboot-removable-bootloader
 @code{grub-efi-netboot-removable-bootloader} is identical to
-@code{grub-efi-netboot-bootloader} with the exception that the sub-directory
-@file{efi/boot} will be used instead of @file{efi/Guix} to comply to the UEFI
-specification for removable media.
+@code{grub-efi-netboot-bootloader} with the exception that the
+sub-directory @file{efi/boot} will be used instead of @file{efi/Guix} to
+comply with the UEFI specification for removable media.
 
 @quotation Note
 This @emph{will} overwrite the GRUB file from any other operating systems that
modified   gnu/bootloader.scm
@@ -361,8 +361,7 @@ (define name-ends-with-/? (cut string-suffix? "/" <>))
             (define (name-is-store-entry? name)
               "Return #t if NAME is a direct store entry and nothing inside."
               (not (string-index (strip-store-file-name name) #\/)))
-            (let* ((output #$output)
-                   (files '#$files)
+            (let* ((files '#$files)
                    (directories (filter name-ends-with-/? files))
                    (names-from-directories
                     (append-map (lambda (directory)
@@ -370,11 +369,11 @@ (define (name-is-store-entry? name)
                                 directories))
                    (names (append names-from-directories
                                   (remove name-ends-with-/? files))))
-              (mkdir-p output)
+              (mkdir-p #$output)
               (if (every file-exists? names)
                   (begin
                     (for-each (lambda (name)
-                               (symlink-to name output
+                               (symlink-to name #$output
                                             (if (name-is-store-entry? name)
                                                 strip-store-file-name
                                                 basename)))
@@ -410,7 +409,7 @@ (define* (efi-bootloader-chain final-bootloader
 The package of the FINAL-BOOTLOADER and all PACKAGES and FILES will be placed
 in an efi-bootloader-profile, which will be passed to the INSTALLER.
 
-FILES may contain file like objects produced by procedures like plain-file,
+FILES may contain file-like objects produced by procedures like plain-file,
 local-file, etc., or package contents produced with file-append.
 
 If a directory name in FILES ends with '/', then the directory content instead
@@ -424,7 +423,7 @@ (define* (efi-bootloader-chain final-bootloader
 FINAL-BOOTLOADER will be called.
 
 If the DISK-IMAGE-INSTALLER is used, then this gexp procedure will be called
-to install the efi-bootloader-profile into a disk-image.  Otherwise the
+to install the efi-bootloader-profile into a disk image.  Otherwise the
 disk-image-installer of the FINAL-BOOTLOADER will be called."
   (bootloader
     (inherit final-bootloader)
modified   gnu/bootloader/grub.scm
@@ -685,7 +685,7 @@ (define* (make-grub-efi-netboot-installer grub-efi grub-cfg subdir)
   "Make a bootloader-installer for a grub-efi-netboot bootloader, which expects
 its files in SUBDIR and its configuration file in GRUB-CFG.
 
-As a grub-efi-netboot package is already preinstalled by 'grub-mknetdir', the
+As a grub-efi-netboot package is already pre-installed by 'grub-mknetdir', the
 installer basically copies all files from the bootloader-package (or profile)
 into the bootloader-target directory.
 
@@ -700,12 +700,12 @@ (define* (make-grub-efi-netboot-installer grub-efi grub-cfg subdir)
 The installer is usable for any efi-bootloader-chain, which prepares the
 bootloader-profile in a way ready for copying.
 
-The installer does not manipulate the system's 'UEFI Boot Manager'."
-  (with-imported-modules '((guix build union))
-    #~(lambda (bootloader target mount-point)
-        "Copy the BOOTLOADER, which must be a preinstalled grub-efi-netboot
-package with a SUBDIR like efi/boot or efi/Guix, below the directory
-TARGET for the system whose root is mounted at MOUNT-POINT.
+The installer does not manipulate the system's 'UEFI Boot Manager'.
+
+The returned installer accepts the BOOTLOADER, TARGET and MOUNT-POINT
+arguments.  Its job is to copy the BOOTLOADER, which must be a pre-installed
+grub-efi-netboot package with a SUBDIR like efi/boot or efi/Guix, below the
+directory TARGET for the system whose root is mounted at MOUNT-POINT.
 
 MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
 or '/' for other 'guix system' commands.
@@ -720,13 +720,14 @@ (define* (make-grub-efi-netboot-installer grub-efi grub-cfg subdir)
  …)
 
 TARGET is required to be an absolute directory name, usually mounted via NFS,
-and finally needs to be provided by a TFTP server as the TFTP root directory.
+and finally needs to be provided by a TFTP server as
+the TFTP root directory.
 
 Usually the installer will be used to prepare network booting over TFTP.  Then
 GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
 load more files from the store like tftp://server/gnu/store/…-linux…/Image.
 
-To make this possible two symlinks will be created.  The first symlink points
+To make this possible two symlinks are created.  The first symlink points
 relatively form MOUNT-POINT/TARGET/SUBDIR/grub.cfg to
 MOUNT-POINT/boot/grub/grub.cfg, and the second symlink points relatively from
 MOUNT-POINT/TARGET/%store-prefix to MOUNT-POINT/%store-prefix.
@@ -740,16 +741,18 @@ (define* (make-grub-efi-netboot-installer grub-efi grub-cfg subdir)
 accesses outside its TFTP root directory.  This all may need to be considered
 for security aspects.  It is advised to disable any TFTP write access!
 
-The installer can also be used to prepare booting from local storages, if the
+The installer can also be used to prepare booting from local storage, if the
 underlying file-system, like FAT on an EFI System Partition (ESP), does not
 support symlinks.  In this case the MOUNT-POINT/TARGET/SUBDIR/grub.cfg will be
 created with the help of GRUB-EFI to load the /boot/grub/grub.cfg file.  A
 symlink to the store is not needed in this case."
+  (with-imported-modules '((guix build union))
+    #~(lambda (bootloader target mount-point)
         ;; In context of a disk image creation TARGET will be #f and an
         ;; installer is expected to do necessary installations on MOUNT-POINT,
-        ;; which will become the root file system.
-        ;; If TARGET is #f, this installer has nothing to do, as it only cares
-        ;; about the EFI System Partition (ESP).
+        ;; which will become the root file system.  If TARGET is #f, this
+        ;; installer has nothing to do, as it only cares about the EFI System
+        ;; Partition (ESP).
         (when target
           (use-modules ((guix build union) #:select (symlink-relative))
                        (ice-9 popen)
@@ -779,35 +782,35 @@ (define* (make-grub-efi-netboot-installer grub-efi grub-cfg subdir)
             (mkdir-p (dirname grub-cfg-link))
             (false-if-exception (delete-file grub-cfg-link))
             (if (unspecified?
-                (false-if-exception (symlink-relative grub-cfg grub-cfg-link)))
-              ;; Symlinks are supported.
-              (begin
-                ;; Prepare the symlink to the store.
-                (mkdir-p (dirname store-link))
-                (false-if-exception (delete-file store-link))
-                (symlink-relative store store-link))
-              ;; Creating symlinks does not seem to be supported.
-              ;; Probably an ESP is used.
-              ;; Instead we can script to search and load the actual grub.cfg.
-              (let* ((probe #$(file-append grub-efi "/sbin/grub-probe"))
-                     (port
-                       (open-pipe* OPEN_READ probe "--target=fs_uuid" grub-cfg))
-                     (search-root
-                       (match (read-line port)
-                         ((? eof-object?)
+                 (false-if-exception (symlink-relative grub-cfg grub-cfg-link)))
+                ;; Symlinks are supported.
+                (begin
+                  ;; Prepare the symlink to the store.
+                  (mkdir-p (dirname store-link))
+                  (false-if-exception (delete-file store-link))
+                  (symlink-relative store store-link))
+                ;; Creating symlinks does not seem to be supported.  Probably
+                ;; an ESP is used.  Add a script to search and load the actual
+                ;; grub.cfg.
+                (let* ((probe #$(file-append grub-efi "/sbin/grub-probe"))
+                       (port (open-pipe* OPEN_READ probe "--target=fs_uuid"
+                                         grub-cfg))
+                       (search-root
+                        (match (read-line port)
+                          ((? eof-object?)
                            ;; There is no UUID available. As a fallback search
                            ;; everywhere for the grub.cfg.
                            (string-append "search --file --set " #$grub-cfg))
-                         (fs-uuid
+                          (fs-uuid
                            ;; The UUID to load the grub.cfg from is known.
                            (string-append "search --fs-uuid --set " fs-uuid))))
-                     (load-grub-cfg (string-append "configfile " #$grub-cfg)))
-                (close-pipe port)
-                (with-output-to-file grub-cfg-link
-                  (lambda ()
-                    (display (string-join (list search-root
-                                                load-grub-cfg)
-                                          "\n")))))))))))
+                       (load-grub-cfg (string-append "configfile " #$grub-cfg)))
+                  (close-pipe port)
+                  (with-output-to-file grub-cfg-link
+                    (lambda ()
+                      (display (string-join (list search-root
+                                                  load-grub-cfg)
+                                            "\n")))))))))))
 
 
 
modified   gnu/packages/bootloaders.scm
@@ -427,8 +427,8 @@ (define-public (make-grub-efi-netboot name subdir)
     (build-system trivial-build-system)
     (arguments
      (let* ((system (string-split (nix-system->gnu-triplet
-                                  (or (%current-target-system)
-                                      (%current-system)))
+                                   (or (%current-target-system)
+                                       (%current-system)))
                                   #\-))
             (arch (first system))
             (boot-efi
@@ -454,29 +454,30 @@ (define-public (make-grub-efi-netboot name subdir)
                          ("riscv"   "riscv32")
                          (_         arch))
                        "-efi/core.efi")))
-       `(#:modules ((guix build utils))
-         #:builder
-         (begin
-           (use-modules (guix build utils))
-           (let* ((bootloader (assoc-ref %build-inputs "grub-efi"))
-                  (net-dir (assoc-ref %outputs "out"))
-                  (sub-dir (string-append net-dir "/" ,subdir "/"))
-                  (boot-efi (string-append sub-dir ,boot-efi))
-                  (core-efi (string-append sub-dir ,core-efi)))
-             ;; Install GRUB, which refers to the grub.cfg, with support for
-             ;; encrypted partitions,
-             (setenv "GRUB_ENABLE_CRYPTODISK" "y")
-             (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
-                           (string-append "--net-directory=" net-dir)
-                           (string-append "--subdir=" ,subdir)
-                           ;; These modules must be preloaded to allow booting
-                           ;; from an ESP or a similar partition with a FAT
-                           ;; file system.
-                           (string-append "--modules=part_msdos part_gpt fat"))
-             ;; Move GRUB's core.efi to the removable media name.
-             (false-if-exception (delete-file boot-efi))
-             (rename-file core-efi boot-efi))))))
-    (inputs `(("grub-efi" ,grub-efi)))
+       (list
+        #:modules ((guix build utils))
+        #:builder
+        #~(begin
+            (use-modules (guix build utils))
+            (let* ((bootloader #$(this-package-input "grub-efi"))
+                   (net-dir #$output)
+                   (sub-dir (string-append net-dir "/" #$subdir "/"))
+                   (boot-efi (string-append sub-dir #$boot-efi))
+                   (core-efi (string-append sub-dir #$core-efi)))
+              ;; Install GRUB, which refers to the grub.cfg, with support for
+              ;; encrypted partitions,
+              (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+              (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                            (string-append "--net-directory=" net-dir)
+                            (string-append "--subdir=" #$subdir)
+                            ;; These modules must be pre-loaded to allow booting
+                            ;; from an ESP or a similar partition with a FAT
+                            ;; file system.
+                            (string-append "--modules=part_msdos part_gpt fat"))
+              ;; Move GRUB's core.efi to the removable media name.
+              (false-if-exception (delete-file boot-efi))
+              (rename-file core-efi boot-efi))))))
+    (inputs (list grub-efi))
     (synopsis (package-synopsis grub-efi))
     (description (package-description grub-efi))
     (home-page (package-home-page grub-efi))
--8<---------------cut here---------------end--------------->8---

It's a pity we do not have tests for that, but I'll try to test it
manually and if it works I can push it shortly.  I'd still like feedback
on my question above.

-- 
Thanks,
Maxim