Skip to content

Commit 59f98b0

Browse files
committed
Improve get_efi_vendor() to use Path
This follows #991 (comment)
1 parent 12c2362 commit 59f98b0

File tree

4 files changed

+17
-24
lines changed

4 files changed

+17
-24
lines changed

src/bios.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ impl Component for Bios {
267267
Ok(ValidationResult::Skip)
268268
}
269269

270-
fn get_efi_vendor(&self, _: &str) -> Result<Option<String>> {
270+
fn get_efi_vendor(&self, _: &Path) -> Result<Option<String>> {
271271
Ok(None)
272272
}
273273
}

src/bootupd.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ pub(crate) fn install(
9999
log::info!("Installed {} {}", component.name(), meta.meta.version);
100100
state.installed.insert(component.name().into(), meta);
101101
// Yes this is a hack...the Component thing just turns out to be too generic.
102-
if let Some(vendor) = component.get_efi_vendor(&source_root)? {
102+
if let Some(vendor) = component.get_efi_vendor(&Path::new(source_root))? {
103103
assert!(installed_efi_vendor.is_none());
104104
installed_efi_vendor = Some(vendor);
105105
}

src/component.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pub(crate) trait Component {
7878
fn validate(&self, current: &InstalledContent) -> Result<ValidationResult>;
7979

8080
/// Locating efi vendor dir
81-
fn get_efi_vendor(&self, sysroot: &str) -> Result<Option<String>>;
81+
fn get_efi_vendor(&self, sysroot: &Path) -> Result<Option<String>>;
8282
}
8383

8484
/// Given a component name, create an implementation.
@@ -202,7 +202,6 @@ mod tests {
202202
let tdp = td.path();
203203
let tdupdates = "usr/lib/bootupd/updates/EFI";
204204
let tdir = openat::Dir::open(tdp)?;
205-
let td = tdp.to_str().unwrap();
206205

207206
tdir.ensure_dir_all(tdupdates, 0o755)?;
208207
let efi = tdir.sub_dir(tdupdates)?;
@@ -225,13 +224,13 @@ mod tests {
225224
let target_components: Vec<_> = all_components.values().collect();
226225
for &component in target_components.iter() {
227226
if component.name() == "BIOS" {
228-
assert_eq!(component.get_efi_vendor(&td)?, None);
227+
assert_eq!(component.get_efi_vendor(tdp)?, None);
229228
}
230229
if component.name() == "EFI" {
231-
let x = component.get_efi_vendor(&td);
230+
let x = component.get_efi_vendor(tdp);
232231
assert_eq!(x.is_err(), true);
233232
efi.remove_all("centos")?;
234-
assert_eq!(component.get_efi_vendor(&td)?, Some("fedora".to_string()));
233+
assert_eq!(component.get_efi_vendor(tdp)?, Some("fedora".to_string()));
235234
{
236235
let td_vendor = "usr/lib/efi/shim/15.8-3/EFI/centos";
237236
tdir.ensure_dir_all(td_vendor, 0o755)?;
@@ -243,16 +242,15 @@ mod tests {
243242
)?;
244243

245244
// usr/lib/efi wins and get 'centos'
246-
assert_eq!(component.get_efi_vendor(&td)?, Some("centos".to_string()));
245+
assert_eq!(component.get_efi_vendor(tdp)?, Some("centos".to_string()));
247246
// find directly from usr/lib/efi and get 'centos'
248-
let td_usr = format!("{td}/usr/lib/efi");
247+
let td_usr = tdp.join("usr/lib/efi");
249248
assert_eq!(
250249
component.get_efi_vendor(&td_usr)?,
251250
Some("centos".to_string())
252251
);
253252
// find directly from updates and get 'fedora'
254-
let td_efi =
255-
format!("{td}/{}", component_updatedirname(&**component).display());
253+
let td_efi = tdp.join(component_updatedirname(&**component));
256254
assert_eq!(
257255
component.get_efi_vendor(&td_efi)?,
258256
Some("fedora".to_string())

src/efi.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ impl Component for Efi {
257257
fn migrate_static_grub_config(&self, sysroot_path: &str, destdir: &openat::Dir) -> Result<()> {
258258
let sysroot =
259259
openat::Dir::open(sysroot_path).with_context(|| format!("Opening {sysroot_path}"))?;
260-
let Some(vendor) = self.get_efi_vendor(sysroot_path)? else {
260+
let Some(vendor) = self.get_efi_vendor(&Path::new(sysroot_path))? else {
261261
anyhow::bail!("Failed to find efi vendor");
262262
};
263263

@@ -380,7 +380,7 @@ impl Component for Efi {
380380
.current_dir(format!("/proc/self/fd/{}", src_dir.as_raw_fd()))
381381
.run()?;
382382
if update_firmware {
383-
if let Some(vendordir) = self.get_efi_vendor(&src_root)? {
383+
if let Some(vendordir) = self.get_efi_vendor(&Path::new(src_root))? {
384384
self.update_firmware(device, destd, &vendordir)?
385385
}
386386
}
@@ -545,20 +545,15 @@ impl Component for Efi {
545545
}
546546
}
547547

548-
fn get_efi_vendor(&self, sysroot: &str) -> Result<Option<String>> {
549-
let sysroot_path = Path::new(sysroot);
550-
let efi_lib = sysroot_path.join(EFILIB);
551-
let updates = sysroot_path.join(component_updatedirname(self));
548+
fn get_efi_vendor(&self, sysroot: &Path) -> Result<Option<String>> {
549+
let efi_lib = sysroot.join(EFILIB);
550+
let updates = sysroot.join(component_updatedirname(self));
552551

553-
let target = if let Some(p) = [efi_lib.as_path(), updates.as_path(), sysroot_path]
552+
let paths: [&Path; 3] = [&efi_lib, &updates, sysroot];
553+
let target = paths
554554
.into_iter()
555555
.find(|p| p.exists())
556-
{
557-
p
558-
} else {
559-
bail!("Failed to find valid target path");
560-
};
561-
556+
.ok_or_else(|| anyhow::anyhow!("Failed to find valid target path"))?;
562557
let shim_files = find_file_recursive(target, SHIM)?;
563558

564559
// Does not support multiple shim for efi

0 commit comments

Comments
 (0)