Skip to content

Commit

Permalink
utils: Add a little CommandRunExt helper trait
Browse files Browse the repository at this point in the history
When I added the `Task` stuff it wasn't with the idea
that we'd use it for *all* subprocess invocations.

I think in some cases we really just do want a `Command` instance
without the extra wrappering.

Add an implementation (there are many of variants of this out
there in lots of Rust codebases)

This makes sense to use instead of `Task` where we're using
`.quiet()` so that we don't print the description, and especially
where we don't expect the task to fail usually, so it's OK
if the error message is odd.

Signed-off-by: Colin Walters <[email protected]>
  • Loading branch information
cgwalters committed Jul 25, 2024
1 parent a557143 commit 858d886
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
7 changes: 3 additions & 4 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use crate::mount::Filesystem;
use crate::spec::ImageReference;
use crate::store::Storage;
use crate::task::Task;
use crate::utils::sigpolicy_from_opts;
use crate::utils::{sigpolicy_from_opts, CommandRunExt};

/// The default "stateroot" or "osname"; see https://github.com/ostreedev/ostree/issues/2794
const STATEROOT_DEFAULT: &str = "default";
Expand Down Expand Up @@ -584,10 +584,9 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result
("sysroot.bootprefix", "true"),
("sysroot.readonly", "true"),
] {
Task::new("Configuring ostree repo", "ostree")
Command::new("ostree")
.args(["config", "--repo", "ostree/repo", "set", k, v])
.cwd(rootfs_dir)?
.quiet()
.cwd_dir(rootfs_dir.try_clone()?)
.run()?;
}
Task::new("Initializing sysroot", "ostree")
Expand Down
25 changes: 25 additions & 0 deletions lib/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,25 @@ 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<()>;
}

impl CommandRunExt for Command {
/// Synchronously execute the child, and return an error if the child exited unsuccessfully.
fn run(&mut self) -> Result<()> {
let st = self.status()?;
if !st.success() {
// 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.
anyhow::bail!(format!("Subprocess failed: {st:?}"))
}
Ok(())
}
}

/// 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 {
Expand Down Expand Up @@ -190,3 +209,9 @@ fn test_sigpolicy_from_opts() {
SignatureSource::ContainerPolicyAllowInsecure
);
}

#[test]
fn command_run_ext() {
Command::new("true").run().unwrap();
assert!(Command::new("false").run().is_err());
}

0 comments on commit 858d886

Please sign in to comment.