diff --git a/src/cargo/ops/cargo_uninstall.rs b/src/cargo/ops/cargo_uninstall.rs index 1f22e191e80..7b9fdccd4cb 100644 --- a/src/cargo/ops/cargo_uninstall.rs +++ b/src/cargo/ops/cargo_uninstall.rs @@ -6,7 +6,6 @@ use crate::util::errors::CargoResult; use crate::util::Config; use crate::util::Filesystem; use anyhow::bail; -use cargo_util::paths; use std::collections::BTreeSet; use std::env; @@ -103,7 +102,6 @@ fn uninstall_pkgid( bins: &[String], config: &Config, ) -> CargoResult<()> { - let mut to_remove = Vec::new(); let installed = match tracker.installed_bins(pkgid) { Some(bins) => bins.clone(), None => bail!("package `{}` is not installed", pkgid), @@ -137,19 +135,18 @@ fn uninstall_pkgid( } } - if bins.is_empty() { - to_remove.extend(installed.iter().map(|b| dst.join(b))); - tracker.remove(pkgid, &installed); - } else { - for bin in bins.iter() { - to_remove.push(dst.join(bin)); + let to_remove = { + if bins.is_empty() { + installed + } else { + bins } - tracker.remove(pkgid, &bins); - } - tracker.save()?; + }; + for bin in to_remove { - config.shell().status("Removing", bin.display())?; - paths::remove_file(bin)?; + let bin_path = dst.join(&bin); + config.shell().status("Removing", bin_path.display())?; + tracker.remove_bin_then_save(pkgid, &bin, &bin_path)?; } Ok(()) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 77eb431c9ce..3da59340369 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -7,6 +7,7 @@ use std::rc::Rc; use std::task::Poll; use anyhow::{bail, format_err, Context as _}; +use cargo_util::paths; use ops::FilterRule; use serde::{Deserialize, Serialize}; @@ -319,6 +320,20 @@ impl InstallTracker { self.v1.remove(pkg_id, bins); self.v2.remove(pkg_id, bins); } + + /// Remove a bin after it successfully had been removed in disk and then save the tracker at last. + pub fn remove_bin_then_save( + &mut self, + pkg_id: PackageId, + bin: &str, + bin_path: &PathBuf, + ) -> CargoResult<()> { + paths::remove_file(bin_path)?; + self.v1.remove_bin(pkg_id, bin); + self.v2.remove_bin(pkg_id, bin); + self.save()?; + Ok(()) + } } impl CrateListingV1 { @@ -359,6 +374,17 @@ impl CrateListingV1 { } } + fn remove_bin(&mut self, pkg_id: PackageId, bin: &str) { + let mut installed = match self.v1.entry(pkg_id) { + btree_map::Entry::Occupied(e) => e, + btree_map::Entry::Vacant(..) => panic!("v1 unexpected missing `{}`", pkg_id), + }; + installed.get_mut().remove(bin); + if installed.get().is_empty() { + installed.remove(); + } + } + fn save(&self, lock: &FileLock) -> CargoResult<()> { let mut file = lock.file(); file.seek(SeekFrom::Start(0))?; @@ -468,6 +494,17 @@ impl CrateListingV2 { } } + fn remove_bin(&mut self, pkg_id: PackageId, bin: &str) { + let mut info_entry = match self.installs.entry(pkg_id) { + btree_map::Entry::Occupied(e) => e, + btree_map::Entry::Vacant(..) => panic!("v1 unexpected missing `{}`", pkg_id), + }; + info_entry.get_mut().bins.remove(bin); + if info_entry.get().bins.is_empty() { + info_entry.remove(); + } + } + fn save(&self, lock: &FileLock) -> CargoResult<()> { let mut file = lock.file(); file.seek(SeekFrom::Start(0))?; diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index fd53b607bf3..db35d6996f4 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -3,6 +3,7 @@ use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::path::Path; +use std::thread; use cargo_test_support::compare; use cargo_test_support::cross_compile; @@ -11,10 +12,10 @@ use cargo_test_support::registry::{self, registry_path, Package}; use cargo_test_support::{ basic_manifest, cargo_process, no_such_file_err_msg, project, project_in, symlink_supported, t, }; -use cargo_util::ProcessError; +use cargo_util::{ProcessBuilder, ProcessError}; use cargo_test_support::install::{ - assert_has_installed_exe, assert_has_not_installed_exe, cargo_home, + assert_has_installed_exe, assert_has_not_installed_exe, cargo_home, exe, }; use cargo_test_support::paths::{self, CargoPathExt}; use std::env; @@ -2507,3 +2508,100 @@ fn install_incompat_msrv() { ") .with_status(101).run(); } + +fn assert_tracker_noexistence(key: &str) { + let v1_data: toml::Value = + toml::from_str(&fs::read_to_string(cargo_home().join(".crates.toml")).unwrap()).unwrap(); + let v2_data: serde_json::Value = + serde_json::from_str(&fs::read_to_string(cargo_home().join(".crates2.json")).unwrap()) + .unwrap(); + + assert!(v1_data["v1"].get(key).is_none()); + assert!(v2_data["installs"][key].is_null()); +} + +#[cargo_test] +fn uninstall_running_binary() { + Package::new("foo", "0.0.1") + .file("src/lib.rs", "") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [[bin]] + name = "foo" + path = "src/main.rs" +"#, + ) + .file( + "src/main.rs", + r#" + use std::{{thread, time}}; + fn main() { + thread::sleep(time::Duration::from_secs(3)); + } +"#, + ) + .publish(); + + cargo_process("install foo") + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] foo v0.0.1 (registry [..]) +[INSTALLING] foo v0.0.1 +[COMPILING] foo v0.0.1 +[FINISHED] release [optimized] target(s) in [..] +[INSTALLING] [CWD]/home/.cargo/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`) +[WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries +", + ) + .run(); + assert_has_installed_exe(cargo_home(), "foo"); + + let foo_bin = cargo_home().join("bin").join(exe("foo")); + let t = thread::spawn(|| ProcessBuilder::new(foo_bin).exec().unwrap()); + let key = "foo 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)"; + + #[cfg(windows)] + { + cargo_process("uninstall foo") + .with_status(101) + .with_stderr_contains("[ERROR] failed to remove file `[CWD]/home/.cargo/bin/foo[EXE]`") + .run(); + t.join().unwrap(); + cargo_process("uninstall foo") + .with_stderr("[REMOVING] [CWD]/home/.cargo/bin/foo[EXE]") + .run(); + }; + + #[cfg(not(windows))] + { + cargo_process("uninstall foo") + .with_stderr("[REMOVING] [CWD]/home/.cargo/bin/foo[EXE]") + .run(); + t.join().unwrap(); + }; + + assert_has_not_installed_exe(cargo_home(), "foo"); + assert_tracker_noexistence(key); + + cargo_process("install foo") + .with_stderr( + "\ +[UPDATING] `[..]` index +[INSTALLING] foo v0.0.1 +[COMPILING] foo v0.0.1 +[FINISHED] release [optimized] target(s) in [..] +[INSTALLING] [CWD]/home/.cargo/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`) +[WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries +", + ) + .run(); +}