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

Alyssa Ross
2025-11-25T12:41:30Z

[-- Attachment #1: Type: text/plain, Size: 7132 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>
> ---
> I kept release/live/default.nix using the UKI's systemd because the old
> code did it that way. Changing this would be better in a separate
> commit.
> ---
> host/efi.nix | 40 ++++++++++++++++++++++++++++++++++++++++
> host/rootfs/Makefile | 8 ++++----
> release/live/Makefile | 16 ++--------------
> release/live/default.nix | 27 +++++++++++----------------
> release/live/shell.nix | 10 ++++++++--
> 5 files changed, 65 insertions(+), 36 deletions(-)

Looking good. Just some style notes.

> diff --git a/host/rootfs/Makefile b/host/rootfs/Makefile
> index 5e3c9238f0e00f86aa5943212b8fc8fd896ce54a..aac915ffb2781aee0997c169e86e3fd1983aa3b3 100644
> --- a/host/rootfs/Makefile
> +++ b/host/rootfs/Makefile
> @@ -40,6 +40,10 @@ FIFOS = etc/s6-linux-init/run-image/service/s6-svscan-log/fifo
>
> BUILD_FILES = build/etc/s6-rc
>
> +# This rule produces three files but Make only (portably)
> +# supports one output per rule. Instead of resorting to temporary
> +# files, a timestamp file is created as the last step. The actual
> +# outputs are produced as side-effects.
> build/verity-timestamp: $(ROOT_FS)
> $(VERITYSETUP) format \
> --root-hash-file $(ROOT_FS_VERITY_ROOTHASH) \
> @@ -48,10 +52,6 @@ build/verity-timestamp: $(ROOT_FS)
> echo >> $(ROOT_FS_VERITY_ROOTHASH)
> touch -- $(ROOT_FS_DIR)/verity-timestamp
>
> -# This rule produces three files but Make only (portably)
> -# supports one output per rule. Instead of resorting to temporary
> -# files, a timestamp file is created as the last step. The actual
> -# outputs are produced as side-effects.
> $(ROOT_FS): ../../scripts/make-erofs.sh $(PACKAGES_FILE) $(FILES) $(BUILD_FILES) build/empty build/fifo file-list.mk
> mkdir -p $(ROOT_FS_DIR) && \
> { \
> diff --git a/release/live/Makefile b/release/live/Makefile
> index 7372b41d94bfb10f7761955d9d1a246e9785b7f8..d61248e94599adc5229d0ad38d54b9f649d66ca1 100644
> --- a/release/live/Makefile
> +++ b/release/live/Makefile
> @@ -19,23 +19,11 @@ $(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

I'd call "EFI_IMAGE" "SPECTRUM_EFI", so we aren't using two different
naming schemes for the two different EFI executables.

> $(TRUNCATE) -s 440401920 $@
> $(MKFS_FAT) $@
> $(MMD) -i $@ ::/EFI ::/EFI/BOOT ::/EFI/Linux
> - $(MCOPY) -i $@ build/spectrum.efi ::/EFI/Linux
> + $(MCOPY) -i $@ $(EFI_IMAGE) ::/EFI/Linux/spectrum.efi
> $(MCOPY) -i $@ $(SYSTEMD_BOOT_EFI) ::/EFI/BOOT/$(EFINAME)
>
> clean:
> diff --git a/release/live/default.nix b/release/live/default.nix
> index 7adaefef330daf11372cff0d2d04cca400efba1f..ac2d7a55fd4fe0c02108309ecea20e368000af0d 100644
> --- a/release/live/default.nix
> +++ b/release/live/default.nix
> @@ -3,10 +3,9 @@
> # SPDX-FileCopyrightText: 2022 Unikie
>
> import ../../lib/call-package.nix (
> -{ callSpectrumPackage, spectrum-build-tools, rootfs, src
> +{ callSpectrumPackage, spectrum-build-tools, src
> , lib, pkgsStatic, stdenvNoCC
> , cryptsetup, dosfstools, jq, mtools, util-linux
> -, systemdUkify
> }:
>
> let
> @@ -14,14 +13,12 @@ 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;
> +
> + efi = callSpectrumPackage ../../host/efi.nix {};
> +
> + # The initramfs and rootfs must match those used to build the UKI.
> + inherit (efi) initramfs rootfs systemd;
> in
>
> stdenv.mkDerivation {
> @@ -40,17 +37,15 @@ 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_DIR = rootfs;
> + KERNEL = "${efi.rootfs.kernel}/${stdenv.hostPlatform.linux-kernel.target}";
> + ROOT_FS_DIR = "${efi.rootfs}";

Why inherit these from efi above if you're going to refer to them
through efi here anyway?

> SYSTEMD_BOOT_EFI = "${systemd}/lib/systemd/boot/efi/systemd-boot${efiArch}.efi";
> + EFI_IMAGE = efi;
> EFINAME = "BOOT${toUpper efiArch}.EFI";
> - } // lib.optionalAttrs stdenv.hostPlatform.linux-kernel.DTB or false {
> - DTBS = "${rootfs.kernel}/dtbs";
> };
>
> buildFlags = [ "dest=$(out)" ];
> @@ -63,6 +58,6 @@ stdenv.mkDerivation {
> unsafeDiscardReferences = { out = true; };
> dontFixup = true;
>
> - passthru = { inherit initramfs rootfs; };
> + passthru = { inherit efi initramfs rootfs; };
> }
> ) (_: {})
> diff --git a/release/live/shell.nix b/release/live/shell.nix
> index c5db7b732ef048b4c0cb87a4c5ea614e993db516..ffaa9a571c662810348822a5952d479d251a25e5 100644
> --- a/release/live/shell.nix
> +++ b/release/live/shell.nix
> @@ -1,7 +1,12 @@
> # SPDX-License-Identifier: MIT
> # SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>
>
> -import ../../lib/call-package.nix ({ callSpectrumPackage, stdenv, qemu_kvm, rootfs }:
> +import ../../lib/call-package.nix (
> +{ callSpectrumPackage, stdenv, qemu_kvm }:

This has reduced in length, so it doesn't need to be broken on to a
separate line.

> +
> +let
> + efi = callSpectrumPackage ../../host/efi.nix {};
> +in
>
> (callSpectrumPackage ./. {}).overrideAttrs (
> { nativeBuildInputs ? [], env ? {}, ... }:
> @@ -10,7 +15,8 @@ import ../../lib/call-package.nix ({ callSpectrumPackage, stdenv, qemu_kvm, root
>
> env = env // {
> OVMF_CODE = "${qemu_kvm}/share/qemu/edk2-${stdenv.hostPlatform.qemuArch}-code.fd";
> - ROOT_FS_DIR = rootfs;
> + ROOT_FS_DIR = efi.rootfs;
> + EFI_IMAGE = efi;
> };
> }
> )) (_: {})
>
> --
> 2.52.0

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