Skip to content

efi: Choose update layout depending on /usr/lib/efi #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Johan-Liebert1 merged 2 commits into from
Apr 29, 2026
Merged

efi: Choose update layout depending on /usr/lib/efi #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d60084
efi: Choose update layout depending on /usr/lib/efi
Johan-Liebert1 Apr 23, 2026
fbb5911
tests: Update tests for new and old layouts
Johan-Liebert1 Apr 24, 2026
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
165 changes: 89 additions & 76 deletions src/efi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,50 +568,49 @@ impl Component for Efi {
}

fn generate_update_metadata(&self, sysroot: &str) -> Result<Option<ContentMetadata>> {
let sysroot_path = Path::new(sysroot);
let sysroot_path = Utf8Path::new(sysroot);
let sysroot_dir = Dir::open_ambient_dir(sysroot_path, cap_std::ambient_authority())?;

if let Some(ostreeboot) = sysroot_dir
let ostreeboot = sysroot_dir
.open_dir_optional(ostreeutil::BOOT_PREFIX)
.context("Opening usr/lib/ostree-boot")?
{
.context("Opening usr/lib/ostree-boot")?;

// Newer images >= F44 have boot entries in /usr/lib/efi, but older images
// have them in /usr/lib/ostree-boot, which we move to /usr/lib/bootupd/updates/EFI
let metadata = if sysroot_path.join(EFILIB).exists() {
println!("Generating metadata from {EFILIB}");
generate_meta_from_usr_efi(sysroot_path)?
} else {
match &ostreeboot {
Some(..) => {
println!(
"Moving {}/efi/EFI to {BOOTUPD_UPDATES_DIR}/EFI",
ostreeutil::BOOT_PREFIX
);
// Transfer usr/lib/ostree-boot/EFI files to usr/lib/bootupd/updates/EFI
transfer_ostree_boot_to_bootupd_updates(sysroot_path, self)?
}

// If /usr/lib/ostree-boot doesn't exist, assume /usr/lib/efi will
None => anyhow::bail!("/usr/lib/ostree-boot and /usr/lib/efi not found"),
}
};

if let Some(ostreeboot) = ostreeboot {
let cruft = ["loader", "grub2"];
for p in cruft.iter() {
println!("Removing {p} from {}", ostreeutil::BOOT_PREFIX);
ostreeboot.remove_all_optional(p)?;
}
// Transfer ostree-boot EFI files to usr/lib/efi
transfer_ostree_boot_to_usr(sysroot_path)?;

println!("Removing efi/EFI from {}", ostreeutil::BOOT_PREFIX);
// Remove usr/lib/ostree-boot/efi/EFI dir (after transfer) or if it is empty
ostreeboot.remove_all_optional("efi/EFI")?;
}
};

if let Some(efi_components) =
get_efi_component_from_usr(Utf8Path::from_path(sysroot_path).unwrap(), EFILIB)?
{
let mut packages = Vec::new();
let mut modules_vec: Vec<Module> = vec![];
for efi in efi_components {
packages.push(format!("{}-{}", efi.name, efi.version));
modules_vec.push(Module {
name: efi.name,
rpm_evr: efi.version,
});
}
modules_vec.sort_unstable();

// change to now to workaround https://github.com/coreos/bootupd/issues/933
let timestamp = std::time::SystemTime::now();
let meta = ContentMetadata {
timestamp: chrono::DateTime::<Utc>::from(timestamp),
version: packages.join(","),
versions: Some(modules_vec),
};
write_update_metadata(sysroot, self, &meta)?;
Ok(Some(meta))
} else {
anyhow::bail!("Failed to find EFI components");
}
write_update_metadata(sysroot_path.as_str(), self, &metadata)?;

Ok(Some(metadata))
}

fn query_update(&self, sysroot: &openat::Dir) -> Result<Option<ContentMetadata>> {
Expand Down Expand Up @@ -801,6 +800,34 @@ fn find_file_recursive<P: AsRef<Path>>(dir: P, target_file: &str) -> Result<Vec<
Ok(result)
}

#[context("Generating metadata from usr/lib/efi")]
fn generate_meta_from_usr_efi(sysroot_path: &Utf8Path) -> Result<ContentMetadata> {
let Some(efi_components) = get_efi_component_from_usr(sysroot_path, EFILIB)? else {
anyhow::bail!("Failed to find EFI components");
};
Comment on lines +805 to +807

@jbtrystram jbtrystram Apr 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's an unusual way of handling the unwrapping.
Why do you need to bail here ? Can't the error simply be bubbled up ?

@Johan-Liebert1 Johan-Liebert1 Apr 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

get_efi_component_from_usr returns Result<Option<T>>, the let .. else {} is to handle the Option part after the result is bubbled up by the ? operator

@jbtrystram jbtrystram Apr 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, T is an option already, I missed that. There is flatten that could work I guess.


let mut packages = Vec::new();
let mut modules_vec: Vec<Module> = vec![];
for efi in efi_components {
packages.push(format!("{}-{}", efi.name, efi.version));
modules_vec.push(Module {
name: efi.name,
rpm_evr: efi.version,
});
}
modules_vec.sort_unstable();

// change to now to workaround https://github.com/coreos/bootupd/issues/933

@jbtrystram jbtrystram Apr 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we account for SOURCE_DATE_EPOCH here ?
See #1075

@Johan-Liebert1 Johan-Liebert1 Apr 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This does sound like a one liner fix, afaics we are comparing package names and version for sorting and not the timestamps. This could be a separate PR though. wdyt?

let timestamp = std::time::SystemTime::now();
let meta = ContentMetadata {
timestamp: chrono::DateTime::<Utc>::from(timestamp),
version: packages.join(","),
versions: Some(modules_vec),
};

Ok(meta)
}

#[derive(Debug, PartialEq, Eq)]
pub struct EFIComponent {
pub name: String,
Expand Down Expand Up @@ -851,57 +878,43 @@ fn get_efi_component_from_usr<'a>(
Ok(Some(components))
}

/// Copy files from usr/lib/ostree-boot/efi/EFI to /usr/lib/efi/<component>/<evr>/
fn transfer_ostree_boot_to_usr(sysroot: &Path) -> Result<()> {
let ostreeboot_efi = Path::new(ostreeutil::BOOT_PREFIX).join("efi");
let ostreeboot_efi_path = sysroot.join(&ostreeboot_efi);
/// Copies usr/lib/ostree-boot/EFI to usr/lib/bootupd/updates
fn transfer_ostree_boot_to_bootupd_updates(
sysroot: &Utf8Path,
component: &Efi,
) -> Result<ContentMetadata> {
let ostreebootdir = sysroot.join(ostreeutil::BOOT_PREFIX);

let efi = ostreeboot_efi_path.join("EFI");
if !efi.exists() {
return Ok(());
// move EFI files to updates dir from /usr/lib/ostree-boot
if !ostreebootdir.exists() {
anyhow::bail!("Failed to find {ostreebootdir}");
}
for entry in WalkDir::new(&efi) {
let entry = entry?;

if entry.file_type().is_file() {
let entry_path = entry.path();
let efisrc = ostreebootdir.join("efi/EFI");
if !efisrc.exists() {
bail!("Failed to find {:?}", &efisrc);
}

// get path EFI/{BOOT,<vendor>}/<file>
let filepath = entry_path.strip_prefix(&ostreeboot_efi_path)?;
// get path /boot/efi/EFI/{BOOT,<vendor>}/<file>
let boot_filepath = Path::new("/boot/efi").join(filepath);
let dest_efidir = component_updatedir(sysroot.as_str(), component);
let dest_efidir = Utf8PathBuf::from_path_buf(dest_efidir).expect("Path is invalid UTF-8");

// Run `rpm -qf <filepath>`
let pkg = crate::packagesystem::query_file(
sysroot.to_str().unwrap(),
boot_filepath.to_str().unwrap(),
)?;
// Fork off mv() because on overlayfs one can't rename() a lower level
// directory today, and this will handle the copy fallback.
Command::new("mv")
.args([&efisrc, &dest_efidir])
.run_capture_stderr()?;

let (name, evr) = pkg.split_once(' ').unwrap();
let component = name.split('-').next().unwrap_or("");
// get path usr/lib/efi/<component>/<evr>
let efilib_path = Path::new(EFILIB).join(component).join(evr);
let efidir = openat::Dir::open(dest_efidir.as_std_path())
.with_context(|| format!("Opening {}", dest_efidir))?;

let sysroot_dir = openat::Dir::open(sysroot)?;
// Ensure dest parent directory exists
if let Some(parent) = efilib_path.join(filepath).parent() {
sysroot_dir.ensure_dir_all(parent, 0o755)?;
}
let files = crate::util::filenames(&efidir)?.into_iter().map(|mut f| {
f.insert_str(0, "/boot/efi/EFI");
f
});

// Source dir is usr/lib/ostree-boot/efi
let src = sysroot_dir
.sub_dir(&ostreeboot_efi)
.context("Opening ostree-boot dir")?;
// Dest dir is usr/lib/efi/<component>/<evr>
let dest = sysroot_dir
.sub_dir(&efilib_path)
.context("Opening usr/lib/efi dir")?;
// Copy file from ostree-boot to usr/lib/efi
src.copy_file_at(filepath, &dest, filepath)
.context("Copying file to usr/lib/efi")?;
}
}
Ok(())
let files = files.collect::<Vec<_>>();

query_files(sysroot.as_str(), files)
}

#[cfg(test)]
Expand Down
15 changes: 0 additions & 15 deletions src/packagesystem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,6 @@ fn split_name_version(input: &str) -> Option<(String, String)> {
Some((name.to_string(), format!("{version}-{release}")))
}

/// Query the rpm database and get package "<name> <evr>"
pub(crate) fn query_file(sysroot_path: &str, path: &str) -> Result<String> {
let mut c = ostreeutil::rpm_cmd(sysroot_path)?;
c.args(["-q", "--queryformat", "%{NAME} %{EVR}", "-f", path]);

let rpmout = c.output()?;
if !rpmout.status.success() {
std::io::stderr().write_all(&rpmout.stderr)?;
bail!("Failed to invoke rpm -qf");
}

let output = String::from_utf8(rpmout.stdout)?;
Ok(output.trim().to_string())
}

fn parse_evr(pkg: &str) -> Module {
// assume it is "grub2-1:2.12-28.fc42" (from usr/lib/efi)
if !pkg.ends_with(std::env::consts::ARCH) {
Expand Down
20 changes: 15 additions & 5 deletions tests/e2e-update/e2e-update-in-vm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,21 @@ ok validate

bootupctl status | tee out.txt
assert_file_has_content_literal out.txt 'Component EFI'
assert_file_has_content_literal out.txt ' Installed: grub2-1:'
assert_not_file_has_content out.txt ' Installed:.*test_bootupd_payload'
assert_not_file_has_content out.txt ' Installed:.*'"${TARGET_GRUB_EVR}"
assert_file_has_content out.txt 'Update: Available:.*'"${TARGET_GRUB_EVR}"
assert_file_has_content out.txt 'Update: Available:.*test_bootupd_payload-1.0'

if [[ -d /usr/lib/efi ]]; then
assert_file_has_content_literal out.txt ' Installed: grub2-1:'
assert_not_file_has_content out.txt ' Installed:.*test_bootupd_payload'
assert_not_file_has_content out.txt ' Installed:.*'"${TARGET_GRUB_EVR}"
assert_file_has_content out.txt 'Update: Available:.*'"${TARGET_GRUB_EVR}"
assert_file_has_content out.txt 'Update: Available:.*test_bootupd_payload-1.0'
else
assert_file_has_content_literal out.txt ' Installed: grub2-efi-x64-'
assert_not_file_has_content out.txt ' Installed:.*test_bootupd_payload'
assert_not_file_has_content out.txt ' Installed:.*'"${TARGET_GRUB_PKG}"
assert_file_has_content out.txt 'Update: Available:.*'"${TARGET_GRUB_PKG}"
assert_file_has_content out.txt 'Update: Available:.*test_bootupd_payload-1.0'
fi

bootupctl status --print-if-available > out.txt
assert_file_has_content_literal 'out.txt' 'Updates available: BIOS EFI'
ok update avail
Expand Down
2 changes: 2 additions & 0 deletions tests/e2e-update/e2e-update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ case $(arch) in
*) fatal "Unhandled arch $(arch)";;
esac
target_grub_name=grub2-efi-${grubarch}
target_grub_pkg=$(rpm -qp --queryformat='%{nevra}\n' ${overrides}/rpm/${target_grub_name}-2*.rpm)
target_grub_evr=$(rpm -qp --queryformat='%{evr}\n' ${overrides}/rpm/${target_grub_name}-2*.rpm)
target_commit=$(cosa meta --get-value ostree-commit)
echo "Target commit: ${target_commit}"
Expand All @@ -97,6 +98,7 @@ systemd:
RemainAfterExit=yes
Environment=TARGET_COMMIT=${target_commit}
Environment=TARGET_GRUB_NAME=${target_grub_name}
Environment=TARGET_GRUB_PKG=${target_grub_pkg}
Environment=TARGET_GRUB_EVR=${target_grub_evr}
Environment=SRCDIR=/run/bootupd-source
# Run via shell because selinux denies systemd writing to 9p apparently
Expand Down
Loading
Loading