fix: Rm uninstalled crates from binstall/crates-v1.json (#2197)

* Add Records::retain

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Sync with cargo manifest

Rm uninstalled crates

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix entry.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Test manifest syncing in e2e-tests/live.sh

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix binstall_crates_v1.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix crates_manifests.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix binstall_crates_v1.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix entry.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix clippy in entry.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix fmt in entry.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix fmt in crates_manifests.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

* Fix fmt in crates_manifests.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

---------

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

crates/bin/src/entry.rs

@@ -67,7 +67,7 @@ pub fn install_crates(
 
     // Compute paths
     let cargo_root = args.root;
-    let (install_path, mut manifests, temp_dir) = compute_paths_and_load_manifests(
+    let (install_path, manifests, temp_dir) = compute_paths_and_load_manifests(
         cargo_root.clone(),
         args.install_path,
         args.no_track,
@@ -77,7 +77,7 @@ pub fn install_crates(
 
     // Remove installed crates
     let mut crate_names =
-        filter_out_installed_crates(args.crate_names, args.force, manifests.as_mut())?.peekable();
+        filter_out_installed_crates(args.crate_names, args.force, manifests.as_ref()).peekable();
 
     if crate_names.peek().is_none() {
         debug!("Nothing to do");
@@ -456,26 +456,23 @@ fn compute_paths_and_load_manifests(
 }
 
 /// Return vec of (crate_name, current_version)
-fn filter_out_installed_crates(
+fn filter_out_installed_crates<'a>(
     crate_names: Vec<CrateName>,
     force: bool,
-    manifests: Option<&mut Manifests>,
-) -> Result<impl Iterator<Item = (CrateName, Option<semver::Version>)> + '_> {
-    let mut installed_crates = manifests
-        .map(Manifests::load_installed_crates)
-        .transpose()?;
+    manifests: Option<&'a Manifests>,
+) -> impl Iterator<Item = (CrateName, Option<semver::Version>)> + 'a {
+    let installed_crates = manifests.map(|m| m.installed_crates());
 
-    Ok(CrateName::dedup(crate_names)
+    CrateName::dedup(crate_names)
     .filter_map(move |crate_name| {
         let name = &crate_name.name;
 
         let curr_version = installed_crates
-            .as_mut()
             // Since crate_name is deduped, every entry of installed_crates
             // can be visited at most once.
             //
             // So here we take ownership of the version stored to avoid cloning.
-            .and_then(|crates| crates.remove(name));
+            .and_then(|crates| crates.get(name));
 
         match (
             force,
@@ -483,7 +480,7 @@ fn filter_out_installed_crates(
             &crate_name.version_req,
         ) {
             (false, Some(curr_version), Some(version_req))
-                if version_req.is_latest_compatible(&curr_version) =>
+                if version_req.is_latest_compatible(curr_version) =>
             {
                 debug!("Bailing out early because we can assume wanted is already installed from metafile");
                 info!("{name} v{curr_version} is already installed, use --force to override");
@@ -492,12 +489,12 @@ fn filter_out_installed_crates(
 
             // The version req is "*" thus a remote upgraded version could exist
             (false, Some(curr_version), None) => {
-                Some((crate_name, Some(curr_version)))
+                Some((crate_name, Some(curr_version.clone())))
             }
 
             _ => Some((crate_name, None)),
         }
-    }))
+    })
 }
 
 #[allow(clippy::vec_box)]

crates/binstalk-manifests/src/binstall_crates_v1.rs

@@ -214,6 +214,11 @@ impl Records {
         self.data.remove(value.as_ref())
     }
 
+    /// Remove crates that `f(&data.crate_info)` returns `false`.
+    pub fn retain(&mut self, mut f: impl FnMut(&CrateInfo) -> bool) {
+        self.data.retain(|data| f(&data.crate_info))
+    }
+
     pub fn take(&mut self, value: impl AsRef<str>) -> Option<CrateInfo> {
         self.data.take(value.as_ref()).map(CrateInfo::from)
     }

crates/binstalk-manifests/src/crates_manifests.rs

@@ -35,6 +35,7 @@ pub enum ManifestsError {
 pub struct Manifests {
     binstall: BinstallCratesV1Records,
     cargo_crates_v1: FileLock,
+    installed_crates: BTreeMap<CompactString, Version>,
 }
 
 impl Manifests {
@@ -43,16 +44,22 @@ impl Manifests {
         let metadata_path = cargo_roots.join("binstall/crates-v1.json");
         fs::create_dir_all(metadata_path.parent().unwrap())?;
 
-        let binstall = BinstallCratesV1Records::load_from_path(&metadata_path)?;
+        let mut binstall = BinstallCratesV1Records::load_from_path(&metadata_path)?;
 
         // Read cargo_install_v1_metadata
         let manifest_path = cargo_roots.join(".crates.toml");
 
-        let cargo_crates_v1 = create_if_not_exist(&manifest_path)?;
+        let mut cargo_crates_v1 = create_if_not_exist(&manifest_path)?;
+
+        let installed_crates = CratesToml::load_from_reader(&mut cargo_crates_v1)
+            .and_then(CratesToml::collect_into_crates_versions)?;
+
+        binstall.retain(|crate_info| installed_crates.contains_key(&crate_info.name));
 
         Ok(Self {
             binstall,
             cargo_crates_v1,
+            installed_crates,
         })
     }
 
@@ -64,14 +71,8 @@ impl Manifests {
     /// but it only updates .crates.toml.
     ///
     /// So here we will honour .crates.toml only.
-    pub fn load_installed_crates(
-        &mut self,
-    ) -> Result<BTreeMap<CompactString, Version>, ManifestsError> {
-        self.rewind_cargo_crates_v1()?;
-
-        CratesToml::load_from_reader(&mut self.cargo_crates_v1)
-            .and_then(CratesToml::collect_into_crates_versions)
-            .map_err(ManifestsError::from)
+    pub fn installed_crates(&self) -> &BTreeMap<CompactString, Version> {
+        &self.installed_crates
     }
 
     pub fn update(mut self, metadata_vec: Vec<CrateInfo>) -> Result<(), ManifestsError> {

e2e-tests/live.sh

@@ -66,3 +66,8 @@ git_mob_version="$(git-mob --version)"
 echo "$git_mob_version"
 
 [ "$git_mob_version" = "git-mob-tool 1.6.1" ]
+
+cargo uninstall b3sum cargo-binstall
+
+"./$1" binstall -y cargo-binstall@0.20.1
+jq <"$CARGO_HOME/binstall/crates-v1.json" | grep -v b3sum