Fixed uninstall a running binary failed on Windows

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(())

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))?;

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();
+}