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

[Alyssa Ross <hi@alyssa.is>]

[-- 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 --]