Skip to content

Allow to skip BIOS component installation for x86_64 #1010

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
merged 3 commits into from
Sep 26, 2025
Merged

Allow to skip BIOS component installation for x86_64 #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
365ef1f
component: Make update metadata generation to be optional
martinezjavier Sep 25, 2025
81edbae
component: Skip installing components without update metadata
martinezjavier Sep 25, 2025
f9de0fd
bios: Don't bail out if `grub2-install` is not present
martinezjavier Sep 25, 2025
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
7 changes: 4 additions & 3 deletions src/bios.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,17 @@ impl Component for Bios {
})
}

fn generate_update_metadata(&self, sysroot_path: &str) -> Result<ContentMetadata> {
fn generate_update_metadata(&self, sysroot_path: &str) -> Result<Option<ContentMetadata>> {
let grub_install = Path::new(sysroot_path).join(GRUB_BIN);
if !grub_install.exists() {
bail!("Failed to find {:?}", grub_install);
println!("Failed to find {:?}", grub_install);
Copy link
Member

Choose a reason for hiding this comment

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

How about: Skipping, executable not found: {:?} (it's not a failure) - as you've used elsewhere in the PR

return Ok(None);
Comment on lines +133 to +134
Copy link

Choose a reason for hiding this comment

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

medium

Using println! for this diagnostic message can add noise to the standard output, which is primarily used for user-facing progress messages. It's better to use the log crate for such messages. This provides more control over verbosity and separates diagnostic output from application output. The calling function in bootupd.rs already prints a user-friendly message about skipping the component.

Note: You will need to add use log; at the top of the file.

Suggested change
println!("Failed to find {:?}", grub_install);
return Ok(None);
log::info!("Skipping BIOS metadata generation; grub2-install not found at {:?}", grub_install);
return Ok(None);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the println!() macro to be consistent with how other similar messages were printed.

}

// Query the rpm database and list the package and build times for /usr/sbin/grub2-install
let meta = packagesystem::query_files(sysroot_path, [&grub_install])?;
write_update_metadata(sysroot_path, self, &meta)?;
Ok(meta)
Ok(Some(meta))
}

fn query_adopt(&self, devices: &Option<Vec<String>>) -> Result<Option<Adoptable>> {
Expand Down
27 changes: 21 additions & 6 deletions src/bootupd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ pub(crate) fn install(
continue;
}

// skip components that don't have an update metadata
Copy link
Member

Choose a reason for hiding this comment

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

Hmm this one is interesting. I think actually the problem is we're iterating over the static list of components, when actually I think we should just accept whatever the updated components in the updates set are to start with or so.

Although it raises a different point - it would probably make sense in the future to handle the case where a component is removed. (e.g. an OS update drops out BIOS support and we would then in theory clear out the BIOS partition e.g.)

And to do that, we do need to iterate over the components we know (as this is doing) and intersect them with the update set.

if component.query_update(&source_root_dir)?.is_none() {
println!(
"Skip installing component {} without update metadata",
component.name()
);
continue;
}

let meta = component
.install(&source_root, dest_root, device, update_firmware)
.with_context(|| format!("installing component {}", component.name()))?;
Expand Down Expand Up @@ -199,12 +208,18 @@ pub(crate) fn generate_update_metadata(sysroot_path: &str) -> Result<()> {
std::fs::create_dir_all(&updates_dir)
.with_context(|| format!("Failed to create updates dir {:?}", &updates_dir))?;
for component in get_components().values() {
let v = component.generate_update_metadata(sysroot_path)?;
println!(
"Generated update layout for {}: {}",
component.name(),
v.version,
);
if let Some(v) = component.generate_update_metadata(sysroot_path)? {
println!(
"Generated update layout for {}: {}",
component.name(),
v.version,
);
} else {
println!(
"Generating update layout for {} was not possible, skipping.",
component.name(),
);
}
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/component.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub(crate) trait Component {
/// this is an `rpm-ostree compose tree` for example. For a dual-partition
/// style updater, this would be run as part of a postprocessing step
/// while the filesystem for the partition is mounted.
fn generate_update_metadata(&self, sysroot: &str) -> Result<ContentMetadata>;
fn generate_update_metadata(&self, sysroot: &str) -> Result<Option<ContentMetadata>>;

/// Used on the client to query for an update cached in the current booted OS.
fn query_update(&self, sysroot: &openat::Dir) -> Result<Option<ContentMetadata>>;
Expand Down
4 changes: 2 additions & 2 deletions src/efi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ impl Component for Efi {
})
}

fn generate_update_metadata(&self, sysroot: &str) -> Result<ContentMetadata> {
fn generate_update_metadata(&self, sysroot: &str) -> Result<Option<ContentMetadata>> {
let sysroot_path = Utf8Path::new(sysroot);

// copy EFI files to updates dir from usr/lib/efi
Expand Down Expand Up @@ -533,7 +533,7 @@ impl Component for Efi {
};

write_update_metadata(sysroot, self, &meta)?;
Ok(meta)
Ok(Some(meta))
}

fn query_update(&self, sysroot: &openat::Dir) -> Result<Option<ContentMetadata>> {
Expand Down