diff --git a/Cargo.lock b/Cargo.lock index ee1f8a61c..8e7082fd9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -139,6 +139,7 @@ dependencies = [ "futures-util", "guess_host_triple", "home", + "itertools", "jobserver", "log", "miette", diff --git a/Cargo.toml b/Cargo.toml index 7892a712c..6b714bd1a 100644 --- a/Cargo.toml +++ b/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" diff --git a/src/binstall/resolve.rs b/src/binstall/resolve.rs index a207192e2..5593ba2cf 100644 --- a/src/binstall/resolve.rs +++ b/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, package: Package, - name: String, - version: String, + name: CompactString, + version: CompactString, bin_path: PathBuf, bin_files: Vec, }, @@ -82,11 +83,11 @@ pub async fn resolve( ) -> Result { info!("Installing package: '{}'", crate_name); - let mut version = match (&crate_name.version, &opts.version) { - (Some(version), None) => version.to_string(), - (None, Some(version)) => version.to_string(), + let mut version: CompactString = match (&crate_name.version, &opts.version) { + (Some(version), None) => version.clone(), + (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 diff --git a/src/helpers/crate_name.rs b/src/helpers/crate_name.rs index 58ef59c0f..0f9ec2f4b 100644 --- a/src/helpers/crate_name.rs +++ b/src/helpers/crate_name.rs @@ -1,11 +1,12 @@ -use std::convert::Infallible; -use std::fmt; -use std::str::FromStr; +use std::{convert::Infallible, fmt, str::FromStr}; -#[derive(Debug, Clone)] +use compact_str::CompactString; +use itertools::Itertools; + +#[derive(Debug, Clone, Eq, PartialEq)] pub struct CrateName { - pub name: String, - pub version: Option, + pub name: CompactString, + pub version: Option, } impl fmt::Display for CrateName { @@ -26,14 +27,82 @@ impl FromStr for CrateName { fn from_str(s: &str) -> Result { Ok(if let Some((name, version)) = s.split_once('@') { CrateName { - name: name.to_string(), - version: Some(version.to_string()), + name: name.into(), + version: Some(version.into()), } } else { CrateName { - name: s.to_string(), + name: s.into(), version: None, } }) } } + +impl CrateName { + pub fn dedup(mut crate_names: Vec) -> impl Iterator { + 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 = 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")] + ); + } +} diff --git a/src/main.rs b/src/main.rs index ccbc519a9..478fedbb9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -31,6 +31,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, @@ -241,11 +244,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(),