lib: Move Command extensions to new mod cmdutil

Prep for more stuff there.

Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

lib/src/cmdutils.rs

@@ -0,0 +1,130 @@
+use std::{
+    io::{Read, Seek},
+    process::Command,
+};
+
+use anyhow::Result;
+
+/// Helpers intended for [`std::process::Command`].
+pub(crate) trait CommandRunExt {
+    fn run(&mut self) -> Result<()>;
+}
+
+/// Helpers intended for [`std::process::ExitStatus`].
+pub(crate) trait ExitStatusExt {
+    /// If the exit status signals it was not successful, return an error.
+    /// Note that we intentionally *don't* include the command string
+    /// in the output; we leave it to the caller to add that if they want,
+    /// as it may be verbose.
+    fn check_status(&mut self, stderr: std::fs::File) -> Result<()>;
+}
+
+/// Parse the last chunk (e.g. 1024 bytes) from the provided file,
+/// ensure it's UTF-8, and return that value. This function is infallible;
+/// if the file cannot be read for some reason, a copy of a static string
+/// is returned.
+fn last_utf8_content_from_file(mut f: std::fs::File) -> String {
+    // u16 since we truncate to just the trailing bytes here
+    // to avoid pathological error messages
+    const MAX_STDERR_BYTES: u16 = 1024;
+    let size = f
+        .metadata()
+        .map_err(|e| {
+            tracing::warn!("failed to fstat: {e}");
+        })
+        .map(|m| m.len().try_into().unwrap_or(u16::MAX))
+        .unwrap_or(0);
+    let size = size.min(MAX_STDERR_BYTES);
+    let seek_offset = -(size as i32);
+    let mut stderr_buf = Vec::with_capacity(size.into());
+    // We should never fail to seek()+read() really, but let's be conservative
+    let r = match f
+        .seek(std::io::SeekFrom::End(seek_offset.into()))
+        .and_then(|_| f.read_to_end(&mut stderr_buf))
+    {
+        Ok(_) => String::from_utf8_lossy(&stderr_buf),
+        Err(e) => {
+            tracing::warn!("failed seek+read: {e}");
+            "<failed to read stderr>".into()
+        }
+    };
+    (&*r).to_owned()
+}
+
+impl ExitStatusExt for std::process::ExitStatus {
+    fn check_status(&mut self, stderr: std::fs::File) -> Result<()> {
+        let stderr_buf = last_utf8_content_from_file(stderr);
+        if self.success() {
+            return Ok(());
+        }
+        anyhow::bail!(format!("Subprocess failed: {self:?}\n{stderr_buf}"))
+    }
+}
+
+impl CommandRunExt for Command {
+    /// Synchronously execute the child, and return an error if the child exited unsuccessfully.
+    fn run(&mut self) -> Result<()> {
+        let stderr = tempfile::tempfile()?;
+        self.stderr(stderr.try_clone()?);
+        self.status()?.check_status(stderr)
+    }
+}
+
+/// Helpers intended for [`tokio::process::Command`].
+#[allow(dead_code)]
+pub(crate) trait AsyncCommandRunExt {
+    async fn run(&mut self) -> Result<()>;
+}
+
+impl AsyncCommandRunExt for tokio::process::Command {
+    /// Asynchronously execute the child, and return an error if the child exited unsuccessfully.
+    ///
+    async fn run(&mut self) -> Result<()> {
+        let stderr = tempfile::tempfile()?;
+        self.stderr(stderr.try_clone()?);
+        self.status().await?.check_status(stderr)
+    }
+}
+
+#[test]
+fn command_run_ext() {
+    // The basics
+    Command::new("true").run().unwrap();
+    assert!(Command::new("false").run().is_err());
+
+    // Verify we capture stderr
+    let e = Command::new("/bin/sh")
+        .args(["-c", "echo expected-this-oops-message 1>&2; exit 1"])
+        .run()
+        .err()
+        .unwrap();
+    similar_asserts::assert_eq!(
+        e.to_string(),
+        "Subprocess failed: ExitStatus(unix_wait_status(256))\nexpected-this-oops-message\n"
+    );
+
+    // Ignoring invalid UTF-8
+    let e = Command::new("/bin/sh")
+        .args([
+            "-c",
+            r"echo -e 'expected\xf5\x80\x80\x80\x80-foo\xc0bar\xc0\xc0' 1>&2; exit 1",
+        ])
+        .run()
+        .err()
+        .unwrap();
+    similar_asserts::assert_eq!(
+        e.to_string(),
+        "Subprocess failed: ExitStatus(unix_wait_status(256))\nexpected�����-foo�bar��\n"
+    );
+}
+
+#[tokio::test]
+async fn async_command_run_ext() {
+    use tokio::process::Command as AsyncCommand;
+    let mut success = AsyncCommand::new("true");
+    let mut fail = AsyncCommand::new("false");
+    // Run these in parallel just because we can
+    let (success, fail) = tokio::join!(success.run(), fail.run(),);
+    success.unwrap();
+    assert!(fail.is_err());
+}

lib/src/image.rs

@@ -6,7 +6,7 @@ use anyhow::{Context, Result};
 use fn_error_context::context;
 use ostree_ext::container::{ImageReference, Transport};
 
-use crate::{imgstorage::Storage, utils::CommandRunExt};
+use crate::{cmdutils::CommandRunExt, imgstorage::Storage};
 
 /// The name of the image we push to containers-storage if nothing is specified.
 const IMAGE_DEFAULT: &str = "localhost/bootc";

lib/src/imgstorage.rs

@@ -22,7 +22,7 @@ use fn_error_context::context;
 use std::os::fd::OwnedFd;
 use tokio::process::Command as AsyncCommand;
 
-use crate::utils::{AsyncCommandRunExt, CommandRunExt, ExitStatusExt};
+use crate::cmdutils::{AsyncCommandRunExt, CommandRunExt, ExitStatusExt};
 
 // Pass only 100 args at a time just to avoid potentially overflowing argument
 // vectors; not that this should happen in reality, but just in case.

lib/src/install.rs

@@ -43,12 +43,13 @@ use rustix::fs::{FileTypeExt, MetadataExt as _};
 use serde::{Deserialize, Serialize};
 
 use self::baseline::InstallBlockDeviceOpts;
+use crate::cmdutils::CommandRunExt;
 use crate::containerenv::ContainerExecutionInfo;
 use crate::mount::Filesystem;
 use crate::spec::ImageReference;
 use crate::store::Storage;
 use crate::task::Task;
-use crate::utils::{sigpolicy_from_opts, CommandRunExt};
+use crate::utils::sigpolicy_from_opts;
 
 /// The default "stateroot" or "osname"; see https://github.com/ostreedev/ostree/issues/2794
 const STATEROOT_DEFAULT: &str = "default";

lib/src/lib.rs

@@ -10,6 +10,7 @@
 
 mod boundimage;
 pub mod cli;
+mod cmdutils;
 pub(crate) mod deploy;
 pub(crate) mod generator;
 mod image;

lib/src/utils.rs

@@ -1,5 +1,5 @@
 use std::future::Future;
-use std::io::{Read, Seek, Write};
+use std::io::Write;
 use std::os::fd::BorrowedFd;
 use std::process::Command;
 use std::time::Duration;
@@ -10,87 +10,6 @@ use ostree::glib;
 use ostree_ext::container::SignatureSource;
 use ostree_ext::ostree;
 
-/// Helpers intended for [`std::process::Command`].
-pub(crate) trait CommandRunExt {
-    fn run(&mut self) -> Result<()>;
-}
-
-/// Helpers intended for [`std::process::ExitStatus`].
-pub(crate) trait ExitStatusExt {
-    /// If the exit status signals it was not successful, return an error.
-    /// Note that we intentionally *don't* include the command string
-    /// in the output; we leave it to the caller to add that if they want,
-    /// as it may be verbose.
-    fn check_status(&mut self, stderr: std::fs::File) -> Result<()>;
-}
-
-/// Parse the last chunk (e.g. 1024 bytes) from the provided file,
-/// ensure it's UTF-8, and return that value. This function is infallible;
-/// if the file cannot be read for some reason, a copy of a static string
-/// is returned.
-fn last_utf8_content_from_file(mut f: std::fs::File) -> String {
-    // u16 since we truncate to just the trailing bytes here
-    // to avoid pathological error messages
-    const MAX_STDERR_BYTES: u16 = 1024;
-    let size = f
-        .metadata()
-        .map_err(|e| {
-            tracing::warn!("failed to fstat: {e}");
-        })
-        .map(|m| m.len().try_into().unwrap_or(u16::MAX))
-        .unwrap_or(0);
-    let size = size.min(MAX_STDERR_BYTES);
-    let seek_offset = -(size as i32);
-    let mut stderr_buf = Vec::with_capacity(size.into());
-    // We should never fail to seek()+read() really, but let's be conservative
-    let r = match f
-        .seek(std::io::SeekFrom::End(seek_offset.into()))
-        .and_then(|_| f.read_to_end(&mut stderr_buf))
-    {
-        Ok(_) => String::from_utf8_lossy(&stderr_buf),
-        Err(e) => {
-            tracing::warn!("failed seek+read: {e}");
-            "<failed to read stderr>".into()
-        }
-    };
-    (&*r).to_owned()
-}
-
-impl ExitStatusExt for std::process::ExitStatus {
-    fn check_status(&mut self, stderr: std::fs::File) -> Result<()> {
-        let stderr_buf = last_utf8_content_from_file(stderr);
-        if self.success() {
-            return Ok(());
-        }
-        anyhow::bail!(format!("Subprocess failed: {self:?}\n{stderr_buf}"))
-    }
-}
-
-impl CommandRunExt for Command {
-    /// Synchronously execute the child, and return an error if the child exited unsuccessfully.
-    fn run(&mut self) -> Result<()> {
-        let stderr = tempfile::tempfile()?;
-        self.stderr(stderr.try_clone()?);
-        self.status()?.check_status(stderr)
-    }
-}
-
-/// Helpers intended for [`tokio::process::Command`].
-#[allow(dead_code)]
-pub(crate) trait AsyncCommandRunExt {
-    async fn run(&mut self) -> Result<()>;
-}
-
-impl AsyncCommandRunExt for tokio::process::Command {
-    /// Asynchronously execute the child, and return an error if the child exited unsuccessfully.
-    ///
-    async fn run(&mut self) -> Result<()> {
-        let stderr = tempfile::tempfile()?;
-        self.stderr(stderr.try_clone()?);
-        self.status().await?.check_status(stderr)
-    }
-}
-
 /// Try to look for keys injected by e.g. rpm-ostree requesting machine-local
 /// changes; if any are present, return `true`.
 pub(crate) fn origin_has_rpmostree_stuff(kf: &glib::KeyFile) -> bool {
@@ -271,46 +190,3 @@ fn test_sigpolicy_from_opts() {
         SignatureSource::ContainerPolicyAllowInsecure
     );
 }
-
-#[test]
-fn command_run_ext() {
-    // The basics
-    Command::new("true").run().unwrap();
-    assert!(Command::new("false").run().is_err());
-
-    // Verify we capture stderr
-    let e = Command::new("/bin/sh")
-        .args(["-c", "echo expected-this-oops-message 1>&2; exit 1"])
-        .run()
-        .err()
-        .unwrap();
-    similar_asserts::assert_eq!(
-        e.to_string(),
-        "Subprocess failed: ExitStatus(unix_wait_status(256))\nexpected-this-oops-message\n"
-    );
-
-    // Ignoring invalid UTF-8
-    let e = Command::new("/bin/sh")
-        .args([
-            "-c",
-            r"echo -e 'expected\xf5\x80\x80\x80\x80-foo\xc0bar\xc0\xc0' 1>&2; exit 1",
-        ])
-        .run()
-        .err()
-        .unwrap();
-    similar_asserts::assert_eq!(
-        e.to_string(),
-        "Subprocess failed: ExitStatus(unix_wait_status(256))\nexpected�����-foo�bar��\n"
-    );
-}
-
-#[tokio::test]
-async fn async_command_run_ext() {
-    use tokio::process::Command as AsyncCommand;
-    let mut success = AsyncCommand::new("true");
-    let mut fail = AsyncCommand::new("false");
-    // Run these in parallel just because we can
-    let (success, fail) = tokio::join!(success.run(), fail.run(),);
-    success.unwrap();
-    assert!(fail.is_err());
-}