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

[Demi Marie Obenour <demiobenour@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 7416 bytes --]

On 11/13/25 06:57, Alyssa Ross wrote:
> 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.)

I think it would be best to relicense the Makefiles under MIT if we can,
so that we can move code back and forth even after neither of us knows every
single copyright holder.  Feel free to relicense my contributions to them.

>> +# 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?

Will remove.

>> +  __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.

Done, except for DTBS which is used more than once.

>> 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).

What is the general rule for what should go in pkgs/default.nix?
If you could add it to the docs that would be great.

>> 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?

The basename of the path is actually important.  I tried using
$(EFI_IMAGE) and the system didn't boot.

>> 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?

Will change in v4.

>> -    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.

Would it be better to have the override in a Spectrum-wide overlay?

>> +    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
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]