From f8f3de0b6512423efc128fa697165603f93f4578 Mon Sep 17 00:00:00 2001 From: Caelan Sayler Date: Sun, 16 Jun 2024 23:10:31 +0100 Subject: [PATCH] Stop redirecting stdout on hook execution --- src/Rust/src/commands/install.rs | 16 +---- src/Rust/src/commands/start.rs | 26 ++++--- src/Rust/src/windows/util.rs | 114 ++++++++++--------------------- 3 files changed, 54 insertions(+), 102 deletions(-) diff --git a/src/Rust/src/commands/install.rs b/src/Rust/src/commands/install.rs index cbc5594d..8abc1434 100644 --- a/src/Rust/src/commands/install.rs +++ b/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, ¤t_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() { diff --git a/src/Rust/src/commands/start.rs b/src/Rust/src/commands/start.rs index 93132d75..c3e0c43c 100644 --- a/src/Rust/src/commands/start.rs +++ b/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>, 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 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(()) } diff --git a/src/Rust/src/windows/util.rs b/src/Rust/src/windows/util.rs index d243703a..6a5088cc 100644 --- a/src/Rust/src/windows/util.rs +++ b/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, ¤t_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(¤t_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, P2: AsRef>(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(exe: S, args: Vec<&str>, work_dir: P, timeout: Option) -> Result -where - S: AsRef, - P: AsRef, -{ - 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 { - 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(exe: S, args: Vec<&str>, work_dir: P) -> Result<()> -where - S: AsRef, - P: AsRef, -{ - let cmd = Process::new(exe).args(args).current_dir(work_dir).spawn()?; - let _ = unsafe { AllowSetForegroundWindow(cmd.id()) }; - Ok(()) -} - -pub fn run_process_raw_args(exe: S, args: &str, work_dir: P) -> Result<()> -where - S: AsRef, - P: AsRef, -{ - 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 { let machine = RuntimeArch::from_current_system(); if machine.is_none() { @@ -319,7 +275,7 @@ pub fn is_cpu_architecture_supported(architecture: &str) -> Result { // 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