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

Alyssa Ross
2025-11-14T11:58:37Z

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