Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/grub/grub.spec
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ BuildRequires: automake
BuildRequires: bison
BuildRequires: flex
BuildRequires: gettext-devel
Provides: %{_cross_os}bootloader(efi)
Conflicts: %{_cross_os}image-feature(uki-image)

%description
%{summary}.
Expand Down

This file was deleted.

2 changes: 0 additions & 2 deletions packages/kernel-6.1/kernel-6.1.spec
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ Patch1004: 1004-af_unix-increase-default-max_dgram_qlen-to-512.patch
Patch1005: 1005-Revert-Revert-drm-fb_helper-improve-CONFIG_FB-depend.patch
# Backport patch to ensure NUL-terminated task->comm buffer
Patch1006: 1006-strscpy-write-destination-buffer-only-once.patch
# Disable incomplete measurement into PCR 9 on aarch64.
Patch1007: 1007-efi-libstub-don-t-measure-kernel-command-line-into-P.patch

BuildRequires: bc
BuildRequires: elfutils-devel
Expand Down

This file was deleted.

2 changes: 0 additions & 2 deletions packages/kernel-6.12/kernel-6.12.spec
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ Patch1005: 1005-Lustre-cast-unsigned-long-to-pointer.patch
Patch1006: 1006-Select-prerequisites-for-gpu-drivers.patch
# Backport patch to ensure NUL-terminated task->comm buffer
Patch1007: 1007-strscpy-write-destination-buffer-only-once.patch
# Disable incomplete measurement into PCR 9 on aarch64.
Patch1008: 1008-efi-libstub-don-t-measure-kernel-command-line-into-P.patch
Patch1009: 1009-Revert-selinux-fix-overlayfs-mmap-and-mprotect-acces.patch

BuildRequires: bc
Expand Down

This file was deleted.

2 changes: 0 additions & 2 deletions packages/kernel-6.18/kernel-6.18.spec
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ Patch1003: 1003-initramfs-unlink-INITRAMFS_FORCE-from-CMDLINE_-EXTEN.patch
Patch1004: 1004-af_unix-increase-default-max_dgram_qlen-to-512.patch
# Select prerequisites for GPU drivers.
Patch1005: 1005-drm-simpledrm-Select-prerequisites-for-gpu-drivers.patch
# Disable incomplete measurement into PCR 9 on aarch64.
Patch1006: 1006-efi-libstub-don-t-measure-kernel-command-line-into-P.patch
Patch1007: 1007-Revert-selinux-fix-overlayfs-mmap-and-mprotect-acces.patch
# Fix incorrect FIPS crypto module path for x86_64 builds.
Patch1008: 1008-kbuild-use-SRCARCH-in-crypto-module-folders.patch
Expand Down
64 changes: 54 additions & 10 deletions packages/shim/shim.spec
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
%global __strip %{_bindir}/true

%global efidir /boot/efi/EFI/BOOT
%global boot_efi_image boot%{_cross_efi_arch}.efi
%global grub_efi_image grub%{_cross_efi_arch}.efi
%global systemd_boot_efi_image systemd-boot%{_cross_efi_arch}.efi
%global shim_systemd_boot_efi_image shim-systemd-boot%{_cross_efi_arch}.efi
Comment on lines +6 to +7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you could interpolate these variables e.g.

%global shim_systemd_boot_efi_image shim-%{systemd_boot_efi_image}

%global shim_efi_image shim%{_cross_efi_arch}.efi
%global mokm_efi_image mm%{_cross_efi_arch}.efi

%global boot_efi_image boot%{_cross_efi_arch}.efi

%global shimver 16.0
%global commit 18d98bfb34be583a5fe2987542e4b15e0db9cb61

Expand All @@ -20,9 +23,32 @@ Source0: https://github.com/rhboot/shim/releases/download/%{shimver}/shim-%{shim
Source1: https://github.com/rhboot/shim/releases/download/%{shimver}/shim-%{shimver}.tar.bz2.asc
Source2: gpgkey-8107B101A432AAC9FE8E547CA348D61BC2713E9F.asc

Requires: %{name}(shim-efi)

%description
%{summary}.

%package grub
Summary: Shim built to chain-load GRUB
# Avoid explicit image-feature(no-uki-image) requires for backwards compatibility
# The conflict is enough for now to prevent installing GRUB when UKIs are used
Comment on lines +33 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Avoid explicit image-feature(no-uki-image) requires for backwards compatibility
# The conflict is enough for now to prevent installing GRUB when UKIs are used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually wrote this one comment. Usually, we do the strong Requires and Conflicts, but in this case I avoided the Requires for a reason - don't require downstreams to bump their twoliter version if they take this kernel kit.

Requires: %{name}
Provides: %{name}(shim-efi) = 1:
Conflicts: %{_cross_os}image-feature(uki-image)

%description grub
%{summary}.

%package systemd-boot
Summary: Shim built to chain-load systemd-boot
Requires: %{name}
Requires: %{_cross_os}image-feature(uki-image)
Provides: %{name}(shim-efi) = 0:
Comment on lines +36 to +46

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Epoch-only Provides: %{name}(shim-efi) = N: is non-idiomatic and not load-bearing

Both subpackages declare a versioned virtual provide with only an Epoch and an empty version string:

  • shim-grub: Provides: %{name}(shim-efi) = 1: (line 36)
  • shim-systemd-boot: Provides: %{name}(shim-efi) = 0: (line 46)

The base package has an unversioned Requires: %{name}(shim-efi) (line 26).

The normal RPM syntax is NAME = [EPOCH:]VERSION[-RELEASE]. = 1: with an empty version is parseable by rpm (Epoch=1, Version="") but:

  1. The unversioned Requires in the base does not consider the Epoch at all — either subpackage satisfies it. So the Epoch never participates in dependency resolution for this Requires.
  2. Subpackage selection is actually driven by the Conflicts/Requires: image-feature(...) rules, not by the Epoch. The epoch-only versioning implies a tie-break mechanism that isn't actually load-bearing.
  3. If in the future both subpackages become eligible under some variant, the well-formedness of = 1: vs = 0: becomes depsolver-dependent.

Suggested fix: either drop the versioned Provides entirely and rely on the image-feature Conflicts/Requires (which are what really determine selection today), or make it well-formed and document the intent, e.g.:

Provides: %{name}(shim-efi) = 1:%{version}-%{release}
...
Provides: %{name}(shim-efi) = 0:%{version}-%{release}

with a comment explaining it is a tie-breaker for the shim-grub default.

Confidence: ~70%.

Conflicts: %{_cross_os}image-feature(no-uki-image)

%description systemd-boot
%{summary}.
Comment on lines +31 to +50

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] shim-grub and shim-systemd-boot have no explicit mutual Conflicts

The two subpackages are designed to be mutually exclusive, but nothing in the spec directly prevents co-installation. Mutual exclusion is only enforced transitively through the image-feature(...) Requires/Conflicts:

  • shim-grub Conflicts image-feature(uki-image) (line 37)
  • shim-systemd-boot Requires image-feature(uki-image) (line 45) and Conflicts image-feature(no-uki-image) (line 47)

The two subpackages install into disjoint paths (bootXX.efi vs shim-systemd-bootXX.efi), so RPM sees no file conflict either.

Scenario: a variant recipe that lists both Requires: shim-grub and Requires: shim-systemd-boot (dual-boot image or copy/paste error) will resolve without any RPM-level signal. The resulting image ships both loaders and boot behavior is then only determined by whatever bootXX.efi the firmware/UKI configuration points at.

If the intent really is "exactly one", encoding it in the spec is stronger than delegating it to image-feature discipline in downstream recipes.

Suggested fix: add mutual Conflicts to make the invariant explicit:

%package grub
...
Conflicts: %{name}-systemd-boot
Conflicts: %{_cross_os}image-feature(uki-image)

%package systemd-boot
...
Conflicts: %{name}-grub
Conflicts: %{_cross_os}image-feature(no-uki-image)

Alternatively, document in a comment that mutual exclusion is intentionally delegated to image-feature declarations upstream of this spec.

Confidence: ~55%.


%prep
%{gpgverify} --data=%{S:0} --signature=%{S:1} --keyring=%{S:2}
%autosetup -n shim-%{shimver} -p1
Expand All @@ -38,7 +64,6 @@ make\\\
CROSS_COMPILE="%{_cross_target}-"\\\
COMMIT_ID="%{commit}"\\\
RELEASE="%{release}"\\\
DEFAULT_LOADER="%{grub_efi_image}"\\\
DISABLE_REMOVABLE_LOAD_OPTIONS=y\\\
DESTDIR="%{buildroot}"\\\
EFIDIR="BOOT"\\\
Expand All @@ -47,20 +72,39 @@ make\\\
%{nil}

%build
%shim_make
# Build shim twice using separate source-tree copies. Building in-tree ensures
# DEFAULT_LOADER propagates correctly to the preprocessor defines.
cp -a %{_builddir}/shim-%{shimver} %{_builddir}/build-grub
cp -a %{_builddir}/shim-%{shimver} %{_builddir}/build-systemd-boot

cd %{_builddir}/build-grub
%shim_make DEFAULT_LOADER="%{grub_efi_image}"

cd %{_builddir}/build-systemd-boot
%shim_make DEFAULT_LOADER="%{systemd_boot_efi_image}"

%install
%shim_make install-as-data
install -d %{buildroot}%{efidir}
find %{buildroot}%{_datadir} -name '%{shim_efi_image}' -exec \
mv {} "%{buildroot}%{efidir}/%{boot_efi_image}" \;
find %{buildroot}%{_datadir} -name '%{mokm_efi_image}' -exec \
mv {} "%{buildroot}%{efidir}/%{mokm_efi_image}" \;
rm -rf %{buildroot}%{_datadir}

# Install grub-chaining shim at the default boot path, for backwards
# compatibility with image builds that expect to find it there.
find %{_builddir}/build-grub -name '%{shim_efi_image}' -exec \
cp {} "%{buildroot}%{efidir}/%{boot_efi_image}" \;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason these all changed from mv to cp? And no longer rm -rf %{buildroot}%{_datadir} ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm -f is no longer needed because we don't do the direct install anymore. re cp mv, nothing in particular, I can revert back if you feel strongly about this.


find %{_builddir}/build-systemd-boot -name '%{shim_efi_image}' -exec \
cp {} "%{buildroot}%{efidir}/%{shim_systemd_boot_efi_image}" \;

find %{_builddir}/build-grub -name '%{mokm_efi_image}' -exec \
cp {} "%{buildroot}%{efidir}/%{mokm_efi_image}" \;
Comment on lines +97 to +98

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] MokManager is copied only from build-grub but shipped in the base package

%install copies mm*.efi exclusively out of the build-grub tree, and the resulting file is packaged in the base %files list (line 104), which is installed alongside both subpackages.

Concerns:

  1. Asymmetric build coupling. MokManager doesn't depend on DEFAULT_LOADER, so both build trees produce it. Sourcing it only from build-grub couples the base package to that specific build. If a follow-up PR conditionally disables the grub build for UKI-only variants (e.g., wraps lines 77, 80-81 in %if !%{uki_only}), the base %files list still references %{efidir}/%{mokm_efi_image} and the build breaks non-obviously.
  2. find … -exec cp doesn't signal 'no match'. find returns 0 whether or not it matched. If the grub build silently stops producing mm*.efi in the future, the failure surfaces later at %files ("File not found") rather than at the copy step.

Suggested fix: either copy mm*.efi from build-systemd-boot (identical bits, decoupled from the grub build) or add a brief comment stating that MokManager is loader-independent and either build tree is acceptable, e.g.:

# MokManager is not affected by DEFAULT_LOADER; either build tree is fine.
find %{_builddir}/build-grub -name '%{mokm_efi_image}' -exec \
  cp {} "%{buildroot}%{efidir}/%{mokm_efi_image}" \;

Confidence: ~60%.


%files
%license COPYRIGHT
%{_cross_attribution_file}
%dir %{efidir}
%{efidir}/%{boot_efi_image}
%{efidir}/%{mokm_efi_image}

%files grub
%{efidir}/%{boot_efi_image}

%files systemd-boot
%{efidir}/%{shim_systemd_boot_efi_image}
Loading