[bug#68524] [PATCH v2 1/2] gnu: bootloaders: Add uki packages.

[Hilton Chain via Guix-patches via <guix-patches@gnu.org>]

Hi Lilah,

On Sun, 28 Jan 2024 17:51:40 +0800,
Lilah Tascheter via Guix-patches wrote:
>
> * gnu/packages/bootloaders.scm (systemd-stub-name): New procedure.
>   (systemd-version,systemd-source,systemd-stub,ukify): New variables.

First of all, please split this commit into two commits, each adding a single
package.
(Other comments are between quote blocks.)

> Change-Id: I67776ec35d165afebc2eb4b11bea0459259e4bd8
> ---
>  gnu/packages/bootloaders.scm | 95 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>
> diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
> index 986f0ac035..b0d4979f44 100644
> --- a/gnu/packages/bootloaders.scm
> +++ b/gnu/packages/bootloaders.scm
> @@ -19,6 +19,7 @@
>  ;;; Copyright © 2021 Stefan <stefan-guix@vodafonemail.de>
>  ;;; Copyright © 2022, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
>  ;;; Copyright © 2023 Herman Rimm <herman@rimm.ee>
> +;;; Copyright © 2024 Lilah Tascheter <lilah@lunabee.space>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -46,11 +47,13 @@ (define-module (gnu packages bootloaders)
>    #:use-module (gnu packages compression)
>    #:use-module (gnu packages cross-base)
>    #:use-module (gnu packages disk)
> +  #:use-module (gnu packages efi)
>    #:use-module (gnu packages firmware)
>    #:use-module (gnu packages flex)
>    #:use-module (gnu packages fontutils)
>    #:use-module (gnu packages gcc)
>    #:use-module (gnu packages gettext)
> +  #:use-module (gnu packages gperf)
>    #:use-module (gnu packages linux)
>    #:use-module (gnu packages man)
>    #:use-module (gnu packages mtools)
> @@ -71,11 +74,13 @@ (define-module (gnu packages bootloaders)
>    #:use-module (gnu packages valgrind)
>    #:use-module (gnu packages virtualization)
>    #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages python-crypto)
>    #:use-module (gnu packages python-web)
>    #:use-module (gnu packages python-xyz)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system meson)
>    #:use-module (guix build-system pyproject)
> +  #:use-module (guix build-system python)
>    #:use-module (guix build-system trivial)
>    #:use-module (guix download)
>    #:use-module (guix gexp)
> @@ -632,6 +637,96 @@ (define-public syslinux
>                       ;; Also contains:
>                       license:expat license:isc license:zlib)))))
>
> +(define systemd-version "255")
> +(define systemd-source
> +  (origin
> +    (method git-fetch)
> +    (uri (git-reference
> +           (url "https://github.com/systemd/systemd")
> +           (commit (string-append "v" systemd-version))))
> +    (file-name (git-file-name "systemd" systemd-version))
> +    (sha256
> +      (base32
> +        "1qdyw9g3jgvsbc1aryr11gpc3075w5pg00mqv4pyf3hwixxkwaq6"))))
> +
> +(define-public (systemd-stub-name)
> +  (let ((arch (cond ((target-x86-32?) "ia32")
> +                ((target-x86-64?) "x64")
> +                ((target-arm32?) "arm")
> +                ((target-aarch64?) "aa64")
> +                ((target-riscv64?) "riscv64"))))
> +    (string-append "linux" arch ".efi.stub")))

How about exporting this procedure in the module definition instead?

> +
> +(define-public systemd-stub
> +  (package
> +    (name "systemd-stub")
> +    (version systemd-version)
> +    (source systemd-source)
> +    (build-system meson-build-system)
> +    (arguments
> +      (list
> +        #:configure-flags
> +        `(list "-Defi=true" "-Dsbat-distro=guix"
> +               "-Dsbat-distro-generation=1" ; package revision!
> +               "-Dsbat-distro-summary=Guix System"
> +               "-Dsbat-distro-url=https://guix.gnu.org"
> +               ,(string-append "-Dsbat-distro-pkgname=" name)
> +               ,(string-append "-Dsbat-distro-version=" version))

Please use a G-expression for #:configure-flags, replace ‘name’ and ‘version’
to ‘#$(package-name this-package)’ and ‘#$(package-version this-package)’.

"-Dmode=release" can be added, too.

> +        #:phases
> +        #~(let ((stub #$(string-append "src/boot/efi/" (systemd-stub-name))))
> +            (modify-phases %standard-phases
> +              (replace 'build
> +                (lambda* (#:key parallel-build? #:allow-other-keys)
> +                  (invoke "ninja" stub
> +                    "-j" (if parallel-build?
> +                           (number->string (parallel-job-count)) "1"))))
> +              (replace 'install
> +                (lambda _
> +                  (install-file stub (string-append #$output "/libexec"))))
> +              (delete 'check)))))
> +    (inputs (list libcap python-pyelftools `(,util-linux "lib")))
> +    (native-inputs (list gperf pkg-config python-3 python-jinja2))
> +    (home-page "https://systemd.io")

I think its homepage has an ending slash, as in "https://systemd.io/".

> +    (synopsis "Unified kernel image UEFI stub")
> +    (description "Simple UEFi boot stub that loads a conjoined kernel image and
> +supporting data to their proper locations, before chainloading to the kernel.
> +Supports measured and/or verified boot environments.")
> +    (license license:lgpl2.1+)))
> +
> +(define-public ukify
> +  (package
> +    (name "ukify")
> +    (version systemd-version)
> +    (source systemd-source)
> +    (build-system python-build-system)
> +    (arguments
> +      (list #:phases
> +            #~(modify-phases %standard-phases
> +                (replace 'build
> +                  (lambda _
> +                    (substitute* "src/ukify/ukify.py" ; added in python 3.11
> +                      (("datetime\\.UTC") "datetime.timezone.utc"))))

It's likely that only ‘systemd-source’ will be touched in the future, so I'd
suggest moving this substitution into ‘systemd-source’ as a snippet.

> +                (delete 'check)
> +                (replace 'install
> +                  (lambda* (#:key inputs #:allow-other-keys)
> +                    (let* ((bin (string-append #$output "/bin"))
> +                           (file (string-append bin "/ukify"))
> +                           (binutils (assoc-ref inputs "binutils"))
> +                           (sbsign (assoc-ref inputs "sbsigntools")))

Getting inputs' path with ‘assoc-ref’ is not recommended.  ‘search-input-file’
or ‘this-package-input’ can be used instead.

> +                      (mkdir-p bin)
> +                      (copy-file "src/ukify/ukify.py" file)
> +                      (wrap-program file
> +                        `("PATH" ":" prefix
> +                          (,(string-append binutils "/bin")
> +                           ,(string-append sbsign "/bin"))))))))))

I'd suggest patching paths instead of wrapping programs when possible, for
example, I have made one when reviewing this patch:

--8<---------------cut here---------------start------------->8---
(replace 'install
  (lambda* (#:key inputs #:allow-other-keys)
    (let ((file (string-append #$output "/bin/ukify")))
      (mkdir-p (dirname file))
      (copy-file "src/ukify/ukify.py" file)
      (substitute* file
        (("(find_tool.'|'name': ')\\<(readelf|sbsign|sbverify)\\>"
          _ pre cmd)
         (string-append
          pre (search-input-file
               inputs (string-append "bin/" cmd))))))))
--8<---------------cut here---------------end--------------->8---

Note that one dependency, ‘pesign’, is currently missing from Guix, thus not
handled here.

I don't know if it has anything to do with our usage, but for the completeness
of the package, I think we can package this dependency, or adding a comment
around the ‘inputs’ field to indicate it's missing.

> +    (inputs (list binutils python-cryptography python-pefile sbsigntools))
> +    (home-page "https://systemd.io")

Same as the homepage mentioned above.

> +    (synopsis "Unified kernel image UEFI tool")
> +    (description "@command{ukify} joins together a UKI stub, linux kernel, initrd,
> +kernel arguments, and optional secure boot signatures into a single, UEFI-bootable
> +image.")
> +    (license license:lgpl2.1+)))
> +
>  (define-public dtc
>    (package
>      (name "dtc")
> --
> 2.41.0

Thanks