WIP

Cargo.toml

@@ -56,6 +56,7 @@ effective-limits = "0.5.5"
 enum-map = "2.5.0"
 flate2 = "1"
 fs_at.workspace = true
+futures.workspace = true
 git-testament = "0.2"
 home = "0.5.4"
 lazy_static.workspace = true
@@ -174,6 +175,7 @@ members = ["download", "rustup-macros"]
 anyhow = "1.0.69"
 derivative = "2.2.0"
 fs_at = "0.1.6"
+futures = "0.3"
 lazy_static = "1"
 once_cell = "1.17.1"
 opentelemetry = { version = "0.18.0", features = ["rt-tokio"] }

src/cli/self_update.rs

@@ -1282,8 +1282,6 @@ pub(crate) fn valid_self_update_modes() -> String {
 mod tests {
     use std::collections::HashMap;
 
-    use anyhow::Result;
-
     use rustup_macros::unit_test as test;
 
     use crate::cli::common;
@@ -1292,15 +1290,15 @@ mod tests {
     use crate::{currentprocess, for_host};
 
     #[test]
-    fn default_toolchain_is_stable() {
-        with_rustup_home(|home| {
+    async fn default_toolchain_is_stable() {
+        with_rustup_home(|home| async move {
             let mut vars = HashMap::new();
             home.apply(&mut vars);
             let tp = Box::new(currentprocess::TestProcess {
                 vars,
                 ..Default::default()
             });
-            currentprocess::with(tp.clone(), || -> Result<()> {
+            currentprocess::with_tokio(tp.clone(), async {
                 // 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();
@@ -1322,23 +1320,24 @@ mod tests {
                     .unwrap() // result
                     .unwrap() // option
                 );
-                Ok(())
+                Ok::<(), anyhow::Error>(())
             })?;
             assert_eq!(
                 for_host!(
                     r"info: profile set to 'default'
 info: default host triple is {0}
 "
                 ),
-                &String::from_utf8(tp.get_stderr()).unwrap()
+                String::from_utf8(tp.get_stderr()).unwrap()
             );
             Ok(())
         })
+        .await
         .unwrap();
     }
 
     #[test]
-    fn install_bins_creates_cargo_home() {
+    async fn install_bins_creates_cargo_home() {
         let root_dir = test_dir().unwrap();
         let cargo_home = root_dir.path().join("cargo");
         let mut vars = HashMap::new();
@@ -1347,9 +1346,9 @@ info: default host triple is {0}
             vars,
             ..Default::default()
         });
-        currentprocess::with(tp, || -> Result<()> {
+        currentprocess::with_tokio(tp, async {
             super::install_bins().unwrap();
-            Ok(())
+            Ok::<(), anyhow::Error>(())
         })
         .unwrap();
         assert!(cargo_home.exists());

src/cli/self_update/windows.rs

@@ -745,11 +745,11 @@ mod tests {
     }
 
     #[test]
-    fn windows_path_regkey_type() {
+    async fn windows_path_regkey_type() {
         // per issue #261, setting PATH should use REG_EXPAND_SZ.
         let tp = Box::new(currentprocess::TestProcess::default());
         with_saved_path(&mut || {
-            currentprocess::with(tp.clone(), || {
+            currentprocess::with_tokio(tp.clone(), async {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
@@ -773,13 +773,13 @@ mod tests {
     }
 
     #[test]
-    fn windows_path_delete_key_when_empty() {
+    async fn windows_path_delete_key_when_empty() {
         use std::io;
         // during uninstall the PATH key may end up empty; if so we should
         // delete it.
         let tp = Box::new(currentprocess::TestProcess::default());
         with_saved_path(&mut || {
-            currentprocess::with(tp.clone(), || {
+            currentprocess::with_tokio(tp.clone(), async {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
@@ -810,7 +810,7 @@ mod tests {
     }
 
     #[test]
-    fn windows_doesnt_mess_with_a_non_string_path() {
+    async fn windows_doesnt_mess_with_a_non_string_path() {
         // This writes an error, so we want a sink for it.
         let tp = Box::new(currentprocess::TestProcess {
             vars: [("HOME".to_string(), "/unused".to_string())]
@@ -820,7 +820,7 @@ mod tests {
             ..Default::default()
         });
         with_saved_path(&mut || {
-            currentprocess::with(tp.clone(), || {
+            currentprocess::with_tokio(tp.clone(), async {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
@@ -845,11 +845,11 @@ mod tests {
     }
 
     #[test]
-    fn windows_treat_missing_path_as_empty() {
+    async fn windows_treat_missing_path_as_empty() {
         // during install the PATH key may be missing; treat it as empty
         let tp = Box::new(currentprocess::TestProcess::default());
         with_saved_path(&mut || {
-            currentprocess::with(tp.clone(), || {
+            currentprocess::with_tokio(tp.clone(), async {
                 let root = RegKey::predef(HKEY_CURRENT_USER);
                 let environment = root
                     .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)

src/currentprocess.rs

@@ -310,17 +310,17 @@ mod tests {
 
     use rustup_macros::unit_test as test;
 
-    use super::{process, with, ProcessSource, TestProcess};
+    use super::{process, with_tokio, ProcessSource, TestProcess};
 
     #[test]
-    fn test_instance() {
+    async fn test_instance() {
         let proc = TestProcess::new(
             env::current_dir().unwrap(),
             &["foo", "bar", "baz"],
             HashMap::default(),
             "",
         );
-        with(Box::new(proc.clone()), || {
+        with_tokio(Box::new(proc.clone()), async {
             assert_eq!(proc.id(), process().id(), "{:?} != {:?}", proc, process())
         });
     }

src/diskio/test.rs

@@ -19,15 +19,15 @@ impl Item {
     }
 }
 
-fn test_incremental_file(io_threads: &str) -> Result<()> {
+async 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 = Box::new(currentprocess::TestProcess {
         vars,
         ..Default::default()
     });
-    currentprocess::with(tp, || -> Result<()> {
+    currentprocess::with_tokio(tp, async {
         let mut written = 0;
         let mut file_finished = false;
         let mut io_executor: Box<dyn Executor> = get_executor(None, 32 * 1024 * 1024)?;
@@ -83,7 +83,7 @@ fn test_incremental_file(io_threads: &str) -> Result<()> {
         }
 
         assert_eq!(io_executor.buffer_used(), 0);
-        Ok(())
+        Ok::<(), anyhow::Error>(())
     })?;
     // We should be able to read back the file
     assert_eq!(
@@ -93,15 +93,15 @@ fn test_incremental_file(io_threads: &str) -> Result<()> {
     Ok(())
 }
 
-fn test_complete_file(io_threads: &str) -> Result<()> {
+async 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 = Box::new(currentprocess::TestProcess {
         vars,
         ..Default::default()
     });
-    currentprocess::with(tp, || -> Result<()> {
+    currentprocess::with_tokio(tp, async {
         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");
@@ -147,7 +147,7 @@ fn test_complete_file(io_threads: &str) -> Result<()> {
             // no more work should be outstanding
             unreachable!();
         }
-        Ok(())
+        Ok::<(), anyhow::Error>(())
     })?;
     // We should be able to read back the file with correct content
     assert_eq!(
@@ -158,21 +158,21 @@ fn test_complete_file(io_threads: &str) -> Result<()> {
 }
 
 #[test]
-fn test_incremental_file_immediate() {
-    test_incremental_file("1").unwrap()
+async fn test_incremental_file_immediate() {
+    test_incremental_file("1").await.unwrap()
 }
 
 #[test]
-fn test_incremental_file_threaded() {
-    test_incremental_file("2").unwrap()
+async fn test_incremental_file_threaded() {
+    test_incremental_file("2").await.unwrap()
 }
 
 #[test]
-fn test_complete_file_immediate() {
-    test_complete_file("1").unwrap()
+async fn test_complete_file_immediate() {
+    test_complete_file("1").await.unwrap()
 }
 
 #[test]
-fn test_complete_file_threaded() {
-    test_complete_file("2").unwrap()
+async fn test_complete_file_threaded() {
+    test_complete_file("2").await.unwrap()
 }

src/env_var.rs

@@ -48,7 +48,7 @@ mod tests {
     use crate::test::{with_saved_path, Env};
 
     #[test]
-    fn prepend_unique_path() {
+    async fn prepend_unique_path() {
         let mut vars = HashMap::new();
         vars.env(
             "PATH",
@@ -59,7 +59,7 @@ mod tests {
             ..Default::default()
         });
         with_saved_path(&mut || {
-            currentprocess::with(tp.clone(), || {
+            currentprocess::with_tokio(tp.clone(), async {
                 let mut path_entries = vec![];
                 let mut cmd = Command::new("test");
 

src/test.rs

@@ -8,9 +8,13 @@ use std::env;
 use std::ffi::OsStr;
 use std::fmt;
 use std::fs;
+#[cfg(test)]
+use std::future::Future;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::Command;
+#[cfg(test)]
+use std::sync::Arc;
 
 #[cfg(test)]
 use anyhow::Result;
@@ -111,21 +115,23 @@ fn tempdir_in_with_prefix<P: AsRef<Path>>(path: P, prefix: &str) -> io::Result<P
 /// triples?
 ///
 /// NOTE: This *cannot* be called within a currentprocess context as it creates
-/// its own context on Windows hosts. This is partly by chance but also partly
-/// deliberate: If you need the host triple, or to call for_host(), you can do
-/// so outside of calls to run() or unit test code that runs in a currentprocess
-/// context.
+/// its own context on Windows hosts. This is partly by chance (from_host()
+/// consults RUSTUP_OVERRIDE_HOST_TRIPLE) but also partly deliberate: If you
+/// need the host triple, or to call for_host(), you can do so outside of calls
+/// to run() or unit test code that runs in a currentprocess context.
 ///
 /// IF it becomes very hard to workaround that, then we can either make a second
 /// this_host_triple that doesn't make its own currentprocess or use
 /// TargetTriple::from_host() from within the currentprocess context as needed.
-pub fn this_host_triple() -> String {
+pub async fn this_host_triple() -> String {
     if cfg!(target_os = "windows") {
         // 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 = Box::new(currentprocess::TestProcess::default());
-        return currentprocess::with(tp, || TargetTriple::from_host().unwrap().to_string());
+        return currentprocess::with_tokio(tp, async {
+            TargetTriple::from_host().unwrap().to_string()
+        });
     }
     let arch = if cfg!(target_arch = "x86") {
         "i686"
@@ -167,9 +173,10 @@ pub fn this_host_triple() -> String {
 // Format a string with this host triple.
 #[macro_export]
 macro_rules! for_host {
-    ($s: expr) => {
-        &format!($s, $crate::test::this_host_triple())
-    };
+    ($s: expr) => {{
+        let triple = $crate::test::this_host_triple().await;
+        format!($s, triple)
+    }};
 }
 
 #[derive(Clone, Debug)]
@@ -212,13 +219,14 @@ impl fmt::Display for RustupHome {
 /// Create an isolated rustup home with no content, then call f with it, and
 /// delete it afterwards.
 #[cfg(test)]
-pub(crate) fn with_rustup_home<F>(f: F) -> Result<()>
+pub(crate) async fn with_rustup_home<F, FR>(f: F) -> Result<()>
 where
-    F: FnOnce(&RustupHome) -> Result<()>,
+    F: FnOnce(Arc<RustupHome>) -> FR,
+    FR: Future<Output = Result<()>>,
 {
     let test_dir = test_dir()?;
-    let rustup_home = RustupHome::new_in(test_dir)?;
-    f(&rustup_home)
+    let rustup_home = RustupHome::new_in(test_dir)?.into();
+    f(rustup_home).await
 }
 
 #[cfg(feature = "otel")]
@@ -227,7 +235,9 @@ use once_cell::sync::Lazy;
 use tokio;
 
 /// A tokio runtime for the sync tests, permitting the use of tracing. This is
-/// never shutdown, instead it is just dropped at end of process.
+/// never shutdown, instead it is just dropped at end of process, which so far
+/// seems to avoid https://github.com/tokio-rs/tokio/issues/631 but we may need
+/// to revisit this.
 #[cfg(feature = "otel")]
 static TRACE_RUNTIME: Lazy<tokio::runtime::Runtime> =
     Lazy::new(|| tokio::runtime::Runtime::new().unwrap());