From 593e14a48f15e0bd1f8178b5bbc63368afbf756a Mon Sep 17 00:00:00 2001 From: baka-gourd <36119339+baka-gourd@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:38:11 +0800 Subject: [PATCH 1/4] refactor(self-update): extract Windows registry cleanup from do_remove_from_path --- src/cli/self_update.rs | 4 +++- src/cli/self_update/windows.rs | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 259535b0b6..72a2ca6e6a 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -94,7 +94,7 @@ pub use windows::complete_windows_uninstall; #[cfg(all(windows, feature = "test"))] pub use windows::{RegistryGuard, RegistryValueId, USER_PATH, get_path}; #[cfg(windows)] -use windows::{do_add_to_path, do_remove_from_path}; +use windows::{do_add_to_path, do_remove_from_path, do_remove_from_programs}; #[cfg(windows)] pub(crate) use windows::{run_update, self_replace}; @@ -1030,6 +1030,8 @@ fn clean_cargo_home(no_modify_path: bool, process: &Process) -> Result<()> { Ok(()) if !no_modify_path => { info!("removing cargo bin directory `{cargo_bin_display}` from $PATH"); do_remove_from_path(process)?; + #[cfg(windows)] + do_remove_from_programs()?; } Ok(()) => {} } diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 84dad678d7..5215cd5293 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -566,8 +566,7 @@ where pub(crate) fn do_remove_from_path(process: &Process) -> Result<()> { let new_path = _with_path_cargo_home_bin(_remove_from_path, process)?; - _apply_new_path(new_path)?; - do_remove_from_programs() + _apply_new_path(new_path) } const RUSTUP_UNINSTALL_ENTRY: &str = r"Software\Microsoft\Windows\CurrentVersion\Uninstall\Rustup"; From 393cadc964b994fe9960d8270d330a9e9476b1ec Mon Sep 17 00:00:00 2001 From: baka-gourd <36119339+baka-gourd@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:38:41 +0800 Subject: [PATCH 2/4] refactor(self-update): extract Windows registry updates from do_add_to_path --- src/cli/self_update.rs | 4 +++- src/cli/self_update/windows.rs | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 72a2ca6e6a..6ba0b6d14c 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -94,7 +94,7 @@ pub use windows::complete_windows_uninstall; #[cfg(all(windows, feature = "test"))] pub use windows::{RegistryGuard, RegistryValueId, USER_PATH, get_path}; #[cfg(windows)] -use windows::{do_add_to_path, do_remove_from_path, do_remove_from_programs}; +use windows::{do_add_to_path, do_add_to_programs, do_remove_from_path, do_remove_from_programs}; #[cfg(windows)] pub(crate) use windows::{run_update, self_replace}; @@ -237,6 +237,8 @@ impl InstallOpts<'_> { if !self.no_modify_path { do_add_to_path(cfg.process)?; + #[cfg(windows)] + do_add_to_programs(cfg.process)?; } // If RUSTUP_HOME is not set, make sure it exists diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 5215cd5293..54f148689a 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -449,8 +449,7 @@ pub(crate) fn wait_for_parent() -> Result<()> { pub(crate) fn do_add_to_path(process: &Process) -> Result<()> { let new_path = _with_path_cargo_home_bin(_add_to_path, process)?; - _apply_new_path(new_path)?; - do_add_to_programs(process) + _apply_new_path(new_path) } fn _apply_new_path(new_path: Option) -> Result<()> { From a79e28003ee646ac2099cf4a0139185f71b1eef1 Mon Sep 17 00:00:00 2001 From: baka-gourd <36119339+baka-gourd@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:40:02 +0800 Subject: [PATCH 3/4] refactor(self-update)!: update Windows registry independently of PATH changes --- src/cli/self_update.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 6ba0b6d14c..5efdf2f70c 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -237,10 +237,11 @@ impl InstallOpts<'_> { if !self.no_modify_path { do_add_to_path(cfg.process)?; - #[cfg(windows)] - do_add_to_programs(cfg.process)?; } + #[cfg(windows)] + do_add_to_programs(cfg.process)?; + // If RUSTUP_HOME is not set, make sure it exists if cfg.process.var_os("RUSTUP_HOME").is_none() { let home = cfg @@ -1017,6 +1018,9 @@ fn clean_cargo_home(no_modify_path: bool, process: &Process) -> Result<()> { utils::remove_file("rustup_bin", &rustup_path)?; + #[cfg(windows)] + do_remove_from_programs()?; + let cargo_bin_display = cargo_bin.display(); info!("removing empty cargo bin directory `{cargo_bin_display}`"); @@ -1032,8 +1036,6 @@ fn clean_cargo_home(no_modify_path: bool, process: &Process) -> Result<()> { Ok(()) if !no_modify_path => { info!("removing cargo bin directory `{cargo_bin_display}` from $PATH"); do_remove_from_path(process)?; - #[cfg(windows)] - do_remove_from_programs()?; } Ok(()) => {} } From 91589c27aac074a6c1778cd4eb5d395a17029535 Mon Sep 17 00:00:00 2001 From: baka-gourd <36119339+baka-gourd@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:41:49 +0800 Subject: [PATCH 4/4] feat(test): isolate Windows registry state per test --- src/cli/self_update.rs | 8 +- src/cli/self_update/unix.rs | 2 +- src/cli/self_update/windows.rs | 252 ++++++++++++++++++++-------- src/env_var.rs | 6 - src/test.rs | 7 +- src/test/clitools.rs | 11 ++ tests/suite/cli_inst_interactive.rs | 6 - tests/suite/cli_paths.rs | 54 +++--- tests/suite/cli_self_upd.rs | 25 +-- 9 files changed, 247 insertions(+), 124 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 5efdf2f70c..92e4e7d62e 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -92,7 +92,7 @@ mod windows; #[cfg(windows)] pub use windows::complete_windows_uninstall; #[cfg(all(windows, feature = "test"))] -pub use windows::{RegistryGuard, RegistryValueId, USER_PATH, get_path}; +pub use windows::{RUSTUP_TEST_REGISTRY_ID, RegistryValueId, USER_PATH, get_path}; #[cfg(windows)] use windows::{do_add_to_path, do_add_to_programs, do_remove_from_path, do_remove_from_programs}; #[cfg(windows)] @@ -505,7 +505,7 @@ impl SelfUpdateMode { let setup_path = prepare_update(dl_cfg).await?; if let Some(setup_path) = &setup_path { - return run_update(setup_path); + return run_update(setup_path, dl_cfg.process); } else { // Try again in case we emitted "tool `{}` is already installed" last time. install_proxies(dl_cfg.process)?; @@ -1019,7 +1019,7 @@ fn clean_cargo_home(no_modify_path: bool, process: &Process) -> Result<()> { utils::remove_file("rustup_bin", &rustup_path)?; #[cfg(windows)] - do_remove_from_programs()?; + do_remove_from_programs(process)?; let cargo_bin_display = cargo_bin.display(); info!("removing empty cargo bin directory `{cargo_bin_display}`"); @@ -1146,7 +1146,7 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result { PackageUpdate::Rustup, Ok(UpdateStatus::Updated(version)), ); - return run_update(&setup_path); + return run_update(&setup_path, cfg.process); } None => { let _ = common::show_channel_update( diff --git a/src/cli/self_update/unix.rs b/src/cli/self_update/unix.rs index e594336675..c8fe9bb016 100644 --- a/src/cli/self_update/unix.rs +++ b/src/cli/self_update/unix.rs @@ -117,7 +117,7 @@ pub(crate) fn do_write_env_files(process: &Process) -> Result<()> { /// Tell the upgrader to replace the rustup bins, then delete /// itself. -pub(crate) fn run_update(setup_path: &Path) -> Result { +pub(crate) fn run_update(setup_path: &Path, _process: &Process) -> Result { let status = Command::new(setup_path) .arg("--self-replace") .status() diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 54f148689a..139ba38522 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -5,8 +5,6 @@ use std::io::Write; use std::os::windows::ffi::OsStrExt; use std::path::Path; use std::process::Command; -#[cfg(any(test, feature = "test"))] -use std::sync::{LockResult, Mutex, MutexGuard}; use anyhow::{Context, Result, anyhow}; use tracing::{info, warn}; @@ -449,10 +447,10 @@ pub(crate) fn wait_for_parent() -> Result<()> { pub(crate) fn do_add_to_path(process: &Process) -> Result<()> { let new_path = _with_path_cargo_home_bin(_add_to_path, process)?; - _apply_new_path(new_path) + _apply_new_path(new_path, process) } -fn _apply_new_path(new_path: Option) -> Result<()> { +fn _apply_new_path(new_path: Option, process: &Process) -> Result<()> { use std::ptr; use windows_sys::Win32::Foundation::{LPARAM, WPARAM}; use windows_sys::Win32::UI::WindowsAndMessaging::{ @@ -463,7 +461,7 @@ fn _apply_new_path(new_path: Option) -> Result<()> { return Ok(()); // No need to set the path }; - let environment = CURRENT_USER.create("Environment")?; + let environment = process.registry_key("Environment", CURRENT_USER)?; if new_path.is_empty() { environment.remove_value("PATH")?; @@ -491,9 +489,9 @@ fn _apply_new_path(new_path: Option) -> Result<()> { // Get the windows PATH variable out of the registry as a String. If // this returns None then the PATH variable is not a string and we // should not mess with it. -fn get_windows_path_var() -> Result> { - let environment = CURRENT_USER - .create("Environment") +fn get_windows_path_var(process: &Process) -> Result> { + let environment = process + .registry_key("Environment", CURRENT_USER) .context("Failed opening Environment key")?; let reg_value = environment.get_hstring("PATH"); @@ -557,7 +555,7 @@ fn _with_path_cargo_home_bin(f: F, process: &Process) -> Result Option, { - let windows_path = get_windows_path_var()?; + let windows_path = get_windows_path_var(process)?; let mut path_str = process.cargo_home()?; path_str.push("bin"); Ok(windows_path.and_then(|old_path| f(old_path, HSTRING::from(path_str.as_path())))) @@ -565,19 +563,47 @@ where pub(crate) fn do_remove_from_path(process: &Process) -> Result<()> { let new_path = _with_path_cargo_home_bin(_remove_from_path, process)?; - _apply_new_path(new_path) + _apply_new_path(new_path, process) } const RUSTUP_UNINSTALL_ENTRY: &str = r"Software\Microsoft\Windows\CurrentVersion\Uninstall\Rustup"; -fn rustup_uninstall_reg_key() -> Result { - CURRENT_USER - .create(RUSTUP_UNINSTALL_ENTRY) +impl Process { + fn registry_sub_key_path(&self, sub_key: &str) -> String { + #[cfg(any(test, feature = "test"))] + if let Some(test_id) = self.var_os(RUSTUP_TEST_REGISTRY_ID) { + let test_id = test_id.to_string_lossy(); + return map_sub_key(sub_key, &test_id); + } + + sub_key.to_owned() + } + + fn registry_key(&self, sub_key: &str, parent: &Key) -> windows_registry::Result { + let mut options = parent.options(); + options.read().write().create(); + + #[cfg(any(test, feature = "test"))] + if self.var_os(RUSTUP_TEST_REGISTRY_ID).is_some() { + options.volatile(); + } + + options.open(self.registry_sub_key_path(sub_key)) + } + + fn remove_registry_tree(&self, sub_key: &str, parent: &Key) -> windows_registry::Result<()> { + parent.remove_tree(self.registry_sub_key_path(sub_key)) + } +} + +fn rustup_uninstall_reg_key(process: &Process) -> Result { + process + .registry_key(RUSTUP_UNINSTALL_ENTRY, CURRENT_USER) .context("Failed creating uninstall key") } -pub(crate) fn do_update_programs_display_version(version: &str) -> Result<()> { - rustup_uninstall_reg_key()? +pub(crate) fn do_update_programs_display_version(version: &str, process: &Process) -> Result<()> { + rustup_uninstall_reg_key(process)? .set_string("DisplayVersion", version) .context("Failed to set `DisplayVersion`") } @@ -585,7 +611,7 @@ pub(crate) fn do_update_programs_display_version(version: &str) -> Result<()> { pub(crate) fn do_add_to_programs(process: &Process) -> Result<()> { use std::path::PathBuf; - let key = rustup_uninstall_reg_key()?; + let key = rustup_uninstall_reg_key(process)?; // Don't overwrite registry if Rustup is already installed let prev = key.get_hstring("UninstallString"); @@ -607,20 +633,20 @@ pub(crate) fn do_add_to_programs(process: &Process) -> Result<()> { .context("Failed to set `UninstallString`")?; key.set_string("DisplayName", "Rustup: the Rust toolchain installer") .context("Failed to set `DisplayName`")?; - do_update_programs_display_version(env!("CARGO_PKG_VERSION"))?; + do_update_programs_display_version(env!("CARGO_PKG_VERSION"), process)?; Ok(()) } -pub(crate) fn do_remove_from_programs() -> Result<()> { - match CURRENT_USER.remove_tree(RUSTUP_UNINSTALL_ENTRY) { +pub(crate) fn do_remove_from_programs(process: &Process) -> Result<()> { + match process.remove_registry_tree(RUSTUP_UNINSTALL_ENTRY, CURRENT_USER) { Ok(()) => Ok(()), Err(e) if e.code() == HRESULT::from_win32(ERROR_FILE_NOT_FOUND) => Ok(()), Err(e) => Err(anyhow!(e)), } } -pub(crate) fn run_update(setup_path: &Path) -> Result { +pub(crate) fn run_update(setup_path: &Path, process: &Process) -> Result { Command::new(setup_path) .arg("--self-replace") .spawn() @@ -630,7 +656,7 @@ pub(crate) fn run_update(setup_path: &Path) -> Result { warn!("failed to get the new rustup version in order to update `DisplayVersion`"); return Ok(utils::ExitCode(1)); }; - do_update_programs_display_version(&version)?; + do_update_programs_display_version(&version, process)?; Ok(utils::ExitCode(0)) } @@ -752,39 +778,26 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> Res // so we use env var here, notifying it if we need to remove $CARGO_HOME/bin from $PATH const GC_MODIFY_PATH: &str = "RUSTUP_GC_MODIFY_PATH"; +/// Environment variable carrying the per-test registry ID. #[cfg(any(test, feature = "test"))] -pub fn get_path() -> Result> { - USER_PATH.get() -} - -#[cfg(any(test, feature = "test"))] -pub struct RegistryGuard<'a> { - _locked: LockResult>, - id: &'static RegistryValueId, - prev: Option, -} +pub const RUSTUP_TEST_REGISTRY_ID: &str = "RUSTUP_TEST_REGISTRY_ID"; +/// Maps a real registry sub-key onto a per-test `RustupTest-{test_id}` subtree. #[cfg(any(test, feature = "test"))] -impl RegistryGuard<'_> { - pub fn new(id: &'static RegistryValueId) -> Result { - Ok(Self { - _locked: REGISTRY_LOCK.lock(), - id, - prev: id.get()?, - }) +fn map_sub_key(sub_key: &str, test_id: &str) -> String { + let root = format!(r"Software\Microsoft\Windows\CurrentVersion\Uninstall\RustupTest-{test_id}"); + if sub_key == RUSTUP_UNINSTALL_ENTRY { + format!(r"{root}\Programs") + } else { + format!(r"{root}\{sub_key}") } } #[cfg(any(test, feature = "test"))] -impl Drop for RegistryGuard<'_> { - fn drop(&mut self) { - self.id.set(self.prev.as_ref()).unwrap(); - } +pub fn get_path(test_id: &str) -> Result> { + USER_PATH.get(test_id, CURRENT_USER) } -#[cfg(any(test, feature = "test"))] -static REGISTRY_LOCK: Mutex<()> = Mutex::new(()); - #[cfg(any(test, feature = "test"))] pub const USER_PATH: RegistryValueId = RegistryValueId { sub_key: "Environment", @@ -799,8 +812,10 @@ pub struct RegistryValueId { #[cfg(any(test, feature = "test"))] impl RegistryValueId { - pub fn get(&self) -> Result> { - let sub_key = CURRENT_USER.create(self.sub_key)?; + pub fn get(&self, test_id: &str, parent: &Key) -> Result> { + let mut options = parent.options(); + options.read().write().create().volatile(); + let sub_key = options.open(map_sub_key(self.sub_key, test_id))?; match sub_key.get_value(self.value_name) { Ok(val) => Ok(Some(val)), Err(e) if e.code() == HRESULT::from_win32(ERROR_FILE_NOT_FOUND) => Ok(None), @@ -808,23 +823,113 @@ impl RegistryValueId { } } - pub fn set(&self, new: Option<&Value>) -> Result<()> { - let sub_key = CURRENT_USER.create(self.sub_key)?; + pub fn set(&self, new: Option<&Value>, test_id: &str, parent: &Key) -> Result<()> { + let mut options = parent.options(); + options.read().write().create().volatile(); + let sub_key = options.open(map_sub_key(self.sub_key, test_id))?; match new { Some(new) => Ok(sub_key.set_value(self.value_name, new)?), - None => Ok(sub_key.remove_value(self.value_name)?), + None => match sub_key.remove_value(self.value_name) { + Ok(()) => Ok(()), + Err(e) if e.code() == HRESULT::from_win32(ERROR_FILE_NOT_FOUND) => Ok(()), + Err(e) => Err(e.into()), + }, } } } #[cfg(test)] mod tests { + use std::collections::HashMap; use std::os::windows::ffi::OsStringExt; use windows_registry::Type; use super::*; use crate::process::TestProcess; + use crate::test::test_id; + + fn test_process(test_id: &str) -> TestProcess { + let vars: HashMap = [ + ("HOME".to_string(), "/unused".to_string()), + (RUSTUP_TEST_REGISTRY_ID.to_string(), test_id.to_owned()), + ] + .into_iter() + .collect(); + TestProcess::with_vars(vars) + } + + fn test_environment_key(process: &Process) -> Key { + process.registry_key("Environment", CURRENT_USER).unwrap() + } + + fn clear_path(environment: &Key) { + match environment.remove_value("PATH") { + Ok(()) => {} + Err(e) if e.code() == HRESULT::from_win32(ERROR_FILE_NOT_FOUND) => {} + Err(e) => panic!("failed to clear PATH: {e}"), + } + } + + #[test] + fn windows_registry_isolated_per_test_id() { + let first_id = test_id(); + let second_id = test_id(); + let first = test_process(&first_id); + let second = test_process(&second_id); + let first_value = Value::from("first"); + let second_value = Value::from("second"); + + USER_PATH + .set(Some(&first_value), &first_id, CURRENT_USER) + .unwrap(); + USER_PATH + .set(Some(&second_value), &second_id, CURRENT_USER) + .unwrap(); + + assert_eq!( + first + .process + .registry_key("Environment", CURRENT_USER) + .unwrap() + .get_hstring("PATH") + .unwrap(), + HSTRING::from("first") + ); + assert_eq!( + second + .process + .registry_key("Environment", CURRENT_USER) + .unwrap() + .get_hstring("PATH") + .unwrap(), + HSTRING::from("second") + ); + + let programs_version = RegistryValueId { + sub_key: RUSTUP_UNINSTALL_ENTRY, + value_name: "DisplayVersion", + }; + programs_version + .set(Some(&first_value), &first_id, CURRENT_USER) + .unwrap(); + assert_eq!( + programs_version.get(&first_id, CURRENT_USER).unwrap(), + Some(first_value) + ); + assert_eq!( + programs_version.get(&second_id, CURRENT_USER).unwrap(), + None + ); + } + + #[test] + fn windows_registry_cleanup_is_idempotent() { + let test_id = test_id(); + let tp = test_process(&test_id); + + do_remove_from_programs(&tp.process).unwrap(); + } #[test] fn windows_install_does_not_add_path_twice() { @@ -862,16 +967,20 @@ mod tests { #[test] fn windows_path_regkey_type() { // per issue #261, setting PATH should use REG_EXPAND_SZ. - let _guard = RegistryGuard::new(&USER_PATH); - let environment = CURRENT_USER.create("Environment").unwrap(); - environment.remove_value("PATH").unwrap(); + let test_id = test_id(); + let tp = test_process(&test_id); + let environment = test_environment_key(&tp.process); + clear_path(&environment); { // Can't compare the Results as Eq isn't derived; thanks error-chain. #![allow(clippy::unit_cmp)] - assert_eq!((), _apply_new_path(Some(HSTRING::from("foo"))).unwrap()); + assert_eq!( + (), + _apply_new_path(Some(HSTRING::from("foo")), &tp.process).unwrap() + ); } - let environment = CURRENT_USER.create("Environment").unwrap(); + let environment = test_environment_key(&tp.process); let path = environment.get_value("PATH").unwrap(); let path_hstring = environment.get_hstring("PATH").unwrap(); assert_eq!(path.ty(), Type::ExpandString); @@ -882,8 +991,9 @@ mod tests { fn windows_path_delete_key_when_empty() { // during uninstall the PATH key may end up empty; if so we should // delete it. - let _guard = RegistryGuard::new(&USER_PATH); - let environment = CURRENT_USER.create("Environment").unwrap(); + let test_id = test_id(); + let tp = test_process(&test_id); + let environment = test_environment_key(&tp.process); environment .set_expand_hstring("PATH", &HSTRING::from("foo")) .unwrap(); @@ -891,7 +1001,10 @@ mod tests { { // Can't compare the Results as Eq isn't derived; thanks error-chain. #![allow(clippy::unit_cmp)] - assert_eq!((), _apply_new_path(Some(HSTRING::new())).unwrap()); + assert_eq!( + (), + _apply_new_path(Some(HSTRING::new()), &tp.process).unwrap() + ); } let reg_value = environment.get_value("PATH"); match reg_value { @@ -903,16 +1016,11 @@ mod tests { #[test] fn windows_doesnt_mess_with_a_non_string_path() { + let test_id = test_id(); // This writes an error, so we want a sink for it. - let tp = TestProcess::with_vars( - [("HOME".to_string(), "/unused".to_string())] - .iter() - .cloned() - .collect(), - ); + let tp = test_process(&test_id); - let _guard = RegistryGuard::new(&USER_PATH); - let environment = CURRENT_USER.create("Environment").unwrap(); + let environment = test_environment_key(&tp.process); environment .set_bytes("PATH", Type::Bytes, &[0x12, 0x34]) .unwrap(); @@ -932,11 +1040,15 @@ mod tests { #[test] fn windows_treat_missing_path_as_empty() { // during install the PATH key may be missing; treat it as empty - let _guard = RegistryGuard::new(&USER_PATH); - let environment = CURRENT_USER.create("Environment").unwrap(); - environment.remove_value("PATH").unwrap(); + let test_id = test_id(); + let tp = test_process(&test_id); + let environment = test_environment_key(&tp.process); + clear_path(&environment); - assert_eq!(Some(HSTRING::new()), get_windows_path_var().unwrap()); + assert_eq!( + Some(HSTRING::new()), + get_windows_path_var(&tp.process).unwrap() + ); } #[test] diff --git a/src/env_var.rs b/src/env_var.rs index 45b19c6389..5ec689843e 100644 --- a/src/env_var.rs +++ b/src/env_var.rs @@ -54,8 +54,6 @@ mod tests { use std::ffi::{OsStr, OsString}; use super::*; - #[cfg(windows)] - use crate::cli::self_update::{RegistryGuard, USER_PATH}; use crate::process::TestProcess; use crate::test::Env; @@ -67,8 +65,6 @@ mod tests { env::join_paths(["/home/a/.cargo/bin", "/home/b/.cargo/bin"].iter()).unwrap(), ); let tp = TestProcess::with_vars(vars); - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); let mut path_entries = vec![]; let mut cmd = Command::new("test"); @@ -115,8 +111,6 @@ mod tests { env::join_paths(["/home/a/.cargo/bin", "/home/b/.cargo/bin"].iter()).unwrap(), ); let tp = TestProcess::with_vars(vars); - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); #[track_caller] fn check(tp: &TestProcess, path_entries: Vec, append: &str, expected: &[&str]) { diff --git a/src/test.rs b/src/test.rs index 41020d8370..ef058d3c41 100644 --- a/src/test.rs +++ b/src/test.rs @@ -23,8 +23,13 @@ use sha2::{Digest, Sha256}; use crate::dist::TargetTuple; use crate::process::TestProcess; +#[cfg(all(windows, any(test, feature = "test")))] +pub(crate) fn test_id() -> String { + crate::utils::raw::random_string(8) +} + #[cfg(windows)] -pub use crate::cli::self_update::{RegistryGuard, RegistryValueId, USER_PATH, get_path}; +pub use crate::cli::self_update::{RUSTUP_TEST_REGISTRY_ID, RegistryValueId, USER_PATH, get_path}; mod clitools; pub use clitools::{ diff --git a/src/test/clitools.rs b/src/test/clitools.rs index 1c76d61089..e99dde166f 100644 --- a/src/test/clitools.rs +++ b/src/test/clitools.rs @@ -64,6 +64,9 @@ pub struct Config { pub workdir: RefCell, /// This is the test root for keeping stuff together test_root_dir: PathBuf, + /// Per-test Windows registry ID. + #[cfg(windows)] + pub test_registry_id: String, } /// Helper type to simplify assertions of a command's output. @@ -324,6 +327,12 @@ impl Config { if let Some(root) = self.rustup_update_root.as_ref() { cmd.env("RUSTUP_UPDATE_ROOT", root); } + + #[cfg(windows)] + cmd.env( + crate::cli::self_update::RUSTUP_TEST_REGISTRY_ID, + &self.test_registry_id, + ); } /// Returns an [`Assert`] object to check the output of running the command @@ -826,6 +835,8 @@ async fn setup_test_state(test_dist_dir: TempDir) -> (TempDir, Config) { rustup_update_root: None, workdir: RefCell::new(workdir), test_root_dir: test_dir.path().to_path_buf(), + #[cfg(windows)] + test_registry_id: crate::test::test_id(), }; let build_path = built_exe_dir.join(format!("rustup-init{EXE_SUFFIX}")); diff --git a/tests/suite/cli_inst_interactive.rs b/tests/suite/cli_inst_interactive.rs index 9b75e951c4..d4cb900244 100644 --- a/tests/suite/cli_inst_interactive.rs +++ b/tests/suite/cli_inst_interactive.rs @@ -5,8 +5,6 @@ use std::io::Write; use std::process::Stdio; use rustup::test::{Assert, CliTestContext, Config, SanitizedOutput, Scenario, this_host_tuple}; -#[cfg(windows)] -use rustup::test::{RegistryGuard, USER_PATH}; use rustup::utils::raw; fn run_input(config: &Config, args: &[&str], input: &str) -> Assert { @@ -44,8 +42,6 @@ fn run_input_with_env(config: &Config, args: &[&str], input: &str, env: &[(&str, #[tokio::test] async fn update() { let cx = CliTestContext::new(Scenario::SimpleV2).await; - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); run_input(&cx.config, &["rustup-init"], "\n\n"); run_input(&cx.config, &["rustup-init"], "\n\n").is_ok(); @@ -100,8 +96,6 @@ Rust is installed now. Great! #[tokio::test] async fn smoke_case_install_with_path_install() { let cx = CliTestContext::new(Scenario::SimpleV2).await; - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); run_input(&cx.config, &["rustup-init"], "\n\n") .is_ok() diff --git a/tests/suite/cli_paths.rs b/tests/suite/cli_paths.rs index b8a5e70dde..cee70513ff 100644 --- a/tests/suite/cli_paths.rs +++ b/tests/suite/cli_paths.rs @@ -504,46 +504,54 @@ error: could not amend shell profile[..] #[cfg(windows)] mod windows { + use retry::delay::{Fibonacci, jitter}; + use retry::{OperationResult, retry}; + use super::INIT_NONE; use rustup::test::{CliTestContext, Scenario}; - use rustup::test::{RegistryGuard, USER_PATH, get_path}; + use rustup::test::{USER_PATH, get_path}; - use windows_registry::{HSTRING, Value}; + use windows_registry::{CURRENT_USER, HSTRING, Value}; #[tokio::test] /// Smoke test for end-to-end code connectivity of the installer path mgmt on windows. async fn install_uninstall_affect_path() { let cx = CliTestContext::new(Scenario::Empty).await; - let _guard = RegistryGuard::new(&USER_PATH).unwrap(); + let test_id = &cx.config.test_registry_id; let cfg_path = cx.config.cargodir.join("bin").display().to_string(); - let get_path_ = || { - HSTRING::try_from(get_path().unwrap().unwrap()) - .unwrap() - .to_string() + let read_path = |test_id: &str| -> Option { + retry( + Fibonacci::from_millis(1).map(jitter).take(21), + || match get_path(test_id).unwrap() { + Some(v) => OperationResult::Ok(HSTRING::try_from(v).unwrap().to_string()), + None => OperationResult::Retry(()), + }, + ) + .ok() }; cx.config.expect(&INIT_NONE).await.is_ok(); + let after_install = read_path(test_id).unwrap_or_default(); assert!( - get_path_().contains(cfg_path.trim_matches('"')), - "`{}` not in `{}`", - cfg_path, - get_path_() + after_install.contains(cfg_path.trim_matches('"')), + "`{cfg_path}` not in `{after_install}`", ); cx.config .expect(&["rustup", "self", "uninstall", "-y"]) .await .is_ok(); - assert!(!get_path_().contains(&cfg_path)); + let after_uninstall = read_path(test_id).unwrap_or_default(); + assert!(!after_uninstall.contains(&cfg_path)); } #[tokio::test] async fn uninstall_keeps_path_when_cargo_bin_is_non_empty() { let cx = CliTestContext::new(Scenario::Empty).await; - let _guard = RegistryGuard::new(&USER_PATH).unwrap(); + let test_id = &cx.config.test_registry_id; let cfg_path = cx.config.cargodir.join("bin").display().to_string(); let get_path_ = || { - HSTRING::try_from(get_path().unwrap().unwrap()) + HSTRING::try_from(get_path(test_id).unwrap().unwrap()) .unwrap() .to_string() }; @@ -565,10 +573,10 @@ mod windows { #[tokio::test] async fn uninstall_doesnt_affect_path_with_no_modify_path() { let cx = CliTestContext::new(Scenario::Empty).await; - let _guard = RegistryGuard::new(&USER_PATH).unwrap(); + let test_id = &cx.config.test_registry_id; let cfg_path = cx.config.cargodir.join("bin").display().to_string(); let get_path_ = || { - HSTRING::try_from(get_path().unwrap().unwrap()) + HSTRING::try_from(get_path(test_id).unwrap().unwrap()) .unwrap() .to_string() }; @@ -591,10 +599,10 @@ mod windows { async fn install_uninstall_affect_path_with_non_unicode() { use std::os::windows::ffi::OsStrExt; - use windows_registry::{CURRENT_USER, Type}; + use windows_registry::Type; let cx = CliTestContext::new(Scenario::Empty).await; - let _guard = RegistryGuard::new(&USER_PATH).unwrap(); + let test_id = &cx.config.test_registry_id; // Set up a non unicode PATH let mut reg_value = Value::from([ 0x00, 0xD8, // leading surrogate @@ -602,10 +610,8 @@ mod windows { 0x00, 0x00, // null ]); reg_value.set_ty(Type::ExpandString); - CURRENT_USER - .create("Environment") - .unwrap() - .set_value("PATH", ®_value) + USER_PATH + .set(Some(®_value), test_id, CURRENT_USER) .unwrap(); // compute expected path after installation @@ -624,12 +630,12 @@ mod windows { expected.set_ty(Type::ExpandString); cx.config.expect(&INIT_NONE).await.is_ok(); - assert_eq!(get_path().unwrap().unwrap(), expected); + assert_eq!(get_path(test_id).unwrap().unwrap(), expected); cx.config .expect(&["rustup", "self", "uninstall", "-y"]) .await .is_ok(); - assert_eq!(get_path().unwrap().unwrap(), reg_value); + assert_eq!(get_path(test_id).unwrap().unwrap(), reg_value); } } diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index c9778c4a54..73d2ccc385 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -13,16 +13,16 @@ use retry::{ delay::{Fibonacci, jitter}, retry, }; +#[cfg(windows)] +use rustup::test::RegistryValueId; use rustup::test::{ CROSS_ARCH1, CliTestContext, Scenario, SelfUpdateTestContext, calc_hash, output_release_file, this_host_tuple, }; -#[cfg(windows)] -use rustup::test::{RegistryGuard, RegistryValueId, USER_PATH}; use rustup::utils::{self, raw}; use rustup::{DUP_TOOLS, TOOLS}; #[cfg(windows)] -use windows_registry::Value; +use windows_registry::{CURRENT_USER, Value}; const TEST_VERSION: &str = "1.1.1"; @@ -58,8 +58,6 @@ async fn setup_installed() -> CliTestContext { #[tokio::test] async fn install_bins_to_cargo_home() { let cx = CliTestContext::new(Scenario::SimpleV2).await; - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); cx.config .expect(["rustup-init", "-y"]) @@ -102,8 +100,6 @@ info: default toolchain set to stable-[HOST_TUPLE] #[tokio::test] async fn proxies_are_relative_symlinks() { let cx = CliTestContext::new(Scenario::SimpleV2).await; - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); cx.config .expect(["rustup-init", "-y"]) @@ -142,8 +138,6 @@ info: default toolchain set to stable-[HOST_TUPLE] #[tokio::test] async fn install_twice() { let cx = CliTestContext::new(Scenario::SimpleV2).await; - #[cfg(windows)] - let _path_guard = RegistryGuard::new(&USER_PATH).unwrap(); cx.config.expect(["rustup-init", "-y"]).await.is_ok(); cx.config.expect(["rustup-init", "-y"]).await.is_ok(); @@ -495,18 +489,25 @@ async fn update_overwrites_programs_display_version() { let version = env!("CARGO_PKG_VERSION"); let cx = SelfUpdateTestContext::new(TEST_VERSION).await; - let _guard = RegistryGuard::new(&USER_RUSTUP_VERSION).unwrap(); + let test_id = &cx.config.test_registry_id; cx.config .expect(["rustup-init", "-y", "--no-modify-path"]) .await .is_ok(); USER_RUSTUP_VERSION - .set(Some(&Value::from(PLACEHOLDER_VERSION))) + .set( + Some(&Value::from(PLACEHOLDER_VERSION)), + test_id, + CURRENT_USER, + ) .unwrap(); cx.config.expect(["rustup", "self", "update"]).await.is_ok(); assert_eq!( - USER_RUSTUP_VERSION.get().unwrap().unwrap(), + USER_RUSTUP_VERSION + .get(test_id, CURRENT_USER) + .unwrap() + .unwrap(), Value::from(version) ); }