Stop redirecting stdout on hook execution

src/Rust/src/commands/install.rs

@@ -10,7 +10,6 @@ use std::{
     env,
     fs::{self, File},
     path::{Path, PathBuf},
-    time::Duration,
 };
 use winsafe::{self as w, co};
 
@@ -184,22 +183,13 @@ fn install_impl(pkg: &bundle::BundleInfo, root_path: &PathBuf, tx: &std::sync::m
     info!("Creating new default shortcuts...");
     let _ = windows::create_default_lnks(&root_path, &app);
 
-    let ver_string = app.version.to_string();
-    info!("Starting process install hook: \"{}\" --veloapp-install {}", &main_exe_path, &ver_string);
-    let args = vec!["--veloapp-install", &ver_string];
-    if let Err(e) = windows::run_process_no_console_and_wait(&main_exe_path, args, &current_path, Some(Duration::from_secs(30))) {
+    info!("Starting process install hook");
+    if windows::run_hook(&app, &root_path, "--veloapp-install", 30) == false {
         let setup_name = format!("{} Setup {}", app.title, app.version);
-        error!("Process install hook failed: {}", e);
-        let _ = tx.send(windows::splash::MSG_CLOSE);
-        dialogs::show_warn(
-            &setup_name,
-            None,
-            format!("Installation has completed, but the application install hook failed ({}). It may not have installed correctly.", e).as_str(),
-        );
+        dialogs::show_warn(&setup_name, None, "Installation has completed, but the application install hook failed. It may not have installed correctly.");
     }
 
     let _ = tx.send(100);
-
     app.write_uninstall_entry(root_path)?;
 
     if !dialogs::get_silent() {

src/Rust/src/commands/start.rs

@@ -1,13 +1,16 @@
 use crate::{
     dialogs,
     shared::{self, bundle, OperationWait},
-    windows,
+    windows as win,
 };
 use anyhow::{anyhow, bail, Result};
+use std::os::windows::process::CommandExt;
 use std::{
     fs,
     path::{Path, PathBuf},
+    process::Command as Process,
 };
+use windows::Win32::UI::WindowsAndMessaging::AllowSetForegroundWindow;
 
 pub fn start(wait: OperationWait, exe_name: Option<&String>, exe_args: Option<Vec<&str>>, legacy_args: Option<&String>) -> Result<()> {
     if legacy_args.is_some() && exe_args.is_some() {
@@ -61,14 +64,17 @@ pub fn start(wait: OperationWait, exe_name: Option<&String>, exe_args: Option<Ve
 
     info!("About to launch: '{}' in dir '{}'", exe_to_execute.to_string_lossy(), current);
 
-    if let Some(args) = exe_args {
-        crate::windows::run_process(exe_to_execute, args, current)?;
-    } else if let Some(args) = legacy_args {
-        crate::windows::run_process_raw_args(exe_to_execute, args, current)?;
-    } else {
-        crate::windows::run_process(exe_to_execute, vec![], current)?;
-    };
+    let mut cmd = Process::new(&exe_to_execute);
+    cmd.current_dir(&current);
 
+    if let Some(args) = exe_args {
+        cmd.args(args);
+    } else if let Some(args) = legacy_args {
+        cmd.raw_arg(args);
+    }
+
+    let cmd = cmd.spawn()?;
+    let _ = unsafe { AllowSetForegroundWindow(cmd.id()) };
     Ok(())
 }
 
@@ -96,12 +102,12 @@ fn try_legacy_migration(root_dir: &PathBuf, app: &bundle::Manifest) -> Result<()
     let _ = remove_dir_all::remove_dir_all(root_dir.join("staging"));
 
     info!("Removing old shortcuts...");
-    if let Err(e) = windows::remove_all_shortcuts_for_root_dir(&root_dir) {
+    if let Err(e) = win::remove_all_shortcuts_for_root_dir(&root_dir) {
         warn!("Failed to remove shortcuts ({}).", e);
     }
 
     info!("Creating new default shortcuts...");
-    let _ = windows::create_default_lnks(&root_dir, &app);
+    let _ = win::create_default_lnks(&root_dir, &app);
 
     Ok(())
 }

src/Rust/src/windows/util.rs

@@ -2,8 +2,6 @@ use crate::shared::{self, runtime_arch::RuntimeArch};
 use anyhow::{anyhow, Result};
 use normpath::PathExt;
 use std::{
-    ffi::OsStr,
-    io::Read,
     os::windows::process::CommandExt,
     path::{Path, PathBuf},
     process::Command as Process,
@@ -20,20 +18,47 @@ use windows::{
 };
 use winsafe::{self as w, co};
 
-pub fn run_hook(app: &shared::bundle::Manifest, root_path: &PathBuf, hook_name: &str, timeout_secs: u64) {
+pub fn run_hook(app: &shared::bundle::Manifest, root_path: &PathBuf, hook_name: &str, timeout_secs: u64) -> bool {
     let sw = simple_stopwatch::Stopwatch::start_new();
     let current_path = app.get_current_path(&root_path);
     let main_exe_path = app.get_main_exe_path(&root_path);
-    info!("Running {} hook...", hook_name);
     let ver_string = app.version.to_string();
     let args = vec![hook_name, &ver_string];
-    if let Err(e) = run_process_no_console_and_wait(&main_exe_path, args, &current_path, Some(Duration::from_secs(timeout_secs))) {
-        warn!("Error running hook {}: {} (took {}ms)", hook_name, e, sw.ms());
-    } else {
-        info!("Hook executed successfully (took {}ms)", sw.ms());
+    let mut success = false;
+
+    info!("Running {} hook...", hook_name);
+    const CREATE_NO_WINDOW: u32 = 0x08000000;
+    let cmd = Process::new(&main_exe_path).args(args).current_dir(&current_path).creation_flags(CREATE_NO_WINDOW).spawn();
+
+    if let Err(e) = cmd {
+        warn!("Failed to start hook {}: {}", hook_name, e);
+        return false;
     }
+
+    let mut cmd = cmd.unwrap();
+    let _ = unsafe { AllowSetForegroundWindow(cmd.id()) };
+
+    match cmd.wait_timeout(Duration::from_secs(timeout_secs)) {
+        Ok(Some(status)) => {
+            if status.success() {
+                info!("Hook executed successfully (took {}ms)", sw.ms());
+                success = true;
+            } else {
+                warn!("Hook exited with non-zero exit code: {}", status.code().unwrap_or(0));
+            }
+        }
+        Ok(None) => {
+            let _ = cmd.kill();
+            error!("Process timed out after {}s", timeout_secs);
+        }
+        Err(e) => {
+            error!("Error waiting for process to finish: {}", e);
+        }
+    }
+
     // in case the hook left running processes
     let _ = shared::force_stop_package(&root_path);
+    success
 }
 
 pub struct MutexDropGuard {
@@ -86,7 +111,7 @@ pub fn is_sub_path<P1: AsRef<Path>, P2: AsRef<Path>>(path: P1, parent: P2) -> Re
     let parent = Path::new(&parent);
 
     // we just bail if paths are not absolute. in the cases where we use this function,
-    // we should have absolute paths from the file system (eg. iterating running processes, reading shortcuts)
+    // we should have absolute paths from the file system (e.g. iterating running processes, reading shortcuts)
     // if we receive a relative path, it's likely coming from a shortcut target/working directory
     // that we can't resolve with ExpandEnvironmentStrings
     if !path.is_absolute() || !parent.is_absolute() {
@@ -226,75 +251,6 @@ pub fn test_os_returns_true_for_everything_on_windows_11_and_below() {
     assert!(!is_os_version_or_greater("12").unwrap());
 }
 
-const CREATE_NO_WINDOW: u32 = 0x08000000;
-pub fn run_process_no_console_and_wait<S, P>(exe: S, args: Vec<&str>, work_dir: P, timeout: Option<Duration>) -> Result<String>
-where
-    S: AsRef<OsStr>,
-    P: AsRef<Path>,
-{
-    let mut cmd = Process::new(exe)
-        .args(args)
-        .current_dir(work_dir)
-        .stdout(std::process::Stdio::piped())
-        .stderr(std::process::Stdio::piped())
-        .creation_flags(CREATE_NO_WINDOW)
-        .spawn()?;
-
-    let _ = unsafe { AllowSetForegroundWindow(cmd.id()) };
-
-    fn check_process_status_and_output(status: std::process::ExitStatus, mut cmd: std::process::Child) -> Result<String> {
-        let mut stdout = cmd.stdout.take().unwrap();
-        let mut stderr = cmd.stderr.take().unwrap();
-        let mut stdout_buf = Vec::new();
-        stdout.read_to_end(&mut stdout_buf)?;
-        stderr.read_to_end(&mut stdout_buf)?;
-
-        if !status.success() {
-            warn!("Process exited with non-zero exit code: {}", status.code().unwrap_or(0));
-            if stdout_buf.len() > 0 {
-                warn!("    Output:\n{}", String::from_utf8_lossy(&stdout_buf));
-            }
-            return Err(anyhow!("Process exited with non-zero exit code: {}", status.code().unwrap_or(0)));
-        }
-
-        Ok(String::from_utf8_lossy(&stdout_buf).to_string())
-    }
-
-    if let Some(t) = timeout {
-        match cmd.wait_timeout(t) {
-            Ok(Some(status)) => check_process_status_and_output(status, cmd),
-            Ok(None) => {
-                cmd.kill()?;
-                return Err(anyhow!("Process timed out after {:?}", t));
-            }
-            Err(e) => return Err(e.into()),
-        }
-    } else {
-        let status = cmd.wait()?;
-        check_process_status_and_output(status, cmd)
-    }
-}
-
-pub fn run_process<S, P>(exe: S, args: Vec<&str>, work_dir: P) -> Result<()>
-where
-    S: AsRef<OsStr>,
-    P: AsRef<Path>,
-{
-    let cmd = Process::new(exe).args(args).current_dir(work_dir).spawn()?;
-    let _ = unsafe { AllowSetForegroundWindow(cmd.id()) };
-    Ok(())
-}
-
-pub fn run_process_raw_args<S, P>(exe: S, args: &str, work_dir: P) -> Result<()>
-where
-    S: AsRef<OsStr>,
-    P: AsRef<Path>,
-{
-    let cmd = Process::new(exe).raw_arg(args).current_dir(work_dir).spawn()?;
-    let _ = unsafe { AllowSetForegroundWindow(cmd.id()) };
-    Ok(())
-}
-
 pub fn is_cpu_architecture_supported(architecture: &str) -> Result<bool> {
     let machine = RuntimeArch::from_current_system();
     if machine.is_none() {
@@ -319,7 +275,7 @@ pub fn is_cpu_architecture_supported(architecture: &str) -> Result<bool> {
         // windows x64 only supports x86 and x64
         Ok(architecture == RuntimeArch::X86 || architecture == RuntimeArch::X64)
     } else if machine == RuntimeArch::Arm64 {
-        // windows arm64 supports x86, and arm64, and only on windows 11 does it support x64
+        // windows arm64 supports x86, and arm64, and only on Windows 11 does it support x64
         Ok(architecture == RuntimeArch::X86 || (architecture == RuntimeArch::X64 && is_win_11) || architecture == RuntimeArch::Arm64)
     } else {
         // we don't know what this is, so try installing anyway