Re: [PATCH v3 2/2] Move UKI creation to a separate derivation

Alyssa Ross
2025-11-13T11:58:00Z

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

Demi Marie Obenour <demiobenour@gmail.com> writes:

> It will be used by the update code later.
>
> No functional change intended, other than a trivial shell script
> refactoring.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
> host/efi.nix | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> pkgs/default.nix | 1 +
> release/live/Makefile | 15 ++-------------
> release/live/default.nix | 19 +++++--------------
> 4 files changed, 54 insertions(+), 27 deletions(-)
>
> diff --git a/host/efi.nix b/host/efi.nix
> new file mode 100644
> index 0000000000000000000000000000000000000000..a2b47fd050fbf00050473a0d5a1373eb96c341b5
> --- /dev/null
> +++ b/host/efi.nix
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: EUPL-1.2+

MIT for Nix files please. (Fine to take my stuff from the EUPL-1.2+
Makefile and use it in a MIT-licensed Nix file.)

> +# SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +import ../lib/call-package.nix (
> +{ bash, callSpectrumPackage, cryptsetup, runCommand
> +, stdenv, systemdUkify, rootfs
> +}:
> +let
> + initramfs = callSpectrumPackage ./initramfs {};
> + kernel = "${rootfs.kernel}/${stdenv.hostPlatform.linux-kernel.target}";
> + systemd = systemdUkify.overrideAttrs ({ mesonFlags ? [], ... }: {
> + # The default limit is too low to build a generic aarch64 distro image:
> + # https://github.com/systemd/systemd/pull/37417
> + mesonFlags = mesonFlags ++ [ "-Defi-stub-extra-sections=3000" ];
> + });
> +in
> +
> +runCommand "spectrum-efi" {
> + nativeBuildInputs = [ cryptsetup systemd bash ];

bash?

> + __structuredAttrs = true;
> + unsafeDiscardReferences = { out = true; };
> + dontFixup = true;
> + passthru = { inherit systemd; };
> + env = {
> + DTBS = "${rootfs.kernel}/dtbs";
> + KERNEL = kernel;
> + INITRAMFS = initramfs;
> + ROOTFS = rootfs;
> + };

Usually we'd just inline these via string interpolation, rather than
passing them through as environment variables.

> diff --git a/pkgs/default.nix b/pkgs/default.nix
> index cc60228a10cddcb70e5ab9faa1bab7d74f3ebb35..c9f6dcfad9369567468b30d1c5697e3551a7b236 100644
> --- a/pkgs/default.nix
> +++ b/pkgs/default.nix
> @@ -36,6 +36,7 @@ let
> path: (import path { inherit (self) callPackage; }).override;
>
> rootfs = self.callSpectrumPackage ../host/rootfs {};
> + efi = self.callSpectrumPackage ../host/efi.nix {};
> spectrum-build-tools = self.callSpectrumPackage ../tools {
> appSupport = false;
> buildSupport = true;

Generally images don't need entries here, and can just be loaded by
callSpectrumPackage. There was a specific reason to make an exception
for rootfs (which I've now forgotten).

> diff --git a/release/live/Makefile b/release/live/Makefile
> index 191b44944af0adf965e1d5f2785719b236bfd99c..4de8743f42dec65aa863c3020cd70124316a6118 100644
> --- a/release/live/Makefile
> +++ b/release/live/Makefile
> @@ -19,19 +19,8 @@ $(dest): ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sf
> build/empty:
> mkdir -p $@
>
> -build/spectrum.efi: $(DTBS) $(KERNEL) $(INITRAMFS) $(ROOT_FS_VERITY_ROOTHASH)
> - { \
> - printf "[UKI]\nDeviceTreeAuto=" && \
> - find $(DTBS) -name '*.dtb' -print0 | tr '\0' ' ' ;\
> - } | $(UKIFY) build \
> - --output $@ \
> - --config /dev/stdin \
> - --linux $(KERNEL) \
> - --initrd $(INITRAMFS) \
> - --os-release $$'NAME="Spectrum"\n' \
> - --cmdline "ro intel_iommu=on roothash=$$(cat "$$ROOT_FS_VERITY_ROOTHASH")"
> -
> -build/boot.fat: $(SYSTEMD_BOOT_EFI) build/spectrum.efi
> +build/boot.fat: $(SYSTEMD_BOOT_EFI) $(EFI_IMAGE) build/empty
> + ln -sf -- "$$EFI_IMAGE" build/spectrum.efi
> $(TRUNCATE) -s 440401920 $@
> $(MKFS_FAT) $@
> $(MMD) -i $@ ::/EFI ::/EFI/BOOT ::/EFI/Linux

Why a symlink? Why not just replace the path we copy from?

> diff --git a/release/live/default.nix b/release/live/default.nix
> index 9a62d4da9cfea11d94d2a1d5764d41587efd5ad5..c234d87e62cc9ae65ba60f94bab6e58b43beddbc 100644
> --- a/release/live/default.nix
> +++ b/release/live/default.nix
> @@ -6,7 +6,7 @@ import ../../lib/call-package.nix (
> { callSpectrumPackage, spectrum-build-tools, rootfs, src
> , lib, pkgsStatic, stdenvNoCC
> , cryptsetup, dosfstools, jq, mtools, util-linux
> -, systemdUkify
> +, systemdUkify, efi
> }:
>
> let
> @@ -14,13 +14,6 @@ let
>
> stdenv = stdenvNoCC;
>
> - systemd = systemdUkify.overrideAttrs ({ mesonFlags ? [], ... }: {
> - # The default limit is too low to build a generic aarch64 distro image:
> - # https://github.com/systemd/systemd/pull/37417
> - mesonFlags = mesonFlags ++ [ "-Defi-stub-extra-sections=3000" ];
> - });
> -
> - initramfs = callSpectrumPackage ../../host/initramfs {};
> efiArch = stdenv.hostPlatform.efiArch;
> in
>
> @@ -40,19 +33,17 @@ stdenv.mkDerivation {
> sourceRoot = "source/release/live";
>
> nativeBuildInputs = [
> - cryptsetup dosfstools jq spectrum-build-tools mtools systemd util-linux
> + cryptsetup dosfstools jq spectrum-build-tools mtools util-linux
> ];
>
> env = {
> - INITRAMFS = initramfs;
> KERNEL = "${rootfs.kernel}/${stdenv.hostPlatform.linux-kernel.target}";
> ROOT_FS = "${rootfs}/rootfs";
> ROOT_FS_VERITY = "${rootfs}/rootfs.verity.superblock";
> ROOT_FS_VERITY_ROOTHASH = "${rootfs}/rootfs.verity.roothash";

Since efi is tied to a specific rootfs, maybe it would be nice to use
efi.rootfs here?

> - SYSTEMD_BOOT_EFI = "${systemd}/lib/systemd/boot/efi/systemd-boot${efiArch}.efi";
> + SYSTEMD_BOOT_EFI = "${efi.systemd}/lib/systemd/boot/efi/systemd-boot${efiArch}.efi";

We can just get this from the default systemd package. Doesn't need to
be efi's special overridden one.

> + EFI_IMAGE = efi;
> EFINAME = "BOOT${toUpper efiArch}.EFI";
> - } // lib.optionalAttrs stdenv.hostPlatform.linux-kernel.DTB or false {
> - DTBS = "${rootfs.kernel}/dtbs";
> };
>
> buildFlags = [ "dest=$(out)" ];
> @@ -65,6 +56,6 @@ stdenv.mkDerivation {
> unsafeDiscardReferences = { out = true; };
> dontFixup = true;
>
> - passthru = { inherit initramfs rootfs; };
> + passthru = { inherit rootfs; };
> }
> ) (_: {})
>
> --
> 2.51.2

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