feat(vdev): add stderr to output

vdev/src/app.rs

@@ -1,11 +1,13 @@
 use std::ffi::{OsStr, OsString};
+use std::io::{PipeReader, pipe};
 use std::{
     borrow::Cow, env, io::Read, path::PathBuf, process::Command, process::ExitStatus,
     process::Stdio, sync::LazyLock, sync::OnceLock, time::Duration,
 };
 
 use anyhow::{Context as _, Result, bail};
 use indicatif::{ProgressBar, ProgressStyle};
+use itertools::Itertools;
 use log::LevelFilter;
 
 use crate::{config::Config, git, platform, util};
@@ -74,119 +76,213 @@ pub fn version() -> Result<String> {
     Ok(version)
 }
 
-/// Overlay some extra helper functions onto `std::process::Command`
-pub trait CommandExt {
-    fn script(script: &str) -> Self;
-    fn in_repo(&mut self) -> &mut Self;
-    fn check_output(&mut self) -> Result<String>;
-    fn check_run(&mut self) -> Result<()>;
-    fn run(&mut self) -> Result<ExitStatus>;
-    fn wait(&mut self, message: impl Into<Cow<'static, str>>) -> Result<()>;
-    fn pre_exec(&self);
-    fn features(&mut self, features: &[String]) -> &mut Self;
+pub struct VDevCommand {
+    pub inner: Command,
 }
 
-impl CommandExt for Command {
+impl VDevCommand {
+    #[must_use]
+    pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
+        Self {
+            inner: Command::new(program.as_ref()),
+        }
+    }
+
     /// Create a new command to execute the named script in the repository `scripts` directory.
     fn script(script: &str) -> Self {
         let path: PathBuf = [path(), "scripts", script].into_iter().collect();
         if cfg!(windows) {
             // On Windows, all scripts must be run through an explicit interpreter.
-            let mut command = Command::new(&*SHELL);
-            command.arg(path);
-            command
+            Self::new(&*SHELL).arg(path)
         } else {
             // On all other systems, we can run scripts directly.
-            Command::new(path)
+            Self::new(path)
         }
     }
+}
+
+impl From<Command> for VDevCommand {
+    fn from(command: Command) -> Self {
+        Self { inner: command }
+    }
+}
+
+impl VDevCommand {
+    #[must_use]
+    pub fn arg<S: AsRef<OsStr>>(mut self, arg: S) -> Self {
+        self.inner.arg(arg);
+        self
+    }
+
+    #[must_use]
+    pub fn args<I, S>(mut self, args: I) -> Self
+    where
+        I: IntoIterator<Item = S>,
+        S: AsRef<OsStr>,
+    {
+        self.inner.args(args);
+        self
+    }
+
+    #[must_use]
+    pub fn features(mut self, features: &[String]) -> Self {
+        self = self.arg("--no-default-features").arg("--features");
+        if features.is_empty() {
+            self = self.arg(platform::default_features());
+        } else {
+            self = self.arg(features.join(","));
+        }
+        self
+    }
+
+    #[must_use]
+    pub fn in_repo(mut self) -> Self {
+        self.inner.current_dir(path());
+        self
+    }
+
+    #[must_use]
+    pub fn env<K, V>(mut self, key: K, val: V) -> Self
+    where
+        K: AsRef<OsStr>,
+        V: AsRef<OsStr>,
+    {
+        self.inner.env(key, val);
+        self
+    }
+
+    #[must_use]
+    pub fn envs<I, K, V>(mut self, vars: I) -> Self
+    where
+        I: IntoIterator<Item = (K, V)>,
+        K: AsRef<OsStr>,
+        V: AsRef<OsStr>,
+    {
+        self.inner.envs(vars);
+        self
+    }
+
+    /// Run the command and capture its output.
+    pub fn check_output(self) -> Result<String> {
+        self.check_output_inner().map(|(_, output)| output)
+    }
 
-    /// Set the command's working directory to the repository directory.
-    fn in_repo(&mut self) -> &mut Self {
-        self.current_dir(path())
+    /// Set up the command's stdout/stderr to be piped to the reader
+    fn setup_output(&mut self) -> Result<PipeReader> {
+        let (reader, writer) = pipe()?;
+        let writer_clone = writer.try_clone()?;
+
+        self.inner.stdout(Stdio::from(writer));
+        self.inner.stderr(Stdio::from(writer_clone));
+        Ok(reader)
     }
 
     /// Run the command and capture its output.
-    fn check_output(&mut self) -> Result<String> {
-        // Set up the command's stdout to be piped, so we can capture it
+    fn check_output_inner(mut self) -> Result<(ExitStatus, String)> {
+        let error_info = format!(
+            "\"{}\" {}",
+            self.inner.get_program().to_string_lossy(),
+            self.inner
+                .get_args()
+                .map(|arg| format!("\"{}\"", arg.to_string_lossy()))
+                .join(" ")
+        );
+
         self.pre_exec();
-        self.stdout(Stdio::piped());
 
-        // Spawn the process
-        let mut child = self.spawn()?;
+        let mut reader = self.setup_output()?;
 
-        // Read the output from child.stdout into a buffer
-        let mut buffer = Vec::new();
-        child.stdout.take().unwrap().read_to_end(&mut buffer)?;
+        // Spawn the process
+        let mut child = self
+            .inner
+            .spawn()
+            .with_context(|| format!("Failed to spawn process {error_info}"))?;
 
         // Catch the exit code
         let status = child.wait()?;
         // There are commands that might fail with stdout, but we probably do not
         // want to capture
-        // If the exit code is non-zero, return an error with the command, exit code, and stderr output
+        // If the exit code is non-zero, return an error with the command, exit code, and full output
+        drop(self.inner); // Drop inner to prevent deadlock when reading
+
+        let mut buffer = Vec::new();
+        reader.read_to_end(&mut buffer).unwrap();
+        let output = String::from_utf8_lossy(&buffer);
+
         if !status.success() {
-            let stdout = String::from_utf8_lossy(&buffer);
             bail!(
-                "Command: {:?}\nfailed with exit code: {}\n\noutput:\n{}",
-                self,
+                "Command: {error_info}\nfailed with exit code: {}\n\noutput:\n{output}",
                 status.code().unwrap(),
-                stdout
             );
         }
 
         // If the command exits successfully, return the output as a string
-        Ok(String::from_utf8(buffer)?)
+        Ok((status, output.into_owned()))
     }
 
     /// Run the command and catch its exit code.
-    fn run(&mut self) -> Result<ExitStatus> {
+    pub fn run(self) -> Result<ExitStatus> {
         self.pre_exec();
-        self.status().map_err(Into::into)
+        self.check_output_inner().map(|(status, _)| status)
     }
 
-    fn check_run(&mut self) -> Result<()> {
-        let status = self.run()?;
-        if status.success() {
-            Ok(())
-        } else {
-            let exit = status.code().unwrap();
-            bail!("command: {self:?}\n  failed with exit code: {exit}")
-        }
+    pub fn check_run(self) -> Result<()> {
+        self.run().map(|_| ())
     }
 
     /// Run the command, capture its output, and display a progress bar while it's
     /// executing. Intended to be used for long-running processes with little interaction.
-    fn wait(&mut self, message: impl Into<Cow<'static, str>>) -> Result<()> {
+    pub fn wait(&mut self, message: impl Into<Cow<'static, str>>) -> Result<()> {
+        let error_info = format!(
+            "\"{}\" {}",
+            self.inner.get_program().to_string_lossy(),
+            self.inner
+                .get_args()
+                .map(|arg| format!("\"{}\"", arg.to_string_lossy()))
+                .join(" ")
+        );
+
         self.pre_exec();
 
+        let mut reader = self.setup_output()?;
+
         let progress_bar = get_progress_bar()?;
         progress_bar.set_message(message);
 
-        let result = self.output();
-        progress_bar.finish_and_clear();
+        // Spawn the process
+        let child = self
+            .inner
+            .spawn()
+            .with_context(|| format!("Failed to spawn process {error_info}"));
 
-        let Ok(output) = result else {
-            bail!("could not run command")
-        };
+        if child.is_err() {
+            progress_bar.finish_and_clear();
+        }
+        let status = child?.wait();
+        if status.is_err() {
+            progress_bar.finish_and_clear();
+        }
+        let status = status?;
+
+        if !status.success() {
+            let mut buffer = Vec::new();
+            reader.read_to_end(&mut buffer).unwrap();
+            let output = String::from_utf8_lossy(&buffer);
 
-        if output.status.success() {
-            Ok(())
-        } else {
             bail!(
-                "{}\nfailed with exit code: {}",
-                String::from_utf8(output.stdout)?,
-                output.status.code().unwrap()
-            )
+                "Command: {error_info}\nfailed with exit code: {}\n\noutput:\n{output}",
+                status.code().unwrap(),
+            );
         }
+        Ok(())
     }
 
     /// Print out a pre-execution debug message.
     fn pre_exec(&self) {
-        debug!("Executing: {self:?}");
-        if let Some(cwd) = self.get_current_dir() {
+        if let Some(cwd) = self.inner.get_current_dir() {
             debug!("  in working directory {cwd:?}");
         }
-        for (key, value) in self.get_envs() {
+        for (key, value) in self.inner.get_envs() {
             let key = key.to_string_lossy();
             if let Some(value) = value {
                 debug!("  ${key}={:?}", value.to_string_lossy());
@@ -195,17 +291,6 @@ impl CommandExt for Command {
             }
         }
     }
-
-    fn features(&mut self, features: &[String]) -> &mut Self {
-        self.arg("--no-default-features");
-        self.arg("--features");
-        if features.is_empty() {
-            self.arg(platform::default_features());
-        } else {
-            self.arg(features.join(","));
-        }
-        self
-    }
 }
 
 /// Short-cut wrapper to create a new command, feed in the args, set the working directory, and then
@@ -216,12 +301,12 @@ pub fn exec<T: AsRef<OsStr>>(
     in_repo: bool,
 ) -> Result<()> {
     let mut command = match program.strip_prefix("scripts/") {
-        Some(script) => Command::script(script),
-        None => Command::new(program),
-    };
-    command.args(args);
+        Some(script) => VDevCommand::script(script),
+        None => VDevCommand::new(program),
+    }
+    .args(args);
     if in_repo {
-        command.in_repo();
+        command = command.in_repo();
     }
     command.check_run()
 }

vdev/src/commands/build/vector.rs

@@ -1,9 +1,7 @@
-use std::process::Command;
-
 use anyhow::Result;
 use clap::Args;
 
-use crate::{app::CommandExt as _, platform};
+use crate::{app::VDevCommand, platform};
 
 /// Build the `vector` executable.
 #[derive(Args, Debug)]
@@ -23,22 +21,17 @@ pub struct Cli {
 
 impl Cli {
     pub fn exec(self) -> Result<()> {
-        let mut command = Command::new("cargo");
-        command.in_repo();
-        command.arg("build");
+        let mut command = VDevCommand::new("cargo").in_repo().arg("build");
 
         if self.release {
-            command.arg("--release");
+            command = command.arg("--release");
         }
 
-        command.features(&self.feature);
-
         let target = self.target.unwrap_or_else(platform::default_target);
-        command.args(["--target", &target]);
+        command = command.features(&self.feature).args(["--target", &target]);
 
         waiting!("Building Vector");
-        command.check_run()?;
 
-        Ok(())
+        command.check_run()
     }
 }

vdev/src/commands/crate_versions.rs

@@ -1,11 +1,11 @@
-use std::{collections::HashMap, collections::HashSet, process::Command};
+use std::{collections::HashMap, collections::HashSet};
 
 use anyhow::Result;
 use clap::Args;
 use itertools::Itertools as _;
 use regex::Regex;
 
-use crate::{app::CommandExt as _, util};
+use crate::{app::VDevCommand, util};
 
 /// Show information about crates versions pulled in by all dependencies
 #[derive(Args, Debug)]
@@ -25,7 +25,7 @@ impl Cli {
         let re_crate = Regex::new(r" (\S+) v([0-9.]+)").unwrap();
         let mut versions: HashMap<String, HashSet<String>> = HashMap::default();
 
-        for line in Command::new("cargo")
+        for line in VDevCommand::new("cargo")
             .arg("tree")
             .features(&self.feature)
             .check_output()?