Merge pull request #264 from NobodyXu/fix-repeated-crates-on-batch-installation

Fix repeated crates on batch installation: Dedup them and only keep the last one

Cargo.lock

@@ -139,6 +139,7 @@ dependencies = [
  "futures-util",
  "guess_host_triple",
  "home",
+ "itertools",
  "jobserver",
  "log",
  "miette",

Cargo.toml

@@ -31,6 +31,7 @@ flate2 = { version = "1.0.24", default-features = false }
 fs4 = "0.6.2"
 futures-util = { version = "0.3.21", default-features = false }
 home = "0.5.3"
+itertools = "0.10.3"
 jobserver = "0.1.24"
 log = "0.4.17"
 miette = "5.1.1"

src/binstall/resolve.rs

@@ -4,6 +4,7 @@ use std::{
 };
 
 use cargo_toml::{Package, Product};
+use compact_str::{format_compact, CompactString};
 use log::{debug, error, info, warn};
 use miette::{miette, Result};
 use reqwest::Client;
@@ -19,8 +20,8 @@ pub enum Resolution {
     Fetch {
         fetcher: Arc<dyn Fetcher>,
         package: Package<Meta>,
-        name: String,
+        name: CompactString,
-        version: String,
+        version: CompactString,
         bin_path: PathBuf,
         bin_files: Vec<bins::BinFile>,
     },
@@ -82,11 +83,11 @@ pub async fn resolve(
 ) -> Result<Resolution> {
     info!("Installing package: '{}'", crate_name);
 
-    let mut version = match (&crate_name.version, &opts.version) {
+    let mut version: CompactString = match (&crate_name.version, &opts.version) {
-        (Some(version), None) => version.to_string(),
+        (Some(version), None) => version.clone(),
-        (None, Some(version)) => version.to_string(),
+        (None, Some(version)) => version.into(),
         (Some(_), Some(_)) => Err(BinstallError::SuperfluousVersionOption)?,
-        (None, None) => "*".to_string(),
+        (None, None) => "*".into(),
     };
 
     // Treat 0.1.2 as =0.1.2
@@ -96,7 +97,7 @@ pub async fn resolve(
         .map(|ch| ch.is_ascii_digit())
         .unwrap_or(false)
     {
-        version.insert(0, '=');
+        version = format_compact!("={version}");
     }
 
     // Fetch crate via crates.io, git, or use a local manifest path

src/helpers/crate_name.rs

@@ -1,11 +1,12 @@
-use std::convert::Infallible;
+use std::{convert::Infallible, fmt, str::FromStr};
-use std::fmt;
-use std::str::FromStr;
 
-#[derive(Debug, Clone)]
+use compact_str::CompactString;
+use itertools::Itertools;
+
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CrateName {
-    pub name: String,
+    pub name: CompactString,
-    pub version: Option<String>,
+    pub version: Option<CompactString>,
 }
 
 impl fmt::Display for CrateName {
@@ -26,14 +27,82 @@ impl FromStr for CrateName {
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Ok(if let Some((name, version)) = s.split_once('@') {
             CrateName {
-                name: name.to_string(),
+                name: name.into(),
-                version: Some(version.to_string()),
+                version: Some(version.into()),
             }
         } else {
             CrateName {
-                name: s.to_string(),
+                name: s.into(),
                 version: None,
             }
         })
     }
 }
+
+impl CrateName {
+    pub fn dedup(mut crate_names: Vec<Self>) -> impl Iterator<Item = Self> {
+        crate_names.sort_by(|x, y| x.name.cmp(&y.name));
+        crate_names.into_iter().coalesce(|previous, current| {
+            if previous.name == current.name {
+                Ok(current)
+            } else {
+                Err((previous, current))
+            }
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    macro_rules! assert_dedup {
+        ([ $( ( $input_name:expr, $input_version:expr ) ),*  ], [ $( ( $output_name:expr, $output_version:expr ) ),*  ]) => {
+            let input_crate_names = vec![$( CrateName {
+                name: $input_name.into(),
+                version: Some($input_version.into())
+            }, )*];
+
+            let mut output_crate_names: Vec<CrateName> = vec![$( CrateName {
+                name: $output_name.into(), version: Some($output_version.into())
+            }, )*];
+            output_crate_names.sort_by(|x, y| x.name.cmp(&y.name));
+
+            let crate_names: Vec<_> = CrateName::dedup(input_crate_names).collect();
+            assert_eq!(crate_names, output_crate_names);
+        };
+    }
+
+    #[test]
+    fn test_dedup() {
+        // Base case 0: Empty input
+        assert_dedup!([], []);
+
+        // Base case 1: With only one input
+        assert_dedup!([("a", "1")], [("a", "1")]);
+
+        // Base Case 2: Only has duplicate names
+        assert_dedup!([("a", "1"), ("a", "2")], [("a", "2")]);
+
+        // Complex Case 0: Having two crates
+        assert_dedup!(
+            [("a", "10"), ("b", "3"), ("a", "0"), ("b", "0"), ("a", "1")],
+            [("a", "1"), ("b", "0")]
+        );
+
+        // Complex Case 1: Having three crates
+        assert_dedup!(
+            [
+                ("d", "1.1"),
+                ("a", "10"),
+                ("b", "3"),
+                ("d", "230"),
+                ("a", "0"),
+                ("b", "0"),
+                ("a", "1"),
+                ("d", "23")
+            ],
+            [("a", "1"), ("b", "0"), ("d", "23")]
+        );
+    }
+}

src/main.rs

@@ -32,6 +32,9 @@ struct Options {
     ///
     /// When multiple names are provided, the --version option and any override options are
     /// unavailable due to ambiguity.
+    ///
+    /// If duplicate names are provided, the last one (and their version requirement)
+    /// is kept.
     #[clap(help_heading = "Package selection", value_name = "crate[@version]")]
     crate_names: Vec<CrateName>,
 
@@ -242,11 +245,14 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
             ""
         };
 
-        if option != "" {
+        if !option.is_empty() {
             return Err(BinstallError::OverrideOptionUsedWithMultiInstall { option }.into());
         }
     }
 
+    // Remove duplicate crate_name, keep the last one
+    let crate_names = CrateName::dedup(crate_names);
+
     let cli_overrides = PkgOverride {
         pkg_url: opts.pkg_url.take(),
         pkg_fmt: opts.pkg_fmt.take(),