Merge pull request #174 from J-ZhengLi/dev-zhengli

fix command output redirecting issue for CLI

Cargo.toml

@@ -56,6 +56,7 @@ fern.workspace = true
 chrono = "0.4.38"
 semver = "1.0.23"
 self-replace = "1"
+os_pipe = "1.2.1"
 
 [target."cfg(windows)".dependencies]
 winreg = "0.52.0"

src/core/custom_instructions/buildtools.rs

@@ -21,7 +21,7 @@ pub(super) fn install(path: &Path, config: &InstallConfiguration) -> Result<Vec<
     let buildtools_exe = any_existing_child_path(path, &["vs_BuildTools.exe", "vs_buildtools.exe"])
         .ok_or_else(|| anyhow!("unable to find the build tools installer binary."))?;
 
-    let mut cmd = vec![
+    let mut args = vec![
         "--wait",
         "--noWeb",
         "--nocache",
@@ -30,8 +30,8 @@ pub(super) fn install(path: &Path, config: &InstallConfiguration) -> Result<Vec<
         "--focusedUi",
     ];
     for component in required_components() {
-        cmd.push("--add");
-        cmd.push(component.component_id());
+        args.push("--add");
+        args.push(component.component_id());
     }
 
     // Step 2: Make a copy of this installer to the `tools` directory,
@@ -42,7 +42,9 @@ pub(super) fn install(path: &Path, config: &InstallConfiguration) -> Result<Vec<
 
     // Step 3: Invoke the install command.
     info!("{}", t!("installing_msvc_info"));
-    let exit_code: VSExitCode = utils::Command::new(buildtools_exe).args(&cmd).run_with_ret_code()?.into();
+    let mut cmd = utils::cmd!(buildtools_exe);
+    cmd.args(args);
+    let exit_code: VSExitCode = utils::execute_for_ret_code(cmd)?.into();
     match exit_code {
         VSExitCode::Success => {
             info!("{}", t!("msvc_installed"));

src/core/custom_instructions/vscode.rs

@@ -68,11 +68,7 @@ impl VSCodeInstaller<'_> {
                 utils::path_to_str(&shortcut_path)?,
                 utils::path_to_str(&target_path)?,
             );
-            if utils::Command::new("powershell")
-                .arg(weird_powershell_cmd)
-                .run()
-                .is_err()
-            {
+            if utils::run!("powershell", weird_powershell_cmd).is_err() {
                 warn!(
                     "unable to create a shortcut for '{}', skipping...",
                     self.tool_name

src/core/rustup.rs

@@ -49,14 +49,13 @@ impl ToolchainInstaller {
             args.push("--component");
             args.extend(components);
         }
-        if let Some(local_server) = manifest.offline_dist_server()? {
-            utils::Command::new(rustup)
-                .args(&args)
-                .env(RUSTUP_DIST_SERVER, local_server.as_str())
-                .run()
+        let mut cmd = if let Some(local_server) = manifest.offline_dist_server()? {
+            utils::cmd!([RUSTUP_DIST_SERVER=local_server.as_str()] rustup)
         } else {
-            utils::Command::new(rustup).args(&args).run()
-        }
+            utils::cmd!(rustup)
+        };
+        cmd.args(args);
+        utils::execute(cmd)
     }
 
     /// Install rust toolchain & components via rustup.
@@ -94,25 +93,15 @@ impl ToolchainInstaller {
         manifest: &ToolsetManifest,
     ) -> Result<()> {
         let rustup = ensure_rustup(config, manifest)?;
-
         let tc_ver = manifest.rust_version();
-        utils::Command::new(&rustup)
-            .args(&["toolchain", "add", tc_ver])
-            .run()?;
-        utils::Command::new(&rustup)
-            .args(&["default", tc_ver])
-            .run()?;
-        Ok(())
+
+        utils::run!(&rustup, "toolchain", "add", tc_ver)
     }
 
     // Rustup self uninstall all the components and toolchains.
     pub(crate) fn remove_self(&self, config: &UninstallConfiguration) -> Result<()> {
         let rustup = config.cargo_bin().join(RUSTUP);
-        utils::Command::new(rustup)
-            .args(&["self", "uninstall", "-y"])
-            .env(CARGO_HOME, config.cargo_home())
-            .env(RUSTUP_HOME, config.rustup_home())
-            .run()
+        utils::run!([CARGO_HOME=config.cargo_home(), RUSTUP_HOME=config.rustup_home()] rustup, "self", "uninstall", "-y")
     }
 }
 
@@ -171,5 +160,7 @@ fn install_rustup(rustup_init: &PathBuf) -> Result<()> {
         env!("TARGET"),
         "-vy",
     ];
-    utils::Command::new(rustup_init).args(&args).run()
+    let mut cmd = utils::cmd!(rustup_init);
+    cmd.args(args);
+    utils::execute(cmd)
 }

src/core/tools.rs

@@ -207,11 +207,9 @@ fn cargo_install_or_uninstall(op: &str, args: &[&str], cargo_home: &Path) -> Res
     cargo_bin.push("bin");
     cargo_bin.push(utils::exe!("cargo"));
 
-    utils::Command::new(cargo_bin)
-        .arg(op)
-        .args(args)
-        .env(CARGO_HOME, cargo_home)
-        .run()
+    let mut cmd = utils::cmd!([CARGO_HOME=cargo_home] cargo_bin, op);
+    cmd.args(args);
+    utils::execute(cmd)
 }
 
 /// Move one path (file/dir) to a new folder with `name` under tools dir.
@@ -314,11 +312,7 @@ impl Plugin {
                                 program = program
                             )
                         );
-                        match utils::Command::new(program)
-                            .arg(arg_opt)
-                            .arg(plugin_path)
-                            .run()
-                        {
+                        match utils::run!(program, arg_opt, plugin_path) {
                             Ok(()) => continue,
                             // Ignore error when uninstalling.
                             Err(_) if uninstall => {

src/core/try_it.rs

@@ -38,11 +38,8 @@ pub fn try_it(path: Option<&Path>) -> Result<()> {
         .iter()
         .find_map(|p| utils::cmd_exist(p).then_some(*p))
         .unwrap_or(file_explorer);
-    // Try to open the project, but don't do anything if it fails cuz it's pretty noisy.
-    _ = utils::Command::new_shell_command(program)
-        .arg(example_dir)
-        .run();
-
+    // Try to open the project, but don't do anything if it fails cuz it's not critical.
+    _ = utils::run!(program, example_dir);
     Ok(())
 }
 

src/utils/process.rs

@@ -1,131 +1,105 @@
 use log::{error, info, warn};
-use std::ffi::OsStr;
 use std::io::{BufRead, BufReader};
-use std::process::{Command as StdCommand, ExitStatus, Stdio};
-use std::sync::Mutex;
+use std::process::{Command, ExitStatus};
 use std::{env, io};
 
 use anyhow::Result;
 
-use super::to_string_lossy;
-
-/// The complete commands in string form, used in error output.
-static COMMAND_STRING: Mutex<Vec<String>> = Mutex::new(Vec::new());
+/// Convenient macro to run a [`Command`], check [`cmd`] for help of the syntax.
+macro_rules! run {
+    ([$($key:tt = $val:expr),*] $program:expr $(, $arg:expr )* $(,)?) => {{
+        let cmd__ = $crate::utils::cmd!([$($key=$val),*] $program $(,$arg)*);
+        $crate::utils::execute(cmd__).map(|_| ())
+    }};
+    ($program:expr $(, $arg:expr )* $(,)?) => {
+        $crate::utils::run!([] $program $(, $arg)*)
+    };
+}
+pub(crate) use run;
+
+/// Convenient macro to create a [`Command`], using shell-like command syntax.
+///
+/// # Example
+/// ```ignore
+/// # use rim::utils::cmd;
+/// cmd!("echo", "$HOME/.profile");
+///
+/// let program = "cargo";
+/// cmd!(program, "build", "--release");
+///
+/// // With env vars
+/// cmd!(["FOO"="foo", "BAR"="bar"] program, "cargo", "build");
+/// ```
+macro_rules! cmd {
+    ([$($key:tt = $val:expr),*] $program:expr $(, $arg:expr )* $(,)?) => {{
+        let mut cmd__ = std::process::Command::new($program);
+        $(cmd__.arg($arg);)*
+        $(cmd__.env($key, $val);)*
+        cmd__
+    }};
+    ($program:expr) => {
+        std::process::Command::new($program)
+    };
+    ($program:expr $(, $arg:expr )* $(,)?) => {
+        $crate::utils::cmd!([] $program $(, $arg)*)
+    };
+}
+pub(crate) use cmd;
 
-cfg_if::cfg_if! {
-    if #[cfg(windows)] {
-        const SHELL: &str = "cmd.exe";
-        const START_ARG: &str = "/C";
-    } else {
-        const SHELL: &str = "sh";
-        const START_ARG: &str = "-c";
-    }
+pub(crate) fn execute(cmd: Command) -> Result<()> {
+    execute_command(cmd, true).map(|_| ())
 }
 
-/// A [`std::process::Command`] wrapper type that supports better error reporting.
-pub struct Command {
-    cmd_: StdCommand,
+// Only used for `windows` for now, but... who knows.
+#[allow(unused)]
+pub(crate) fn execute_for_ret_code(cmd: Command) -> Result<i32> {
+    execute_command(cmd, false)
 }
 
-impl Command {
-    pub fn new<P: AsRef<OsStr>>(program: P) -> Self {
-        let mut guard = COMMAND_STRING.lock().unwrap();
-        *guard = vec![to_string_lossy(&program)];
+fn execute_command(mut cmd: Command, expect_success: bool) -> Result<i32> {
+    let (mut reader, stdout) = os_pipe::pipe()?;
+    let stderr = stdout.try_clone()?;
 
-        Self {
-            cmd_: StdCommand::new(program),
+    cfg_if::cfg_if! {
+        if #[cfg(windows)] {
+            use std::os::windows::process::CommandExt;
+            // Prevent CMD window popup
+            use winapi::um::winbase::CREATE_NO_WINDOW;
+            let mut child = cmd
+                .creation_flags(CREATE_NO_WINDOW)
+                .stdout(stdout)
+                .stderr(stderr)
+                .spawn()?;
+        } else {
+            let mut child = cmd
+                .stdout(stdout)
+                .stderr(stderr)
+                .spawn()?;
         }
     }
-    /// Create a command that will be execute using a separated shell.
-    ///
-    /// This prevents a specific program being shut down because the main process exists.
-    pub fn new_shell_command<P: AsRef<OsStr>>(program: P) -> Self {
-        let mut guard = COMMAND_STRING.lock().unwrap();
-        *guard = vec![to_string_lossy(&program)];
-
-        let mut inner = StdCommand::new(SHELL);
-        inner.arg(START_ARG).arg(program);
-
-        Self { cmd_: inner }
-    }
-    pub fn arg<A: AsRef<OsStr>>(&mut self, arg: A) -> &mut Self {
-        let mut guard = COMMAND_STRING.lock().unwrap();
-        (*guard).push(to_string_lossy(&arg));
 
-        self.cmd_.arg(arg);
-        self
-    }
-    pub fn args<S: AsRef<OsStr>>(&mut self, args: &[S]) -> &mut Self {
-        let mut guard = COMMAND_STRING.lock().unwrap();
-        for arg in args {
-            (*guard).push(to_string_lossy(arg));
-        }
+    let cmd_content = cmd_to_string(cmd);
+    output_to_log(Some(&mut reader));
 
-        self.cmd_.args(args);
-        self
-    }
-    pub fn env<K, V>(&mut self, key: K, val: V) -> &mut Self
-    where
-        K: AsRef<OsStr>,
-        V: AsRef<OsStr>,
-    {
-        let mut guard = COMMAND_STRING.lock().unwrap();
-        (*guard).insert(
-            0,
-            format!("{}={}", to_string_lossy(&key), to_string_lossy(&val)),
+    let status = child.wait()?;
+    let ret_code = get_ret_code(&status);
+    if expect_success && !status.success() {
+        anyhow::bail!(
+            "programm exited with code {ret_code}. \n\
+            Command: {cmd_content}"
         );
-
-        self.cmd_.env(key, val);
-        self
-    }
-    pub fn run(&mut self) -> Result<()> {
-        self.execute_command(true)?;
-        Ok(())
-    }
-
-    pub fn run_with_ret_code(&mut self) -> Result<i32> {
-        self.execute_command(false)
+    } else {
+        Ok(ret_code)
     }
+}
 
-    fn execute_command(&mut self, expect_success: bool) -> Result<i32> {
-        cfg_if::cfg_if! {
-            if #[cfg(windows)] {
-                use std::os::windows::process::CommandExt;
-                // Prevent CMD window popup
-                use winapi::um::winbase::CREATE_NO_WINDOW;
-                let mut child = self.cmd_
-                    .creation_flags(CREATE_NO_WINDOW)
-                    .stdout(Stdio::piped())
-                    .stderr(Stdio::piped())
-                    .spawn()?;
-            } else {
-                let mut child = self.cmd_
-                    .stdout(Stdio::piped())
-                    .stderr(Stdio::piped())
-                    .spawn()?;
-            }
-        }
-
-        // FIXME: (J-ZhengLi) somehow this doesn't stream output anymore, wtf happend?
-        // I clearly remember it did work, I **TESTED** it! I made sure it worked
-        // then I made a PR specifically for it (https://github.com/J-ZhengLi/rim/pull/159).
-        // So how the F that this doesn't work anymore...
-        output_to_log(child.stdout.as_mut());
-        output_to_log(child.stderr.as_mut());
-
-        let status = child.wait()?;
-        let ret_code = get_ret_code(&status);
-        if expect_success && !status.success() {
-            let command = COMMAND_STRING.lock().unwrap();
-            anyhow::bail!(
-                "programm exited with code {ret_code}. \n\
-                Command: {}",
-                (*command).join(" "),
-            );
-        } else {
-            Ok(ret_code)
-        }
-    }
+/// Consumes a [`Command`] and turn it into string using debug formatter.
+///
+/// It is important to call this before reading the output from `os_pipe`,
+/// otherwise there will be deadlock. More information can be found in
+/// [`os_pipe`'s documentation](https://docs.rs/os_pipe/1.2.1/os_pipe/#common-deadlocks-related-to-pipes)
+fn cmd_to_string(cmd: Command) -> String {
+    format!("{cmd:?}")
 }
 
 fn get_ret_code(status: &ExitStatus) -> i32 {