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

[Alyssa Ross <hi@alyssa.is>]

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

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

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

Yes, perhaps worth considering.  I'll think about it.

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

Even so it's very short.

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

Uh, "packages" should go in pkgs/default.nix.  I'd need to remember the
rationale for rootfs being in there to say more, and I don't right now.

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

So this doesn't work?

	$(MCOPY) -i $@ $(EFI_IMAGE) ::/EFI/Linux/spectrum.efi

I'd be very curious to see the diff between that and a working image.

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

You'd end up doing a lot of rebuilding for everything that depends on
systemd.  We could also have it so that Spectrum's "systemd" in
pkgs/default.nix differs from the systemd used inside Nixpkgs and
available as pkgs.systemd, but that would get /extremely/ confusing.

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