LUKS2 support in Guix

LUKS2 support in Guix

[Fabio Natali]

Hi 👋,

I wasn't able to use a LUKS2+PBKDF2 encrypted partition when setting up
a machine recently. I understand this isn't supported by the version of
GRUB currently shipped in Guix.

Basically, with a LUKS2+PBKDF2 drive, you get stuck at boot with no
chance for GRUB to detect the relevant partitions. Or, at least, that
was my experience with that setup.

The Guix manual would indicate that LUKS2 is actually supported, when
used in combination with PBKDF2⁰:

> Note that GRUB can unlock LUKS2 devices since version 2.06, but only
> supports the PBKDF2 key derivation function, which is not the default
> for cryptsetup luksFormat. You can check which key derivation function
> is being used by a device by running cryptsetup luksDump device, and
> looking for the PBKDF field of your keyslots.

If I'm right in thinking that LUKS2+PBKDF2 is not supported and there's
no clear timeline for a fix yet, could it be worth to amend the manual
to say that it has to be LUKS1 at this stage?

Glad to amend the manual in case, but I might as well be missing
something here, so I wanted to check with you first.

Thanks, best wishes, Fabio.


⁰ https://guix.gnu.org/manual/devel/en/html_node/Keyboard-Layout-and-Networking-and-Partitioning.html#Disk-Partitioning


-- 
Fabio Natali
https://fabionatali.com

Re: LUKS2 support in Guix

[Felix Lechner via Development of GNU Guix and the GNU System distribution.]

Hi Fabio,

On Fri, Mar 01 2024, Fabio Natali wrote:

> could it be worth to amend the manual to say that it has to be LUKS1

Based on the many folks who trip over this, especially on IRC, that
seems like a great idea!

Kind regards
Felix

Re: LUKS2 support in Guix

[Oleg Pykhalov]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

Hi,

Felix Lechner via "Development of GNU Guix and the GNU System
distribution." <guix-devel@gnu.org> writes:

> On Fri, Mar 01 2024, Fabio Natali wrote:
>
>> could it be worth to amend the manual to say that it has to be LUKS1
>
> Based on the many folks who trip over this, especially on IRC, that
> seems like a great idea!

I also lost some time due to this incorrect statement. Perhaps it would
be better to replace the LUKS creation step in the info manual with a
link to all the steps listed in gnu/tests/install.scm.


Regards,
Oleg.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: LUKS2 support in Guix

[Fabio Natali]

On 2024-03-02, 10:41 +0300, Oleg Pykhalov <go.wigust@gmail.com> wrote:
> I also lost some time due to this incorrect statement.

Brilliant, thanks Felix and Oleg.

FWIW, micro-patch submitted here⁰.

Let's see what the patch reviewer thinks. I'm not terribly happy about
"downgrading" the manual's recommendation from LUKS2 to LUKS1, but if
LUKS2 is really not supported, then of course that needs to be reflected
in the docs.

Best wishes, Fabio.


⁰ Ticket 69507 on debbugs.gnu.org. I wanted to link to the relevant page
on https://lists.gnu.org/archive/html/guix-patches/2024-03/index.html
but it seem it's taking a while for the mail to pass moderation.


-- 
Fabio Natali
https://fabionatali.com

Re: LUKS2 support in Guix

[Josselin Poiret]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

Hi Fabio and everyone,

Fabio Natali <me@fabionatali.com> writes:

> On 2024-03-02, 10:41 +0300, Oleg Pykhalov <go.wigust@gmail.com> wrote:
>> I also lost some time due to this incorrect statement.
>
> Brilliant, thanks Felix and Oleg.
>
> FWIW, micro-patch submitted here⁰.
>
> Let's see what the patch reviewer thinks. I'm not terribly happy about
> "downgrading" the manual's recommendation from LUKS2 to LUKS1, but if
> LUKS2 is really not supported, then of course that needs to be reflected
> in the docs.
>
> Best wishes, Fabio.
>
>
> ⁰ Ticket 69507 on debbugs.gnu.org. I wanted to link to the relevant page
> on https://lists.gnu.org/archive/html/guix-patches/2024-03/index.html
> but it seem it's taking a while for the mail to pass moderation.

The reason why it hasn't been spelled out in the manual is because I
thought that we could simply upgrade to Grub 2.12, which would solve the
issue as it supports LUKS2 properly.  I have a patch locally upgrading
Grub, but it's not in upstreaming shape yet and I haven't had much time
to work on it recently.  I am running it though, and it's working quite
well.  Would anyone be interested in cleaning it up and sending it
upstream?

Best,
-- 
Josselin Poiret

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 682 bytes --]

Re: LUKS2 support in Guix

[Maxim Cournoyer]

Hello,

Fabio Natali <me@fabionatali.com> writes:

> On 2024-03-02, 10:41 +0300, Oleg Pykhalov <go.wigust@gmail.com> wrote:
>> I also lost some time due to this incorrect statement.
>
> Brilliant, thanks Felix and Oleg.
>
> FWIW, micro-patch submitted here⁰.
>
> Let's see what the patch reviewer thinks. I'm not terribly happy about
> "downgrading" the manual's recommendation from LUKS2 to LUKS1, but if
> LUKS2 is really not supported, then of course that needs to be reflected
> in the docs.
>
> Best wishes, Fabio.
>
>
> ⁰ Ticket 69507 on debbugs.gnu.org. I wanted to link to the relevant page
> on https://lists.gnu.org/archive/html/guix-patches/2024-03/index.html
> but it seem it's taking a while for the mail to pass moderation.

Is there something preventing us from updating GRUB to benefit from full
LUKS2 support?

-- 
Thanks,
Maxim

Re: LUKS2 support in Guix

[Fabio Natali]

Hi Josselin,

Thanks for getting back to me.

On 2024-03-02, 22:23 +0100, Josselin Poiret <dev@jpoiret.xyz> wrote:
> I have a patch locally upgrading Grub, but it's not in upstreaming
> shape yet and I haven't had much time to work on it recently.  I am
> running it though, and it's working quite well.  Would anyone be
> interested in cleaning it up and sending it upstream?

Sure, I'd be interested. Is it hosted anywhere public or do you want to
share it here or in PVT?

Thanks, Fabio.

Re: LUKS2 support in Guix

[Fabio Natali]

Hi Maxim,

On 2024-03-02, 22:08 -0500, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
> Is there something preventing us from updating GRUB to benefit from
> full LUKS2 support?

That I don't know.

There seems to be a patch that updates to GRUB 2.12 already, see other
message in this thread. If the patch can be made upstream-ready without
too much effort, that'd be the way to go - as opposed to amending the
manual.

Cheers, Fabio.

[PATCH 1/2] gnu: grub: Update to 2.12.

[Josselin Poiret]

From: Josselin Poiret <dev@jpoiret.xyz>

* gnu/packages/bootloaders.scm (grub): Update to 2.12.

Change-Id: I5d9ae3952b61b47418cb5666671fc0bcb5318e72
---
 gnu/packages/bootloaders.scm | 182 ++++++++++++++++++++---------------
 1 file changed, 102 insertions(+), 80 deletions(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 986f0ac035..1327055484 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -103,94 +103,113 @@ (define unifont
 (define-public grub
   (package
     (name "grub")
-    (version "2.06")
+    (version "2.12")
     (source (origin
               (method url-fetch)
               (uri (string-append "mirror://gnu/grub/grub-" version ".tar.xz"))
               (sha256
                (base32
-                "1qbycnxkx07arj9f2nlsi9kp0dyldspbv07ysdyd34qvz55a97mp"))
+                "1ahgzvvvwdxx7rl08pv5dyqlgp76jxz0q2cflxvsdsn4yy8p7jgk"))
               (patches (search-patches
                         "grub-efi-fat-serial-number.patch"
-                        "grub-setup-root.patch"))
-              (modules '((guix build utils)))
-              (snippet
-               '(begin
-                  ;; Adjust QEMU invocation to not use a deprecated device
-                  ;; name that was removed in QEMU 6.0.  Remove for >2.06.
-                  (substitute* "tests/ahci_test.in"
-                    (("ide-drive")
-                     "ide-hd"))))))
+                        "grub-setup-root.patch"))))
     (build-system gnu-build-system)
     (arguments
-     `(#:configure-flags
-       ;; Counterintuitively, this *disables* a spurious Python dependency by
-       ;; calling the ‘true’ binary instead.  Python is only needed during
-       ;; bootstrapping (for genptl.py), not when building from a release.
-       (list "PYTHON=true")
-       ;; Grub fails to load modules stripped with --strip-unneeded.
-       #:strip-flags '("--strip-debug" "--enable-deterministic-archives")
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'unpack 'patch-stuff
-           (lambda* (#:key native-inputs inputs #:allow-other-keys)
-             (substitute* "grub-core/Makefile.in"
-               (("/bin/sh") (which "sh")))
-
-             ;; Give the absolute file name of 'mdadm', used to determine the
-             ;; root file system when it's a RAID device.  Failing to do that,
-             ;; 'grub-probe' silently fails if 'mdadm' is not in $PATH.
-             (when (assoc-ref inputs "mdadm")
-               (substitute* "grub-core/osdep/linux/getroot.c"
-                 (("argv\\[0\\] = \"mdadm\"")
-                  (string-append "argv[0] = \""
-                                 (assoc-ref inputs "mdadm")
-                                 "/sbin/mdadm\""))))
-
-             ;; Make the font visible.
-             (copy-file (assoc-ref (or native-inputs inputs)
-                                   "unifont")
-                        "unifont.bdf.gz")
-             (system* "gunzip" "unifont.bdf.gz")
-
-             ;; Give the absolute file name of 'ckbcomp'.
-             (substitute* "util/grub-kbdcomp.in"
-               (("^ckbcomp ")
-                (string-append
-                 (search-input-file inputs "/bin/ckbcomp")
-                 " ")))))
-         (add-after 'unpack 'set-freetype-variables
-           ;; These variables need to be set to the native versions of the
-           ;; dependencies because they are used to build programs which are
-           ;; executed during build time.
-           (lambda* (#:key native-inputs #:allow-other-keys)
-             (when (assoc-ref native-inputs "freetype")
-               (let ((freetype (assoc-ref native-inputs "freetype")))
-                 (setenv "BUILD_FREETYPE_LIBS"
-                         (string-append "-L" freetype
-                                        "/lib -lfreetype"))
-                 (setenv "BUILD_FREETYPE_CFLAGS"
-                         (string-append "-I" freetype
-                                        "/include/freetype2"))))))
-         (add-before 'check 'disable-flaky-test
-           (lambda _
-             ;; This test is unreliable. For more information, see:
-             ;; <https://bugs.gnu.org/26936>.
-             (substitute* "Makefile.in"
-               (("grub_cmd_date grub_cmd_set_date grub_cmd_sleep")
-                "grub_cmd_date grub_cmd_sleep"))))
-         (add-before 'check 'disable-pixel-perfect-test
-           (lambda _
-             ;; This test compares many screenshots rendered with an older
-             ;; Unifont (9.0.06) than that packaged in Guix.
-             (substitute* "Makefile.in"
-               (("test_unset grub_func_test")
-                "test_unset")))))
-       ;; Disable tests on ARM and AARCH64 platforms or when cross-compiling.
-       #:tests? ,(not (or (any (cute string-prefix? <> (or (%current-target-system)
-                                                           (%current-system)))
-                               '("arm" "aarch64"))
-                          (%current-target-system)))))
+     (list
+      #:configure-flags
+      ;; Counterintuitively, this *disables* a spurious Python dependency by
+      ;; calling the ‘true’ binary instead.  Python is only needed during
+      ;; bootstrapping (for genptl.py), not when building from a release.
+      ''("PYTHON=true")
+      ;; Grub fails to load modules stripped with --strip-unneeded.
+      #:strip-flags ''("--strip-debug" "--enable-deterministic-archives")
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'unpack 'add-missing-extra-deps
+            (lambda _
+              (call-with-output-file "grub-core/extra_deps.lst"
+                (lambda (port)
+                  (format port "depends bli part_gpt~%")))))
+          (add-after 'unpack 'patch-stuff
+            (lambda* (#:key native-inputs inputs #:allow-other-keys)
+              (substitute* "grub-core/Makefile.in"
+                (("/bin/sh") (which "sh")))
+
+              ;; Give the absolute file name of 'mdadm', used to determine the
+              ;; root file system when it's a RAID device.  Failing to do that,
+              ;; 'grub-probe' silently fails if 'mdadm' is not in $PATH.
+              (let ((mdadm (false-if-exception
+                            (search-input-file inputs "/sbin/mdadm"))))
+                (when mdadm
+                  (substitute* "grub-core/osdep/linux/getroot.c"
+                    (("argv\\[0\\] = \"mdadm\"")
+                     (string-append "argv[0] = \""
+                                    mdadm "\"")))))
+
+              ;; Make the font visible.
+              (copy-file (assoc-ref (or native-inputs inputs)
+                                    "unifont")
+                         "unifont.bdf.gz")
+              (system* "gunzip" "unifont.bdf.gz")
+
+              ;; Give the absolute file name of 'ckbcomp'.
+              (substitute* "util/grub-kbdcomp.in"
+                (("^ckbcomp ")
+                 (string-append
+                  (search-input-file inputs "/bin/ckbcomp")
+                  " ")))))
+          (add-after 'unpack 'set-freetype-variables
+            ;; These variables need to be set to the native versions of the
+            ;; dependencies because they are used to build programs which are
+            ;; executed during build time.
+            (lambda* (#:key native-inputs #:allow-other-keys)
+              (let ((freetype
+                     (false-if-exception
+                      (dirname
+                       (dirname
+                        (search-input-directory (or native-inputs inputs)
+                                                "/include/freetype2"))))))
+                (when freetype
+                  (setenv "BUILD_FREETYPE_LIBS"
+                          (string-append "-L" freetype
+                                         "/lib -lfreetype"))
+                  (setenv "BUILD_FREETYPE_CFLAGS"
+                          (string-append "-I" freetype
+                                         "/include/freetype2"))))))
+          (add-before 'check 'disable-flaky-test
+            (lambda _
+              (define* (skip-tests #:rest tests)
+                (for-each
+                 (lambda (s)
+                   (copy-file #$(program-file "skip-test.scm" #~(exit 77))
+                              s))
+                 tests))
+              ;; Rely on the built-in `command` instead of coreutils `which`.
+              (substitute*
+                  (cons*
+                   "tests/partmap_test.in"
+                   (map (lambda (s) (string-append "tests/" s "compress_test.in"))
+                        '("gz" "lzo" "xz")))
+                (("which")
+                 "command -v"))
+              (skip-tests
+               ;; This test is unreliable. For more information, see:
+               ;; <https://bugs.gnu.org/26936>.
+               "grub_cmd_set_date"
+               ;; This test compares many screenshots rendered with an older
+               ;; Unifont (9.0.06) than that packaged in Guix.
+               "grub_func_test"
+               ;; Theses tests require root, disable them for now.
+               "ext234_test" "squashfs_test" "iso9660_test" "hfsplus_test"
+               "ntfs_test" "reiserfs_test" "fat_test" "minixfs_test"
+               "xfs_test" "f2fs_test" "nilfs2_test" "romfs_test" "exfat_test"
+               "tar_test" "udf_test" "hfs_test" "jfs_test" "btrfs_test"
+               "zfs_test" "cpio_test" "luks1_test" "luks2_test"
+               "grub_cmd_cryptomount"))))
+      ;; Disable tests on ARM and AARCH64 platforms or when cross-compiling.
+      #:tests? (not (or (target-arm32?)
+                        (target-aarch64?)
+                        (%current-target-system)))))
     (inputs
      `(("gettext" ,gettext-minimal)
 
@@ -252,11 +271,14 @@ (define-public grub
 
        ;; Dependencies for the test suite.  The "real" QEMU is needed here,
        ;; because several targets are used.
+       ("gzip" ,gzip)
+       ("lzop" ,lzop)
        ("parted" ,parted)
        ,@(if (member (%current-system) (package-supported-systems qemu-minimal))
              `(("qemu" ,qemu-minimal))
              '())
-       ("xorriso" ,xorriso)))
+       ("xorriso" ,xorriso)
+       ("xz" ,xz)))
     (home-page "https://www.gnu.org/software/grub/")
     (synopsis "GRand Unified Boot loader")
     (description

base-commit: efdaa885b083e85bd380ca914addfcf32d0834f7
prerequisite-patch-id: 0804a38590177dfed12fd7a69a96c0bfed9b0991
prerequisite-patch-id: c55fab450e37b6202898b430c21597042e067d31
prerequisite-patch-id: 51b6fbcd8a513f1c644d0958811cf4731a0d8c5e
-- 
2.41.0

[PATCH 2/2] gnu: grub: Modernize.

[Josselin Poiret]

From: Josselin Poiret <dev@jpoiret.xyz>

* gnu/packages/bootloaders.scm (grub-minimal, grub-coreboot, grub-efi32, grub-hybrid): Use G-Exps.
(grub-efi): Use G-Exps.  Also use OVMF to test.

Change-Id: Ic9c73753004739d6027b8426eb46c114a6a31051
---
 gnu/packages/bootloaders.scm | 286 +++++++++++++++++++----------------
 1 file changed, 152 insertions(+), 134 deletions(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 1327055484..39b98b0876 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -307,20 +307,18 @@ (define-public grub-minimal
         '(list "PYTHON=true"))
        ((#:tests? _ #t)
         #f)
-       ((#:phases phases '%standard-phases)
-        `(modify-phases ,phases
-           (replace 'patch-stuff
-             (lambda* (#:key native-inputs inputs #:allow-other-keys)
-               (substitute* "grub-core/Makefile.in"
-                 (("/bin/sh") (which "sh")))
-
-               ;; Make the font visible.
-               (copy-file (assoc-ref (or native-inputs inputs)
-                                     "unifont")
-                          "unifont.bdf.gz")
-               (system* "gunzip" "unifont.bdf.gz")
-
-               #t))))))))
+       ((#:phases phases #~%standard-phases)
+        #~(modify-phases #$phases
+            (replace 'patch-stuff
+              (lambda* (#:key native-inputs inputs #:allow-other-keys)
+                (substitute* "grub-core/Makefile.in"
+                  (("/bin/sh") (which "sh")))
+
+                ;; Make the font visible.
+                (copy-file (assoc-ref (or native-inputs inputs)
+                                      "unifont")
+                           "unifont.bdf.gz")
+                (system* "gunzip" "unifont.bdf.gz")))))))))
 
 (define-public grub-coreboot
   (package
@@ -328,64 +326,64 @@ (define-public grub-coreboot
     (name "grub-coreboot")
     (synopsis "GRand Unified Boot loader (Coreboot payload version)")
     (arguments
-     `(,@(substitute-keyword-arguments (package-arguments grub)
-           ((#:phases phases '%standard-phases)
-            `(modify-phases ,phases
-               (add-before 'check 'disable-broken-tests
-                 (lambda _
-                   (setenv "DISABLE_HARD_ERRORS" "1")
-                   (setenv
-                    "XFAIL_TESTS"
-                    (string-join
-                     ;; TODO: All the tests below use grub shell
-                     ;; (tests/util/grub-shell.in), and here grub-shell uses
-                     ;; QEMU and a Coreboot image to run the tests. Since we
-                     ;; don't have a Coreboot package in Guix yet these tests
-                     ;; are disabled. See the Guix bug #64667 for more details
-                     ;; (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64667).
-                     (list
-                      "pata_test"
-                      "ahci_test"
-                      "uhci_test"
-                      "ehci_test"
-                      "example_grub_script_test"
-                      "ohci_test"
-                      "grub_script_eval"
-                      "grub_script_echo1"
-                      "grub_script_test"
-                      "grub_script_leading_whitespace"
-                      "grub_script_echo_keywords"
-                      "grub_script_vars1"
-                      "grub_script_for1"
-                      "grub_script_while1"
-                      "grub_script_if"
-                      "grub_script_comments"
-                      "grub_script_functions"
-                      "grub_script_continue"
-                      "grub_script_break"
-                      "grub_script_shift"
-                      "grub_script_blockarg"
-                      "grub_script_return"
-                      "grub_script_setparams"
-                      "grub_cmd_date"
-                      "grub_cmd_sleep"
-                      "grub_cmd_regexp"
-                      "grub_script_not"
-                      "grub_cmd_echo"
-                      "grub_script_expansion"
-                      "grub_script_gettext"
-                      "grub_script_escape_comma"
-                      "help_test"
-                      "grub_script_strcmp"
-                      "test_sha512sum"
-                      "grub_cmd_tr"
-                      "test_unset"
-                      "file_filter_test")
-                     " "))))))
-           ((#:configure-flags flags
-             ''())
-            `(cons* "--with-platform=coreboot"
-                    ,flags)))))))
+     (substitute-keyword-arguments (package-arguments grub)
+       ((#:phases phases #~%standard-phases)
+        #~(modify-phases #$phases
+            (add-before 'check 'disable-broken-tests
+              (lambda _
+                (setenv "DISABLE_HARD_ERRORS" "1")
+                (setenv
+                 "XFAIL_TESTS"
+                 (string-join
+                  ;; TODO: All the tests below use grub shell
+                  ;; (tests/util/grub-shell.in), and here grub-shell uses
+                  ;; QEMU and a Coreboot image to run the tests. Since we
+                  ;; don't have a Coreboot package in Guix yet these tests
+                  ;; are disabled. See the Guix bug #64667 for more details
+                  ;; (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64667).
+                  (list
+                   "pata_test"
+                   "ahci_test"
+                   "uhci_test"
+                   "ehci_test"
+                   "example_grub_script_test"
+                   "ohci_test"
+                   "grub_script_eval"
+                   "grub_script_echo1"
+                   "grub_script_test"
+                   "grub_script_leading_whitespace"
+                   "grub_script_echo_keywords"
+                   "grub_script_vars1"
+                   "grub_script_for1"
+                   "grub_script_while1"
+                   "grub_script_if"
+                   "grub_script_comments"
+                   "grub_script_functions"
+                   "grub_script_continue"
+                   "grub_script_break"
+                   "grub_script_shift"
+                   "grub_script_blockarg"
+                   "grub_script_return"
+                   "grub_script_setparams"
+                   "grub_cmd_date"
+                   "grub_cmd_sleep"
+                   "grub_cmd_regexp"
+                   "grub_script_not"
+                   "grub_cmd_echo"
+                   "grub_script_expansion"
+                   "grub_script_gettext"
+                   "grub_script_escape_comma"
+                   "help_test"
+                   "grub_script_strcmp"
+                   "test_sha512sum"
+                   "grub_cmd_tr"
+                   "test_unset"
+                   "file_filter_test")
+                  " "))))))
+       ((#:configure-flags flags
+         ''())
+        #~(cons* "--with-platform=coreboot"
+                 #$flags))))))
 
 (define-public grub-efi
   (package
@@ -396,43 +394,58 @@ (define-public grub-efi
      (modify-inputs (package-inputs grub)
        (prepend efibootmgr mtools)))
     (native-inputs
-     ;; The tests are skipped in this package so we remove some test dependencies.
      (modify-inputs (package-native-inputs grub)
-       (delete "parted" "qemu" "xorriso")))
+       (prepend (match (or (%current-target-system) (%current-system))
+                  ((? target-x86?) ovmf)
+                  ((? target-arm32?) ovmf-arm)
+                  ((? target-aarch64?) ovmf-aarch64)))))
     (arguments
-     `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
-       ;; https://github.com/tianocore/edk2/tree/master/OvmfPkg .
-       ;; Search for 'OVMF' in "tests/util/grub-shell.in".
-       ,@(substitute-keyword-arguments (package-arguments grub)
-           ((#:tests? _ #f) #f)
-           ((#:configure-flags flags ''())
-            `(cons* "--with-platform=efi"
-                    ,@(if (string-prefix? "x86_64"
-                                          (or (%current-target-system)
-                                              (%current-system)))
-                          '("--enable-stack-protector") ; EFI-only for now
-                          '())
-                    ,flags))
-           ((#:phases phases)
-            `(modify-phases ,phases
-               (add-after 'patch-stuff 'use-absolute-efibootmgr-path
-                 (lambda* (#:key inputs #:allow-other-keys)
-                   (substitute* "grub-core/osdep/unix/platform.c"
-                     (("efibootmgr")
-                      (search-input-file inputs
-                                         "/sbin/efibootmgr")))))
-               (add-after 'patch-stuff 'use-absolute-mtools-path
-                 (lambda* (#:key inputs #:allow-other-keys)
-                   (let ((mtools (assoc-ref inputs "mtools")))
-                     (substitute* "util/grub-mkrescue.c"
-                       (("\"mformat\"")
-                        (string-append "\"" mtools
-                                       "/bin/mformat\"")))
-                     (substitute* "util/grub-mkrescue.c"
-                       (("\"mcopy\"")
-                        (string-append "\"" mtools
-                                       "/bin/mcopy\"")))
-                     #t))))))))))
+     (substitute-keyword-arguments (package-arguments grub)
+       ((#:configure-flags flags ''())
+        `(cons* "--with-platform=efi"
+                ,@(if (string-prefix? "x86_64"
+                                      (or (%current-target-system)
+                                          (%current-system)))
+                      '("--enable-stack-protector") ; EFI-only for now
+                      '())
+                ,flags))
+       ((#:phases phases)
+        #~(modify-phases #$phases
+            (add-after 'patch-stuff 'use-absolute-efibootmgr-path
+              (lambda* (#:key inputs #:allow-other-keys)
+                (substitute* "grub-core/osdep/unix/platform.c"
+                  (("efibootmgr")
+                   (search-input-file inputs
+                                      "/sbin/efibootmgr")))))
+            (add-after 'patch-stuff 'use-abolute-ovmf-path
+              (lambda* (#:key inputs native-inputs #:allow-other-keys)
+                #$(match-let
+                      (((str . replacement)
+                        (match (or (%current-target-system) (%current-system))
+                          ((? target-x86-32?)
+                           '("OVMF-ia32.fd" . "ovmf_ia32.bin"))
+                          ((? target-x86-64?)
+                           '("OVMF.fd" . "ovmf_x64.bin"))
+                          ((? target-arm32?)
+                           '("/usr/share/ovmf-arm/QEMU_EFI.fd" . "ovmf_arm.bin"))
+                          ((? target-aarch64?)
+                           '("/usr/share/qemu-efi/QEMU_EFI.fd" . "ovmf_aarch64.bin")))))
+                    #~(substitute* "tests/util/grub-shell.in"
+                        ((#$str)
+                         (search-input-file
+                          (or native-inputs inputs)
+                          (string-append "/share/firmware/" #$replacement)))))))
+            (add-after 'patch-stuff 'use-absolute-mtools-path
+              (lambda* (#:key inputs #:allow-other-keys)
+                (let ((mformat (search-input-file inputs "/bin/mformat"))
+                      (mcopy (search-input-file inputs "/bin/mcopy")))
+                  (substitute* "util/grub-mkrescue.c"
+                    (("\"mformat\"")
+                     (string-append "\"" mformat "\"")))
+                  (substitute* "util/grub-mkrescue.c"
+                    (("\"mcopy\"")
+                     (string-append "\"" mcopy "\""))))))))))
+    (supported-systems '("i686-linux" "x86_64-linux" "armhf-linux" "aarch64-linux"))))
 
 (define-public grub-efi32
   (package
@@ -440,17 +453,24 @@ (define-public grub-efi32
     (name "grub-efi32")
     (synopsis "GRand Unified Boot loader (UEFI 32bit version)")
     (arguments
-     `(,@(substitute-keyword-arguments (package-arguments grub-efi)
-           ((#:configure-flags flags ''())
-            `(cons*
-              ,@(cond ((target-x86?) '("--target=i386"))
-                      ((target-aarch64?)
-                       (list "--target=arm"
-                             (string-append "TARGET_CC="
-                                            (cc-for-target "arm-linux-gnueabihf"))))
-                      ((target-arm?) '("--target=arm"))
-                      (else '()))
-              ,flags)))))
+     (substitute-keyword-arguments
+         (parameterize ((%current-target-system
+                         (match (or (%current-target-system) (%current-system))
+                           ((? target-x86?)
+                            "i686-linux")
+                           ((? target-arm?)
+                            "arm-linux-gnueabihf"))))
+           (package-arguments grub-efi))
+       ((#:configure-flags flags ''())
+        `(cons*
+          ,@(cond ((target-x86?) '("--target=i386"))
+                  ((target-aarch64?)
+                   (list "--target=arm"
+                         (string-append "TARGET_CC="
+                                        (cc-for-target "arm-linux-gnueabihf"))))
+                  ((target-arm?) '("--target=arm"))
+                  (else '()))
+          ,flags))))
     (native-inputs
      (if (target-aarch64?)
          (modify-inputs (package-native-inputs grub-efi)
@@ -476,22 +496,20 @@ (define-public grub-hybrid
      (substitute-keyword-arguments (package-arguments grub-efi)
        ((#:modules modules `((guix build utils) (guix build gnu-build-system)))
         `((ice-9 ftw) ,@modules))
-       ((#:phases phases)
-        `(modify-phases ,phases
-           (add-after 'install 'install-non-efi
-             (lambda* (#:key inputs outputs #:allow-other-keys)
-               (let ((input-dir (search-input-directory inputs
-                                                        "/lib/grub"))
-                     (output-dir (string-append (assoc-ref outputs "out")
-                                                "/lib/grub")))
-                 (for-each
-                  (lambda (basename)
-                    (if (not (or (string-prefix? "." basename)
-                                 (file-exists? (string-append output-dir "/" basename))))
-                        (symlink (string-append input-dir "/" basename)
-                                 (string-append output-dir "/" basename))))
-                  (scandir input-dir))
-                 #t)))))))))
+       ((#:phases phases #~%standard-phases)
+        #~(modify-phases #$phases
+            (add-after 'install 'install-non-efi
+              (lambda* (#:key inputs outputs #:allow-other-keys)
+                (let ((input-dir (search-input-directory inputs
+                                                         "/lib/grub"))
+                      (output-dir (string-append #$output "/lib/grub")))
+                  (for-each
+                   (lambda (basename)
+                     (if (not (or (string-prefix? "." basename)
+                                  (file-exists? (string-append output-dir "/" basename))))
+                         (symlink (string-append input-dir "/" basename)
+                                  (string-append output-dir "/" basename))))
+                   (scandir input-dir)))))))))))
 
 (define-public (make-grub-efi-netboot name subdir)
   "Make a grub-efi-netboot package named NAME, which will be able to boot over
-- 
2.41.0

Re: [PATCH 2/2] gnu: grub: Modernize.

[Fabio Natali]

On 2024-03-03, 17:42 +0100, Josselin Poiret <dev@jpoiret.xyz> wrote:
> From: Josselin Poiret <dev@jpoiret.xyz>
>
> * gnu/packages/bootloaders.scm (grub-minimal, grub-coreboot, grub-efi32, grub-hybrid): Use G-Exps.
> (grub-efi): Use G-Exps.  Also use OVMF to test.

Hi Josselin,

This is brilliant, thanks.

I'll see how far I can go with this and get back to you. I might reach
out to you on IRC, maybe, in case I need some guidance?

Thanks, cheers, Fabio.

Re: [PATCH 2/2] gnu: grub: Modernize.

[Josselin Poiret]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

Hi Fabio,

Fabio Natali <me@fabionatali.com> writes:

> On 2024-03-03, 17:42 +0100, Josselin Poiret <dev@jpoiret.xyz> wrote:
>> From: Josselin Poiret <dev@jpoiret.xyz>
>>
>> * gnu/packages/bootloaders.scm (grub-minimal, grub-coreboot, grub-efi32, grub-hybrid): Use G-Exps.
>> (grub-efi): Use G-Exps.  Also use OVMF to test.
>
> Hi Josselin,
>
> This is brilliant, thanks.
>
> I'll see how far I can go with this and get back to you. I might reach
> out to you on IRC, maybe, in case I need some guidance?
>
> Thanks, cheers, Fabio.

Of course, I'm known as jpoiret there.  I use a bouncer, so even if I'm
online I might not be available, but will definitely read your message
later if that's the case.

Best,
-- 
Josselin Poiret

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 682 bytes --]

Re: [PATCH 2/2] gnu: grub: Modernize.

[Fabio Natali]

On 2024-03-09, 10:42 +0100, Josselin Poiret <dev@jpoiret.xyz> wrote:
> Of course, I'm known as jpoiret there.

Hi Josselin,

Thanks for the patches 🙏, which I've applied and tested as follows.

./pre-inst-env guix build grub
./pre-inst-env guix lint grub
./pre-inst-env guix build --check grub

Everything passed successfully, both in the case of the first patch
alone and when using the two patches together. I made sure that the
package built was version 2.12 instead of 2.06.

When building a system image, the first patch gave me an error if used
by itself. The error was along the lines of:

ice-9/read.scm:126:4: In procedure read-expr*: Unknown # object: "#<"

I haven't investigated this further as the error resolved when applying
both patches together.

I then installed GRUB 2.12 on a spare x86 machine. With this new version
of GRUB, I was able to use a LUKS2 partition with PBKDF2
public-key-based key derivation function. Yay! 🚀

When building GRUB 2.12 on the spare machine this test initially failed:

https://git.savannah.gnu.org/cgit/grub.git/tree/tests/grub_cmd_date.in

The error vanished when I tried a second time and I haven't been able to
reproduce it since then.

The patches have two micro-typos:

- First patch, s/Theses/These/.
- Second patch, s/use-abolute-ovmf-path/use-absolute-ovmf-path/.

I haven't tried the patches on any non-x86 architecture, but will ping
you on IRC to see if and how I can help with that.

Thanks, best, Fabio.


-- 
Fabio Natali
https://fabionatali.com