Merge pull request coreos#821 from cgwalters/use-bootc-utils

Use bootc-utils for command helpers
HuijingHei · Jan 21, 2025 · efe41b6 · efe41b6
2 parents e296f82 + 7f46422
commit efe41b6
Show file tree

Hide file tree

Showing 5 changed files with 165 additions and 40 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -21,6 +21,7 @@ path = "src/main.rs"
 [dependencies]
 anyhow = "1.0"
 bincode = "1.3.2"
+bootc-utils = { git = "https://github.com/containers/bootc", commit = "e8bb9f748fdfeb5164667046b3c6678c619ad46c" }
 cap-std-ext = "4.0.4"
 camino = "1.1.9"
 chrono = { version = "0.4.39", features = ["serde"] }

diff --git a/src/efi.rs b/src/efi.rs
@@ -121,13 +121,11 @@ impl Efi {
             if !mnt.exists() {
                 continue;
             }
-            let status = std::process::Command::new("mount")
+            std::process::Command::new("mount")
                 .arg(&esp_device)
                 .arg(&mnt)
-                .status()?;
-            if !status.success() {
-                anyhow::bail!("Failed to mount {:?}", esp_device);
-            }
+                .run()
+                .with_context(|| format!("Failed to mount {:?}", esp_device))?;
             log::debug!("Mounted at {mnt:?}");
             *mountpoint = Some(mnt);
             break;
@@ -137,10 +135,10 @@ impl Efi {
 
     fn unmount(&self) -> Result<()> {
         if let Some(mount) = self.mountpoint.borrow_mut().take() {
-            let status = Command::new("umount").arg(&mount).status()?;
-            if !status.success() {
-                anyhow::bail!("Failed to unmount {mount:?}: {status:?}");
-            }
+            Command::new("umount")
+                .arg(&mount)
+                .run()
+                .with_context(|| format!("Failed to unmount {mount:?}"))?;
             log::trace!("Unmounted");
         }
         Ok(())
@@ -308,15 +306,12 @@ impl Component for Efi {
 
         // TODO - add some sort of API that allows directly setting the working
         // directory to a file descriptor.
-        let r = std::process::Command::new("cp")
+        std::process::Command::new("cp")
             .args(["-rp", "--reflink=auto"])
             .arg(&srcdir_name)
             .arg(destdir)
             .current_dir(format!("/proc/self/fd/{}", src_root.as_raw_fd()))
-            .status()?;
-        if !r.success() {
-            anyhow::bail!("Failed to copy");
-        }
+            .run()?;
         if update_firmware {
             if let Some(vendordir) = self.get_efi_vendor(&src_root)? {
                 self.update_firmware(device, destd, &vendordir)?
@@ -505,17 +500,10 @@ pub(crate) fn clear_efi_target(target: &str) -> Result<()> {
     for entry in boot_entries {
         if entry.name.to_lowercase() == target {
             log::debug!("Deleting matched target {:?}", entry);
-            let output = Command::new(EFIBOOTMGR)
+            Command::new(EFIBOOTMGR)
                 .args(["-b", entry.id.as_str(), "-B"])
-                .output()?;
-            let st = output.status;
-            if !st.success() {
-                std::io::copy(
-                    &mut std::io::Cursor::new(output.stderr),
-                    &mut std::io::stderr().lock(),
-                )?;
-                anyhow::bail!("Failed to invoke {EFIBOOTMGR}: {st:?}");
-            }
+                .run()
+                .with_context(|| format!("Failed to invoke {EFIBOOTMGR}"))?;
         }
     }
 

diff --git a/src/filesystem.rs b/src/filesystem.rs
@@ -1,9 +1,9 @@
-use std::io::Write;
 use std::os::fd::AsRawFd;
 use std::os::unix::process::CommandExt;
 use std::process::Command;
 
-use anyhow::{Context, Result};
+use anyhow::Result;
+use bootc_utils::CommandRunExt;
 use fn_error_context::context;
 use rustix::fd::BorrowedFd;
 use serde::Deserialize;
@@ -27,19 +27,12 @@ pub(crate) struct Findmnt {
 pub(crate) fn inspect_filesystem(root: &openat::Dir, path: &str) -> Result<Filesystem> {
     let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) };
     // SAFETY: This is unsafe just for the pre_exec, when we port to cap-std we can use cap-std-ext
-    let o = unsafe {
+    let o: Findmnt = unsafe {
         Command::new("findmnt")
             .args(["-J", "-v", "--output=SOURCE,FSTYPE,OPTIONS,UUID", path])
             .pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into))
-            .output()?
+            .run_and_parse_json()?
     };
-    let st = o.status;
-    if !st.success() {
-        let _ = std::io::stderr().write_all(&o.stderr)?;
-        anyhow::bail!("findmnt failed: {st:?}");
-    }
-    let o: Findmnt = serde_json::from_reader(std::io::Cursor::new(&o.stdout))
-        .context("Parsing findmnt output")?;
     o.filesystems
         .into_iter()
         .next()

diff --git a/src/filetree.rs b/src/filetree.rs
@@ -286,18 +286,17 @@ pub(crate) fn syncfs(d: &openat::Dir) -> Result<()> {
 /// Copy from src to dst at root dir
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
+    use bootc_utils::CommandRunExt;
+
     let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) };
-    let r = unsafe {
+    unsafe {
         Command::new("cp")
             .args(["-a"])
             .arg(src)
             .arg(dst)
             .pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into))
-            .status()?
+            .run()?
     };
-    if !r.success() {
-        anyhow::bail!("Failed to copy {src} to {dst}");
-    }
     log::debug!("Copy {src} to {dst}");
     Ok(())
 }