From 9391d22fa28c0e5c194a302ebb4a8f0ce4f4618a Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Aug 2022 19:54:01 +1000 Subject: [PATCH 1/6] Apply clippy suggestion Signed-off-by: Jiahao XU --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index ccbc519a..052c57e5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -241,7 +241,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { "" }; - if option != "" { + if !option.is_empty() { return Err(BinstallError::OverrideOptionUsedWithMultiInstall { option }.into()); } } From 58c50cbe9186cdb02073ec885eb77828b6bf9e75 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Aug 2022 20:18:44 +1000 Subject: [PATCH 2/6] Add new dep itertools v0.10.3 Signed-off-by: Jiahao XU --- Cargo.lock | 1 + Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ee1f8a61..8e7082fd 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 7892a712..6b714bd1 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" From 09c1afe616a670f7f9c4a300474381fcdb7c5be8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Aug 2022 22:14:05 +1000 Subject: [PATCH 3/6] Impl new fn `CrateName::dedup` Signed-off-by: Jiahao XU --- src/helpers/crate_name.rs | 76 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 4 deletions(-) diff --git a/src/helpers/crate_name.rs b/src/helpers/crate_name.rs index 58ef59c0..7c38e8f5 100644 --- a/src/helpers/crate_name.rs +++ b/src/helpers/crate_name.rs @@ -1,8 +1,8 @@ -use std::convert::Infallible; -use std::fmt; -use std::str::FromStr; +use std::{convert::Infallible, fmt, str::FromStr}; -#[derive(Debug, Clone)] +use itertools::Itertools; + +#[derive(Debug, Clone, Eq, PartialEq)] pub struct CrateName { pub name: String, pub version: Option, @@ -37,3 +37,71 @@ impl FromStr for CrateName { }) } } + +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")] + ); + } +} From caeb49ce336633b312758adb3eec3595fec36459 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Aug 2022 22:15:09 +1000 Subject: [PATCH 4/6] Rm duplicate `crate_names` specified on cmdline Signed-off-by: Jiahao XU --- src/main.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main.rs b/src/main.rs index 052c57e5..478fedbb 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, @@ -246,6 +249,9 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { } } + // 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(), From d430c077d43bad732a24ce2c9df94a9bb5800102 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Aug 2022 22:17:11 +1000 Subject: [PATCH 5/6] Use `CompactString` for fields of `CrateName` Since most of the time, they are shorter than 24 bytes. Signed-off-by: Jiahao XU --- src/binstall/resolve.rs | 3 ++- src/helpers/crate_name.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/binstall/resolve.rs b/src/binstall/resolve.rs index a207192e..3279bf14 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::CompactString; use log::{debug, error, info, warn}; use miette::{miette, Result}; use reqwest::Client; @@ -19,7 +20,7 @@ pub enum Resolution { Fetch { fetcher: Arc, package: Package, - name: String, + name: CompactString, version: String, bin_path: PathBuf, bin_files: Vec, diff --git a/src/helpers/crate_name.rs b/src/helpers/crate_name.rs index 7c38e8f5..0f9ec2f4 100644 --- a/src/helpers/crate_name.rs +++ b/src/helpers/crate_name.rs @@ -1,11 +1,12 @@ use std::{convert::Infallible, fmt, str::FromStr}; +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,12 +27,12 @@ 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, } }) From b7cfa0aa643f2c1cdee951c099e1f1944a556675 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Aug 2022 22:21:45 +1000 Subject: [PATCH 6/6] Use `CompactString` for field `Resolution::Fetch::version` Signed-off-by: Jiahao XU --- src/binstall/resolve.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/binstall/resolve.rs b/src/binstall/resolve.rs index 3279bf14..5593ba2c 100644 --- a/src/binstall/resolve.rs +++ b/src/binstall/resolve.rs @@ -4,7 +4,7 @@ use std::{ }; use cargo_toml::{Package, Product}; -use compact_str::CompactString; +use compact_str::{format_compact, CompactString}; use log::{debug, error, info, warn}; use miette::{miette, Result}; use reqwest::Client; @@ -21,7 +21,7 @@ pub enum Resolution { fetcher: Arc, package: Package, name: CompactString, - version: String, + version: CompactString, bin_path: PathBuf, bin_files: Vec, }, @@ -83,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 @@ -97,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