Refactor with() to Process::run()

Author: djc

src/bin/rustup-init.rs

@@ -28,15 +28,14 @@ use rustup::cli::self_update;
 use rustup::cli::setup_mode;
 use rustup::env_var::RUST_RECURSION_COUNT_MAX;
 use rustup::is_proxyable_tools;
-use rustup::process::{with, Process};
+use rustup::process::Process;
 use rustup::utils::utils;
 
 fn main() {
     #[cfg(windows)]
     pre_rustup_main_init();
 
-    let process = Process::os();
-    with(process, || match maybe_trace_rustup() {
+    Process::os().run(|| match maybe_trace_rustup() {
         Err(e) => {
             common::report_error(&e);
             std::process::exit(1);

src/cli/self_update.rs

@@ -1306,18 +1306,19 @@ mod tests {
     use crate::cli::common;
     use crate::dist::dist::PartialToolchainDesc;
     use crate::test::{test_dir, with_rustup_home, Env};
-    use crate::{for_host, process};
+    use crate::{for_host, process::TestProcess};
 
     #[test]
     fn default_toolchain_is_stable() {
         with_rustup_home(|home| {
             let mut vars = HashMap::new();
             home.apply(&mut vars);
-            let tp = process::TestProcess {
+            let tp = TestProcess {
                 vars,
                 ..Default::default()
             };
-            process::with(tp.clone().into(), || -> Result<()> {
+
+            tp.clone().run(|| -> Result<()> {
                 // TODO: we could pass in a custom cfg to get notification
                 // callbacks rather than output to the tp sink.
                 let mut cfg = common::set_globals(false, false).unwrap();
@@ -1339,8 +1340,10 @@ mod tests {
                     .unwrap() // result
                     .unwrap() // option
                 );
+
                 Ok(())
             })?;
+
             assert_eq!(
                 for_host!(
                     r"info: profile set to 'default'
@@ -1360,11 +1363,12 @@ info: default host triple is {0}
         let cargo_home = root_dir.path().join("cargo");
         let mut vars = HashMap::new();
         vars.env("CARGO_HOME", cargo_home.to_string_lossy().to_string());
-        let tp = process::TestProcess {
+        let tp = TestProcess {
             vars,
             ..Default::default()
         };
-        process::with(tp.into(), || -> Result<()> {
+
+        tp.run(|| -> Result<()> {
             super::install_bins().unwrap();
             Ok(())
         })

src/cli/self_update/windows.rs

@@ -13,7 +13,7 @@ use super::common;
 use super::{install_bins, InstallOpts};
 use crate::cli::download_tracker::DownloadTracker;
 use crate::dist::dist::TargetTriple;
-use crate::process::{self, Process};
+use crate::process::Process;
 use crate::utils::utils;
 use crate::utils::Notification;
 
@@ -712,7 +712,8 @@ mod tests {
 
     use rustup_macros::unit_test as test;
 
-    use crate::process;
+    #[cfg(feature = "test")]
+    use crate::process::TestProcess;
     use crate::test::with_saved_path;
 
     fn wide(str: &str) -> Vec<u16> {
@@ -753,9 +754,9 @@ mod tests {
     #[test]
     fn windows_path_regkey_type() {
         // per issue #261, setting PATH should use REG_EXPAND_SZ.
-        let tp = process::TestProcess::default();
+        let tp = TestProcess::default();
         with_saved_path(&mut || {
-            process::with(tp.clone().into(), || {
+            tp.clone().run(|| {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
@@ -783,9 +784,9 @@ mod tests {
         use std::io;
         // during uninstall the PATH key may end up empty; if so we should
         // delete it.
-        let tp = process::TestProcess::default();
+        let tp = TestProcess::default();
         with_saved_path(&mut || {
-            process::with(tp.clone().into(), || {
+            tp.clone().run(|| {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
@@ -818,15 +819,15 @@ mod tests {
     #[test]
     fn windows_doesnt_mess_with_a_non_string_path() {
         // This writes an error, so we want a sink for it.
-        let tp = process::TestProcess {
+        let tp = TestProcess {
             vars: [("HOME".to_string(), "/unused".to_string())]
                 .iter()
                 .cloned()
                 .collect(),
             ..Default::default()
         };
         with_saved_path(&mut || {
-            process::with(tp.clone().into(), || {
+            tp.clone().run(|| {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
@@ -853,9 +854,9 @@ mod tests {
     #[test]
     fn windows_treat_missing_path_as_empty() {
         // during install the PATH key may be missing; treat it as empty
-        let tp = process::TestProcess::default();
+        let tp = TestProcess::default();
         with_saved_path(&mut || {
-            process::with(tp.clone().into(), || {
+            tp.clone().run({
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)

src/diskio/test.rs

@@ -7,7 +7,7 @@ use rustup_macros::unit_test as test;
 use crate::test::test_dir;
 
 use super::{get_executor, Executor, Item, Kind};
-use crate::process;
+use crate::process::TestProcess;
 
 impl Item {
     /// The length of the file, for files (for stats)
@@ -23,11 +23,11 @@ fn test_incremental_file(io_threads: &str) -> Result<()> {
     let work_dir = test_dir()?;
     let mut vars = HashMap::new();
     vars.insert("RUSTUP_IO_THREADS".to_string(), io_threads.to_string());
-    let tp = process::TestProcess {
+    TestProcess {
         vars,
         ..Default::default()
-    };
-    process::with(tp.into(), || -> Result<()> {
+    }
+    .run(|| -> Result<()> {
         let mut written = 0;
         let mut file_finished = false;
         let mut io_executor: Box<dyn Executor> = get_executor(None, 32 * 1024 * 1024)?;
@@ -96,11 +96,11 @@ fn test_complete_file(io_threads: &str) -> Result<()> {
     let work_dir = test_dir()?;
     let mut vars = HashMap::new();
     vars.insert("RUSTUP_IO_THREADS".to_string(), io_threads.to_string());
-    let tp = process::TestProcess {
+    TestProcess {
         vars,
         ..Default::default()
-    };
-    process::with(tp.into(), || -> Result<()> {
+    }
+    .run(|| -> Result<()> {
         let mut io_executor: Box<dyn Executor> = get_executor(None, 32 * 1024 * 1024)?;
         let mut chunk = io_executor.get_buffer(10);
         chunk.extend(b"0123456789");

src/dist/manifestation/tests.rs

@@ -25,7 +25,7 @@ use crate::{
         temp, Notification,
     },
     errors::RustupError,
-    process,
+    process::TestProcess,
     test::mock::{dist::*, MockComponentBuilder, MockFile, MockInstallerBuilder},
     utils::{raw as utils_raw, utils},
 };
@@ -556,16 +556,13 @@ fn setup_from_dist_server(
         },
     };
 
-    process::with(
-        process::TestProcess::new(
-            env::current_dir().unwrap(),
-            &["rustup"],
-            HashMap::default(),
-            "",
-        )
-        .into(),
-        || f(url, &toolchain, &prefix, &download_cfg, &tmp_cx),
-    );
+    TestProcess::new(
+        env::current_dir().unwrap(),
+        &["rustup"],
+        HashMap::default(),
+        "",
+    )
+    .run(|| f(url, &toolchain, &prefix, &download_cfg, &tmp_cx));
 }
 
 fn initial_install(comps: Compressions) {

src/env_var.rs

@@ -44,7 +44,7 @@ mod tests {
     use rustup_macros::unit_test as test;
 
     use super::*;
-    use crate::process;
+    use crate::process::TestProcess;
     use crate::test::{with_saved_path, Env};
 
     #[test]
@@ -54,12 +54,12 @@ mod tests {
             "PATH",
             env::join_paths(["/home/a/.cargo/bin", "/home/b/.cargo/bin"].iter()).unwrap(),
         );
-        let tp = process::TestProcess {
+        let tp = TestProcess {
             vars,
             ..Default::default()
         };
         with_saved_path(&mut || {
-            process::with(tp.clone().into(), || {
+            tp.clone().run(|| {
                 let mut path_entries = vec![];
                 let mut cmd = Command::new("test");
 

src/process/mod.rs

@@ -46,6 +46,30 @@ impl Process {
         })
     }
 
+    /// Run a function in the context of a process definition.
+    ///
+    /// If the function panics, the process definition *in that thread* is cleared
+    /// by an implicitly installed global panic hook.
+    pub fn run<R>(self, f: impl FnOnce() -> R) -> R {
+        HOOK_INSTALLED.call_once(|| {
+            let orig_hook = panic::take_hook();
+            panic::set_hook(Box::new(move |info| {
+                clear_process();
+                orig_hook(info);
+            }));
+        });
+
+        PROCESS.with(|p| {
+            if let Some(old_p) = &*p.borrow() {
+                panic!("current process already set {old_p:?}");
+            }
+            *p.borrow_mut() = Some(self);
+            let result = f();
+            *p.borrow_mut() = None;
+            result
+        })
+    }
+
     pub fn name(&self) -> Option<String> {
         let arg0 = match self.var("RUSTUP_FORCE_ARG0") {
             Ok(v) => Some(v),
@@ -171,33 +195,6 @@ impl From<TestProcess> for Process {
 
 static HOOK_INSTALLED: Once = Once::new();
 
-/// Run a function in the context of a process definition.
-///
-/// If the function panics, the process definition *in that thread* is cleared
-/// by an implicitly installed global panic hook.
-pub fn with<F, R>(process: Process, f: F) -> R
-where
-    F: FnOnce() -> R,
-{
-    HOOK_INSTALLED.call_once(|| {
-        let orig_hook = panic::take_hook();
-        panic::set_hook(Box::new(move |info| {
-            clear_process();
-            orig_hook(info);
-        }));
-    });
-
-    PROCESS.with(|p| {
-        if let Some(old_p) = &*p.borrow() {
-            panic!("current process already set {old_p:?}");
-        }
-        *p.borrow_mut() = Some(process);
-        let result = f();
-        *p.borrow_mut() = None;
-        result
-    })
-}
-
 /// Internal - for the panic hook only
 fn clear_process() {
     PROCESS.with(|p| p.replace(None));
@@ -246,6 +243,11 @@ impl TestProcess {
             stderr: Arc::new(Mutex::new(Vec::new())),
         }
     }
+
+    pub(crate) fn run<R>(self, f: impl FnOnce() -> R) -> R {
+        Process::from(self).run(f)
+    }
+
     fn new_id() -> u64 {
         let low_bits: u64 = std::process::id() as u64;
         let mut rng = thread_rng();
@@ -277,7 +279,7 @@ mod tests {
 
     use rustup_macros::unit_test as test;
 
-    use super::{with, Process, TestProcess};
+    use super::{Process, TestProcess};
 
     #[test]
     fn test_instance() {
@@ -287,7 +289,8 @@ mod tests {
             HashMap::default(),
             "",
         );
-        with(proc.clone().into(), || {
+
+        proc.clone().run(|| {
             let cur = Process::get();
             assert_eq!(proc.id, cur.id(), "{:?} != {:?}", proc, cur)
         });

src/process/terminalsource.rs

@@ -251,11 +251,11 @@ mod tests {
         fn assert_color_choice(env_val: &str, stream: StreamSelector, color_choice: ColorChoice) {
             let mut vars = HashMap::new();
             vars.env("RUSTUP_TERM_COLOR", env_val);
-            let tp = process::TestProcess {
+            process::TestProcess {
                 vars,
                 ..Default::default()
-            };
-            process::with(tp.into(), || {
+            }
+            .run(|| {
                 let term = ColorableTerminal::new(stream);
                 let inner = term.inner.lock().unwrap();
                 assert!(matches!(

src/test.rs

@@ -17,7 +17,7 @@ use anyhow::Result;
 
 pub use crate::cli::self_update::test::{get_path, with_saved_path};
 use crate::dist::dist::TargetTriple;
-use crate::process;
+use crate::process::TestProcess;
 
 // Things that can have environment variables applied to them.
 pub trait Env {
@@ -124,8 +124,7 @@ pub fn this_host_triple() -> String {
         // For windows, this host may be different to the target: we may be
         // building with i686 toolchain, but on an x86_64 host, so run the
         // actual detection logic and trust it.
-        let tp = process::TestProcess::default();
-        return process::with(tp.into(), || TargetTriple::from_host().unwrap().to_string());
+        return TestProcess::default().run(|| TargetTriple::from_host().unwrap().to_string());
     }
     let arch = if cfg!(target_arch = "x86") {
         "i686"

src/test/mock/clitools.rs

@@ -19,7 +19,7 @@ use once_cell::sync::Lazy;
 use url::Url;
 
 use crate::cli::rustup_mode;
-use crate::process;
+use crate::process::TestProcess;
 use crate::test as rustup_test;
 use crate::test::const_dist_dir;
 use crate::test::this_host_triple;
@@ -721,13 +721,13 @@ impl Config {
                     .into_boxed_str(),
             );
         }
-        let tp = process::TestProcess::new(&*self.workdir.borrow(), &arg_strings, vars, "");
-        let process_res = process::with(tp.clone().into(), rustup_mode::main);
+        let tp = TestProcess::new(&*self.workdir.borrow(), &arg_strings, vars, "");
+        let process_res = tp.clone().run(rustup_mode::main);
         // convert Err's into an ec
         let ec = match process_res {
             Ok(process_res) => process_res,
             Err(e) => {
-                process::with(tp.clone().into(), || crate::cli::common::report_error(&e));
+                tp.clone().run(|| crate::cli::common::report_error(&e));
                 utils::ExitCode(1)
             }
         };