Skip to content

Commit a3ddfb6

Browse files
composefs/status: Address review feedback on bootloader detection
Two follow-ups from review on #2376: Reword the `BLS_ENTRIES_DIR` doc comment. It claimed the directory's presence signals a non-EFI system using the BLS layout, which is misleading: EFI systems have the directory too, and GRUB reads the same Type 1 entries via `blscfg`. It only carries information in the non-EFI branch, and only alongside the `GRUB_DIRS` probe. Cache the bootloader unconditionally. The previous `if non_efi` gate was added on the mistaken belief that unified caching broke the grub-cc tests; those failures were pre-existing and tracked separately in #2378. The bootloader cannot change over the lifetime of a single bootc invocation, so there is no reason to re-read `EFI_LOADER_INFO` per call. Signed-off-by: Dustin Kirkland <dustin.kirkland@chainguard.dev>
1 parent bf1f012 commit a3ddfb6

1 file changed

Lines changed: 10 additions & 10 deletions

File tree

crates/lib/src/bootc_composefs/status.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -440,9 +440,13 @@ pub(crate) async fn get_container_manifest_and_config(
440440
Ok(ImgConfigManifest { manifest, config })
441441
}
442442

443-
/// Directory where systemd-boot / BLS-compatible bootloaders expect Type 1
444-
/// boot entries. Its presence is used as a signal that a non-EFI system
445-
/// nevertheless uses the BLS layout (see [`classify_bootloader`]).
443+
/// Directory where BLS-compatible bootloaders expect Type 1 boot entries.
444+
///
445+
/// Its presence says nothing about the bootloader on its own: EFI systems
446+
/// have it too, and GRUB reads the same entries via `blscfg`. It is only
447+
/// consulted when there are no EFI variables to inspect, and then only
448+
/// together with [`GRUB_DIRS`] to tell a BLS-native bootloader apart from
449+
/// GRUB (see [`classify_bootloader`]).
446450
const BLS_ENTRIES_DIR: &str = "/boot/loader/entries";
447451

448452
/// Directories where GRUB keeps its own configuration and modules. Their
@@ -522,10 +526,6 @@ pub(crate) fn get_bootloader() -> Result<Bootloader> {
522526
}
523527

524528
let efi_result = read_uefi_var(EFI_LOADER_INFO);
525-
// Non-EFI systems have a stable filesystem-based classification, so we
526-
// can cache. EFI systems are left uncached to preserve the pre-existing
527-
// behavior of re-reading `EFI_LOADER_INFO` on every call — some tests
528-
// observe bootloader-info changes over the course of a run.
529529
let non_efi = matches!(
530530
&efi_result,
531531
Err(EfiError::SystemNotUEFI) | Err(EfiError::MissingVar),
@@ -539,9 +539,9 @@ pub(crate) fn get_bootloader() -> Result<Bootloader> {
539539
non_efi && GRUB_DIRS.iter().any(|d| std::path::Path::new(d).is_dir()),
540540
)?;
541541

542-
if non_efi {
543-
BOOTLOADER.get_or_init(|| bootloader);
544-
}
542+
// The bootloader cannot change over the lifetime of a single bootc
543+
// invocation, so cache unconditionally.
544+
BOOTLOADER.get_or_init(|| bootloader);
545545

546546
Ok(bootloader)
547547
}

0 commit comments

Comments
 (0)