diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 564fe080d8..facfbde9e8 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -24,15 +24,16 @@ //! During uninstall (`rustup self uninstall`): //! //! * Delete `$RUSTUP_HOME`. -//! * Delete everything in `$CARGO_HOME`, including -//! the rustup binary and its hardlinks +//! * Delete all entries in `$CARGO_HOME` except `bin`. +//! * Delete rustup tool links and binary from `$CARGO_HOME/bin`. +//! * Delete `$CARGO_HOME/bin` if it is empty after uninstall. +//! * Delete `$CARGO_HOME` if it is empty after uninstall. //! //! Deleting the running binary during uninstall is tricky //! and racy on Windows. use std::borrow::Cow; use std::env::{self, consts::EXE_SUFFIX}; -#[cfg(not(windows))] use std::io; use std::io::Write; use std::path::{Component, MAIN_SEPARATOR, Path, PathBuf}; @@ -47,7 +48,7 @@ use clap::ValueEnum; use clap::builder::PossibleValue; use clap_cargo::style::{GOOD, WARN}; use itertools::Itertools; -use same_file::Handle; +use same_file::{Handle, is_same_file}; use serde::{Deserialize, Serialize}; use tracing::{error, info, trace, warn}; @@ -80,7 +81,7 @@ mod shell; #[cfg(unix)] mod unix; #[cfg(unix)] -use unix::{delete_rustup_and_cargo_home, do_add_to_path, do_remove_from_path}; +use unix::{do_add_to_path, do_remove_from_path}; #[cfg(unix)] pub(crate) use unix::{run_update, self_replace}; @@ -91,7 +92,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::{delete_rustup_and_cargo_home, do_add_to_path, do_remove_from_path}; +use windows::{do_add_to_path, do_remove_from_path}; #[cfg(windows)] pub(crate) use windows::{run_update, self_replace}; @@ -553,8 +554,8 @@ fn canonical_cargo_home(process: &Process) -> Result> { } /// Installing is a simple matter of copying the running binary to -/// `CARGO_HOME`/bin, hard-linking the various Rust tools to it, -/// and adding `CARGO_HOME`/bin to PATH. +/// `$CARGO_HOME/bin`, hard-linking the various Rust tools to it, +/// and adding `$CARGO_HOME/bin` to PATH. pub(crate) async fn install( no_prompt: bool, mut opts: InstallOpts<'_>, @@ -1048,6 +1049,13 @@ async fn maybe_install_rust(opts: InstallOpts<'_>, cfg: &mut Cfg<'_>) -> Result< Ok(()) } +/// Uninstall process: +/// 1. Remove rustup home. +/// 2. Remove all entries in `$CARGO_HOME` except `bin`. +/// 3. Remove rustup tool links and binary. +/// 4. Try to remove $CARGO_HOME/bin directory if it's empty. +/// 5. Upon successfully removing $CARGO_HOME/bin, clean up $PATH. +/// 6. Try to remove $CARGO_HOME directory if it's empty. pub(crate) fn uninstall( no_prompt: bool, no_modify_path: bool, @@ -1090,16 +1098,32 @@ pub(crate) fn uninstall( utils::remove_dir("rustup_home", &rustup_dir)?; } - info!("removing cargo home"); + // Delete rustup. + #[cfg(unix)] + clean_cargo_home(no_modify_path, process)?; + // NOTE: On windows, this is tricky because this is *probably* + // the running executable and on Windows can't be unlinked until + // the process exits. + // see: windows::{complete_windows_uninstall,spawn_uninstall_gc} + #[cfg(windows)] + windows::spawn_uninstall_gc(no_modify_path, process)?; - // Remove CARGO_HOME/bin from PATH - if !no_modify_path { - do_remove_from_path(process)?; - } + info!("rustup is uninstalled"); + + Ok(ExitCode::SUCCESS) +} + +/// Remove rustup-owned cargo-home state. +/// This removes non-`bin` entries in `$CARGO_HOME`, removes rustup tool links and executable from +/// `$CARGO_HOME/bin`, then removes `$CARGO_HOME/bin` and `$CARGO_HOME` only if they are empty. +/// Nonempty directories are left in place. +fn clean_cargo_home(no_modify_path: bool, process: &Process) -> Result<()> { + let cargo_home = process.cargo_home()?; + let cargo_bin = cargo_home.join("bin"); - // Delete everything in CARGO_HOME *except* the rustup bin + info!("removing cargo home"); - // First everything except the bin directory + // Delete everything in CARGO_HOME except the bin directory first. let diriter = fs::read_dir(&cargo_home).map_err(|e| CliError::ReadDirError { p: cargo_home.clone(), source: e, @@ -1118,44 +1142,58 @@ pub(crate) fn uninstall( } } - // Then everything in bin except rustup and tools. These can't be unlinked - // until this process exits (on windows). - let tools = TOOLS + info!("removing rustup tool links and binary"); + + let rustup_path = cargo_bin.join(format!("rustup{EXE_SUFFIX}")); + + let proxy_paths = TOOLS .iter() .chain(DUP_TOOLS.iter()) - .map(|t| format!("{t}{EXE_SUFFIX}")); - let tools: Vec<_> = tools.chain(vec![format!("rustup{EXE_SUFFIX}")]).collect(); - let bin_dir = cargo_home.join("bin"); - let diriter = fs::read_dir(&bin_dir).map_err(|e| CliError::ReadDirError { - p: bin_dir.clone(), - source: e, - })?; - for dirent in diriter { - let dirent = dirent.map_err(|e| CliError::ReadDirError { - p: bin_dir.clone(), - source: e, - })?; - let name = dirent.file_name(); - let file_is_tool = name.to_str().map(|n| tools.iter().any(|t| *t == n)); - if file_is_tool == Some(false) { - if dirent.path().is_dir() { - utils::remove_dir("cargo_home", &dirent.path())?; - } else { - utils::remove_file("cargo_home", &dirent.path())?; - } + .map(|tool| cargo_bin.join(format!("{tool}{EXE_SUFFIX}"))); + + for proxy_path in proxy_paths { + if is_same_file(&proxy_path, &rustup_path).unwrap_or(false) { + utils::remove_file("rustup tool proxy", &proxy_path)?; } } - info!("removing rustup binaries"); + utils::remove_file("rustup_bin", &rustup_path)?; - // Delete rustup. This is tricky because this is *probably* - // the running executable and on Windows can't be unlinked until - // the process exits. - delete_rustup_and_cargo_home(process)?; + let cargo_bin_display = cargo_bin.display(); + info!("removing empty cargo bin directory `{cargo_bin_display}`"); - info!("rustup is uninstalled"); + match fs::remove_dir(&cargo_bin) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => { + warn!("keeping non-empty cargo bin directory `{cargo_bin_display}`") + } + Err(e) => { + return Err(e).with_context(|| { + format!("failed to remove cargo bin directory `{cargo_bin_display}`") + }); + } + Ok(()) if !no_modify_path => { + info!("removing cargo bin directory `{cargo_bin_display}` from $PATH"); + do_remove_from_path(process)?; + } + Ok(()) => {} + } - Ok(ExitCode::SUCCESS) + let cargo_home_display = cargo_home.display(); + info!("removing empty cargo home directory `{cargo_home_display}`"); + + match fs::remove_dir(&cargo_home) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => { + warn!("keeping non-empty cargo home directory `{cargo_home_display}`"); + } + Err(e) => { + return Err(e).with_context(|| { + format!("failed to remove cargo home directory `{cargo_home_display}`") + }); + } + Ok(()) => {} + } + + Ok(()) } #[derive(Clone, Copy, Debug)] @@ -1195,7 +1233,7 @@ pub(crate) fn self_update_permitted(explicit: bool) -> Result Result) -> Result { common::warn_if_host_is_emulated(cfg.process); diff --git a/src/cli/self_update/unix.rs b/src/cli/self_update/unix.rs index e4e236e2bc..7edcd68b78 100644 --- a/src/cli/self_update/unix.rs +++ b/src/cli/self_update/unix.rs @@ -47,11 +47,6 @@ pub(crate) fn do_anti_sudo_check(no_prompt: bool, process: &Process) -> Result Result<()> { - let cargo_home = process.cargo_home()?; - utils::remove_dir("cargo_home", &cargo_home) -} - pub(crate) fn do_remove_from_path(process: &Process) -> Result<()> { for sh in shell::get_available_shells(process) { let source_bytes = format!("{}\n", sh.source_string(process)?).into_bytes(); @@ -139,7 +134,7 @@ pub(crate) fn run_update(setup_path: &Path) -> Result { } /// This function is as the final step of a self-upgrade. It replaces -/// `CARGO_HOME`/bin/rustup with the running exe, and updates the +/// `$CARGO_HOME/bin/rustup` with the running exe, and updates the /// links to it. pub(crate) fn self_replace(process: &Process) -> Result { install_bins(process)?; diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 07578e3ec1..1dd7e13671 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -355,9 +355,10 @@ pub fn complete_windows_uninstall(process: &Process) -> Result wait_for_parent()?; - // Now that the parent has exited there are hopefully no more files open in CARGO_HOME - let cargo_home = process.cargo_home()?; - utils::remove_dir("cargo_home", &cargo_home)?; + let no_modify_path = process.var_os(GC_MODIFY_PATH).as_deref() != Some(OsStr::new("1")); + + // Now that the parent has exited there are hopefully no more files open in CARGO_HOME. + super::clean_cargo_home(no_modify_path, process)?; // Now, run a *system* binary to inherit the DELETE_ON_CLOSE // handle to *this* process, then exit. The OS will delete the gc @@ -644,10 +645,10 @@ pub(crate) fn self_replace(process: &Process) -> Result { Ok(utils::ExitCode(0)) } -// The last step of uninstallation is to delete *this binary*, -// rustup.exe and the CARGO_HOME that contains it. On Unix, this -// works fine. On Windows you can't delete files while they are open, -// like when they are running. +// Spawn a temporary rustup-gc-$random.exe to finish Windows uninstall +// after the original rustup.exe process exits. On Unix, the running +// executable can be deleted directly. On Windows you can't delete files +// while they are open, like when they are running. // // Here's what we're going to do: // - Copy rustup.exe to a temporary file in @@ -674,7 +675,7 @@ pub(crate) fn self_replace(process: &Process) -> Result { // // .. augmented with this SO answer // https://stackoverflow.com/questions/10319526/understanding-a-self-deleting-program-in-c -pub(crate) fn delete_rustup_and_cargo_home(process: &Process) -> Result<()> { +pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> Result<()> { use std::io; use std::ptr; use std::thread; @@ -734,6 +735,7 @@ pub(crate) fn delete_rustup_and_cargo_home(process: &Process) -> Result<()> { }; Command::new(gc_exe) + .env(GC_MODIFY_PATH, if no_modify_path { "0" } else { "1" }) .spawn() .context(CliError::WindowsUninstallMadness)?; @@ -749,6 +751,10 @@ pub(crate) fn delete_rustup_and_cargo_home(process: &Process) -> Result<()> { Ok(()) } +// The rustup-gc executable cannot accept normal function call here, +// 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"; + #[cfg(any(test, feature = "test"))] pub fn get_path() -> Result> { USER_PATH.get() diff --git a/tests/suite/cli_paths.rs b/tests/suite/cli_paths.rs index f102dd5234..65b6170aab 100644 --- a/tests/suite/cli_paths.rs +++ b/tests/suite/cli_paths.rs @@ -268,6 +268,41 @@ error: could not amend shell profile[..] } } + #[tokio::test] + async fn uninstall_keeps_source_in_rcs_when_cargo_bin_is_non_empty() { + let cx = CliTestContext::new(Scenario::Empty).await; + let rcs: Vec = [ + ".bashrc", + ".bash_profile", + ".bash_login", + ".profile", + // This requires zsh to be installed, so we test it on macOS only. + #[cfg(target_os = "macos")] + ".zshenv", + ] + .iter() + .map(|rc| cx.config.homedir.join(rc)) + .collect(); + + for rc in &rcs { + raw::write_file(rc, FAKE_RC).unwrap(); + } + + let expected = FAKE_RC.to_owned() + &source(cx.config.cargodir.display(), POSIX_SH); + + cx.config.expect(&INIT_NONE).await.is_ok(); + fs::write(cx.config.cargodir.join("bin/custom-tool"), "").unwrap(); + cx.config + .expect(&["rustup", "self", "uninstall", "-y"]) + .await + .is_ok(); + + for rc in &rcs { + let new_rc = fs::read_to_string(rc).unwrap(); + assert_eq!(new_rc, expected); + } + } + #[tokio::test] async fn uninstall_doesnt_modify_rcs_with_no_modify_path() { let cx = CliTestContext::new(Scenario::Empty).await; @@ -467,6 +502,31 @@ mod windows { assert!(!get_path_().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 cfg_path = cx.config.cargodir.join("bin").display().to_string(); + let get_path_ = || { + HSTRING::try_from(get_path().unwrap().unwrap()) + .unwrap() + .to_string() + }; + + cx.config.expect(&INIT_NONE).await.is_ok(); + std::fs::write(cx.config.cargodir.join("bin/custom-tool"), "").unwrap(); + cx.config + .expect(&["rustup", "self", "uninstall", "-y"]) + .await + .is_ok(); + assert!( + get_path_().contains(cfg_path.trim_matches('"')), + "`{}` not in `{}`", + cfg_path, + get_path_() + ); + } + #[tokio::test] async fn uninstall_doesnt_affect_path_with_no_modify_path() { let cx = CliTestContext::new(Scenario::Empty).await; diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index f821659935..4f95a9466b 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -8,6 +8,7 @@ use std::process::Command; use remove_dir_all::remove_dir_all; +#[cfg(windows)] use retry::{ delay::{Fibonacci, jitter}, retry, @@ -268,6 +269,49 @@ async fn uninstall_deletes_cargo_home() { assert!(!cx.config.cargodir.exists()); } +#[tokio::test] +async fn complete_uninstall_removes_empty_cargo_bin() { + let cx = setup_empty_installed().await; + let cargo_bin = cx.config.cargodir.join("bin"); + cx.config + .expect(["rustup", "self", "uninstall", "-y"]) + .await + .is_ok(); + + #[cfg(unix)] + assert!(!cargo_bin.exists()); + + #[cfg(windows)] + retry(Fibonacci::from_millis(1).map(jitter).take(23), || { + if cargo_bin.exists() { + return Err(format!("cargo bin still exists: {}", cargo_bin.display())); + } + Ok(()) + }) + .unwrap() +} + +#[tokio::test] +async fn complete_uninstall_keeps_non_empty_cargo_bin() { + let cx = setup_empty_installed().await; + let cargo_bin = cx.config.cargodir.join("bin"); + + let mock_file = cargo_bin.join("custom-tool"); + fs::write(&mock_file, "").unwrap(); + + cx.config + .expect(["rustup", "self", "uninstall", "-y"]) + .await + .with_stderr(snapbox::str![[r#" +... +warn: keeping non-empty cargo bin directory `[..]` +... +"#]]) + .is_ok(); + assert!(cargo_bin.exists()); + assert!(mock_file.exists()); +} + #[tokio::test] async fn uninstall_fails_if_not_installed() { let cx = setup_empty_installed().await; @@ -325,6 +369,7 @@ async fn uninstall_self_delete_works() { // On windows rustup self uninstall temporarily puts a rustup-gc-$randomnumber.exe // file in CONFIG.CARGODIR/.. ; check that it doesn't exist. #[tokio::test] +#[cfg(windows)] async fn uninstall_doesnt_leave_gc_file() { let cx = setup_empty_installed().await; cx.config @@ -344,19 +389,30 @@ async fn uninstall_doesnt_leave_gc_file() { } } +#[cfg(windows)] fn ensure_empty(dir: &Path) -> Result<(), GcErr> { let garbage = fs::read_dir(dir) .unwrap() - .map(|d| d.unwrap().path().to_string_lossy().to_string()) + .filter_map(|entry| { + let path = entry.unwrap().path(); + let name = path.file_name()?.to_str()?; + // On Windows, this binary is cleaned up on exit + if !(name.starts_with("rustup-gc-") && name.ends_with(EXE_SUFFIX)) { + return None; + } + Some(path.to_string_lossy().to_string()) + }) .collect::>(); - match garbage.len() { - 0 => Ok(()), - _ => Err(GcErr(garbage)), + if garbage.is_empty() { + Ok(()) + } else { + Err(GcErr(garbage)) } } #[derive(thiserror::Error, Debug)] #[error("garbage remaining: {:?}", .0)] +#[cfg(windows)] struct GcErr(Vec); #[tokio::test]