From 04fee49c225f44bd8346f14519331d99fc05ed82 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 4 Aug 2022 23:45:53 +1000 Subject: [PATCH 01/23] Add new option `Options::force` Signed-off-by: Jiahao XU --- src/main.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main.rs b/src/main.rs index 8d74a90a..5d21889e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -131,6 +131,10 @@ struct Options { #[clap(help_heading = "Options", long)] secure: bool, + /// Force a crate to be installed even if it is already installed. + #[clap(help_heading = "Options", long)] + force: bool, + /// Require a minimum TLS version from remote endpoints. /// /// The default is not to require any minimum TLS version, and use the negotiated highest From ac085533cc510dd746b22d1a3c657807f79f55c6 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 00:06:36 +1000 Subject: [PATCH 02/23] Skip crates that are already installed. Signed-off-by: Jiahao XU --- src/main.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5d21889e..9d25c06e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -333,6 +333,25 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { fs::create_dir_all(&install_path).map_err(BinstallError::Io)?; debug!("Using install path: {}", install_path.display()); + // Load metadata + let metadata = if !custom_install_path { + debug!("Reading binstall/crates-v1.json"); + Some(metafiles::binstall_v1::Records::load()?) + } else { + None + }; + + // Filter out installed crate_names + let crate_names = crate_names.filter(|crate_name| { + if opts.force { + true + } else if let Some(records) = &metadata { + !records.contains(&crate_name.name) + } else { + true + } + }); + // Create a temporary directory for downloads etc. // // Put all binaries to a temporary directory under `dst` first, catching @@ -429,7 +448,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { } block_in_place(|| { - if !custom_install_path { + if let Some(mut records) = metadata { // If using standardised install path, // then create_dir_all(&install_path) would also // create .cargo. @@ -438,7 +457,10 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { metafiles::v1::CratesToml::append(metadata_vec.iter())?; debug!("Writing binstall/crates-v1.json"); - metafiles::binstall_v1::append(metadata_vec)?; + for metadata in metadata_vec { + records.replace(metadata); + } + records.overwrite()?; } if opts.no_cleanup { From 6716d75607914e7b6105cf9d1c7c3b7d6790b532 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 00:09:40 +1000 Subject: [PATCH 03/23] Minor Refactor: Gather code related to `crate_names` Signed-off-by: Jiahao XU --- src/main.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index 9d25c06e..5ff11c5f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -285,9 +285,6 @@ 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(), @@ -341,8 +338,8 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { None }; - // Filter out installed crate_names - let crate_names = crate_names.filter(|crate_name| { + // Remove installed crates + let crate_names = CrateName::dedup(crate_names).filter(|crate_name| { if opts.force { true } else if let Some(records) = &metadata { From 4dae214af302d5b0ca39ad910e07b78d6fdff350 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 00:11:59 +1000 Subject: [PATCH 04/23] Use `block_in_place` for loading metadata 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 5ff11c5f..cb7bc8de 100644 --- a/src/main.rs +++ b/src/main.rs @@ -333,7 +333,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { // Load metadata let metadata = if !custom_install_path { debug!("Reading binstall/crates-v1.json"); - Some(metafiles::binstall_v1::Records::load()?) + Some(block_in_place(metafiles::binstall_v1::Records::load)?) } else { None }; From b4c6db7cda8fa2b723092ccb26a0a95963ec86e6 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 00:13:35 +1000 Subject: [PATCH 05/23] Refactor & Optimize: Launch target detection as soon as possible Signed-off-by: Jiahao XU --- src/main.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index cb7bc8de..e8cb2191 100644 --- a/src/main.rs +++ b/src/main.rs @@ -291,6 +291,9 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { bin_dir: opts.bin_dir.take(), }; + // Launch target detection + let desired_targets = get_desired_targets(&opts.targets); + // Initialize reqwest client let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?; @@ -318,9 +321,6 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { // Initialize UI thread let mut uithread = UIThread::new(!opts.no_confirm); - // Launch target detection - let desired_targets = get_desired_targets(&opts.targets); - // Compute install directory let (install_path, custom_install_path) = get_install_path(opts.install_path.as_deref()); let install_path = install_path.ok_or_else(|| { From b8c44839c1d470270dc7c983a153a700c97bbc86 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 00:18:21 +1000 Subject: [PATCH 06/23] Detect install_path & load metadata in `block_in_place` since they involves blocking fs io. Signed-off-by: Jiahao XU --- src/main.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/main.rs b/src/main.rs index e8cb2191..bf7f4955 100644 --- a/src/main.rs +++ b/src/main.rs @@ -322,21 +322,25 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { let mut uithread = UIThread::new(!opts.no_confirm); // Compute install directory - let (install_path, custom_install_path) = get_install_path(opts.install_path.as_deref()); - let install_path = install_path.ok_or_else(|| { - error!("No viable install path found of specified, try `--install-path`"); - miette!("No install path found or specified") - })?; - fs::create_dir_all(&install_path).map_err(BinstallError::Io)?; - debug!("Using install path: {}", install_path.display()); + let (install_path, metadata) = block_in_place(|| -> Result<_> { + let (install_path, custom_install_path) = get_install_path(opts.install_path.as_deref()); + let install_path = install_path.ok_or_else(|| { + error!("No viable install path found of specified, try `--install-path`"); + miette!("No install path found or specified") + })?; + fs::create_dir_all(&install_path).map_err(BinstallError::Io)?; + debug!("Using install path: {}", install_path.display()); - // Load metadata - let metadata = if !custom_install_path { - debug!("Reading binstall/crates-v1.json"); - Some(block_in_place(metafiles::binstall_v1::Records::load)?) - } else { - None - }; + // Load metadata + let metadata = if !custom_install_path { + debug!("Reading binstall/crates-v1.json"); + Some(metafiles::binstall_v1::Records::load()?) + } else { + None + }; + + Ok((install_path, metadata)) + })?; // Remove installed crates let crate_names = CrateName::dedup(crate_names).filter(|crate_name| { From 4b79abeedc4b5a473fbec6256ba9b84b0e7d5e6f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 00:26:17 +1000 Subject: [PATCH 07/23] Refactor: Run `TempDir` creation in `block_in_place` Since it could also issues blocking operations. Signed-off-by: Jiahao XU --- src/main.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main.rs b/src/main.rs index bf7f4955..750a438d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -321,8 +321,8 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { // Initialize UI thread let mut uithread = UIThread::new(!opts.no_confirm); - // Compute install directory - let (install_path, metadata) = block_in_place(|| -> Result<_> { + let (install_path, metadata, temp_dir) = block_in_place(|| -> Result<_> { + // Compute install directory let (install_path, custom_install_path) = get_install_path(opts.install_path.as_deref()); let install_path = install_path.ok_or_else(|| { error!("No viable install path found of specified, try `--install-path`"); @@ -339,7 +339,18 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { None }; - Ok((install_path, metadata)) + // Create a temporary directory for downloads etc. + // + // Put all binaries to a temporary directory under `dst` first, catching + // some failure modes (e.g., out of space) before touching the existing + // binaries. This directory will get cleaned up via RAII. + let temp_dir = tempfile::Builder::new() + .prefix("cargo-binstall") + .tempdir_in(&install_path) + .map_err(BinstallError::from) + .wrap_err("Creating a temporary directory failed.")?; + + Ok((install_path, metadata, temp_dir)) })?; // Remove installed crates @@ -353,17 +364,6 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { } }); - // Create a temporary directory for downloads etc. - // - // Put all binaries to a temporary directory under `dst` first, catching - // some failure modes (e.g., out of space) before touching the existing - // binaries. This directory will get cleaned up via RAII. - let temp_dir = tempfile::Builder::new() - .prefix("cargo-binstall") - .tempdir_in(&install_path) - .map_err(BinstallError::from) - .wrap_err("Creating a temporary directory failed.")?; - let temp_dir_path: Arc = Arc::from(temp_dir.path()); // Create binstall_opts From 34f714c64f7ca4ae839c210760028154667bd5ee Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 15:42:48 +1000 Subject: [PATCH 08/23] Pass `--force` to `cargo-install` Signed-off-by: Jiahao XU --- src/binstall.rs | 1 + src/binstall/install.rs | 7 ++++++- src/main.rs | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/binstall.rs b/src/binstall.rs index 43707c0c..73c474c3 100644 --- a/src/binstall.rs +++ b/src/binstall.rs @@ -13,6 +13,7 @@ pub use install::*; pub struct Options { pub no_symlinks: bool, pub dry_run: bool, + pub force: bool, pub version: Option, pub manifest_path: Option, pub cli_overrides: PkgOverride, diff --git a/src/binstall/install.rs b/src/binstall/install.rs index 455c3961..21015333 100644 --- a/src/binstall/install.rs +++ b/src/binstall/install.rs @@ -47,7 +47,7 @@ pub async fn install( .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; if !opts.dry_run { - install_from_source(package, target, jobserver_client, opts.quiet) + install_from_source(package, target, jobserver_client, opts.quiet, opts.force) .await .map(|_| None) } else { @@ -127,6 +127,7 @@ async fn install_from_source( target: &str, lazy_jobserver_client: LazyJobserverClient, quiet: bool, + force: bool, ) -> Result<()> { let jobserver_client = lazy_jobserver_client.get().await?; @@ -150,6 +151,10 @@ async fn install_from_source( cmd.arg("--quiet"); } + if force { + cmd.arg("--force"); + } + let mut child = cmd .spawn() .into_diagnostic() diff --git a/src/main.rs b/src/main.rs index 750a438d..da87fc49 100644 --- a/src/main.rs +++ b/src/main.rs @@ -370,6 +370,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { let binstall_opts = Arc::new(binstall::Options { no_symlinks: opts.no_symlinks, dry_run: opts.dry_run, + force: opts.force, version: opts.version_req.take(), manifest_path: opts.manifest_path.take(), cli_overrides, From 20c7b61e7a302d51c08429e6e46a3ac31b649048 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 5 Aug 2022 20:30:03 +1000 Subject: [PATCH 09/23] FIx `ci-scripts/tests.sh`: Use `--force` to force installation. Signed-off-by: Jiahao XU --- ci-scripts/tests.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci-scripts/tests.sh b/ci-scripts/tests.sh index 8bb7bd3c..41d10b59 100755 --- a/ci-scripts/tests.sh +++ b/ci-scripts/tests.sh @@ -19,7 +19,7 @@ done cargo binstall --help >/dev/null # Install binaries using `--manifest-path` -"./$1" binstall --log-level debug --manifest-path . --no-confirm cargo-binstall +"./$1" binstall --force --log-level debug --manifest-path . --no-confirm cargo-binstall # Test that the installed binaries can be run cargo binstall --help >/dev/null @@ -28,6 +28,7 @@ min_tls=1.3 [[ "${2:-}" == "Windows" ]] && min_tls=1.2 # WinTLS on GHA doesn't support 1.3 yet "./$1" binstall \ + --force \ --log-level debug \ --secure \ --min-tls-version $min_tls \ From 51d6b3039b37cf47346e096bd6a892a4e07927a5 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 18:37:45 +1000 Subject: [PATCH 10/23] Impl new fn `helpers::parse_version` Signed-off-by: Jiahao XU --- src/helpers.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index 2e010244..b311f75d 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use bytes::Bytes; use cargo_toml::Manifest; +use compact_str::format_compact; use futures_util::stream::Stream; use log::debug; use once_cell::sync::{Lazy, OnceCell}; @@ -86,6 +87,20 @@ pub async fn await_task(task: tokio::task::JoinHandle>) -> } } +pub fn parse_version(version: &str) -> Result { + // Treat 0.1.2 as =0.1.2 + if version + .chars() + .next() + .map(|ch| ch.is_ascii_digit()) + .unwrap_or(false) + { + format_compact!("={version}").parse() + } else { + version.parse() + } +} + /// Load binstall metadata from the crate `Cargo.toml` at the provided path pub fn load_manifest_path>( manifest_path: P, From dd8e6a400d8b33f467d2176d133d49a3d3b27abc Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 18:40:24 +1000 Subject: [PATCH 11/23] Rename `CrateName::version` to `version_req` and change its type to `Option` Signed-off-by: Jiahao XU --- src/helpers/crate_name.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/helpers/crate_name.rs b/src/helpers/crate_name.rs index 0f9ec2f4..500f5614 100644 --- a/src/helpers/crate_name.rs +++ b/src/helpers/crate_name.rs @@ -1,19 +1,22 @@ -use std::{convert::Infallible, fmt, str::FromStr}; +use std::{fmt, str::FromStr}; use compact_str::CompactString; use itertools::Itertools; +use semver::{Error, VersionReq}; + +use super::parse_version; #[derive(Debug, Clone, Eq, PartialEq)] pub struct CrateName { pub name: CompactString, - pub version: Option, + pub version_req: Option, } impl fmt::Display for CrateName { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.name)?; - if let Some(version) = &self.version { + if let Some(version) = &self.version_req { write!(f, "@{version}")?; } @@ -22,18 +25,18 @@ impl fmt::Display for CrateName { } impl FromStr for CrateName { - type Err = Infallible; + type Err = Error; fn from_str(s: &str) -> Result { Ok(if let Some((name, version)) = s.split_once('@') { CrateName { name: name.into(), - version: Some(version.into()), + version_req: Some(parse_version(version)?), } } else { CrateName { name: s.into(), - version: None, + version_req: None, } }) } @@ -60,11 +63,11 @@ mod tests { ([ $( ( $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()) + version_req: Some($input_version.parse().unwrap()) }, )*]; let mut output_crate_names: Vec = vec![$( CrateName { - name: $output_name.into(), version: Some($output_version.into()) + name: $output_name.into(), version_req: Some($output_version.parse().unwrap()) }, )*]; output_crate_names.sort_by(|x, y| x.name.cmp(&y.name)); From 497ef80b27eba895f8701845fdfa319841be61dd Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 18:40:59 +1000 Subject: [PATCH 12/23] Take `&VersionReq` for 1st param of `find_version` instead of `&str` Signed-off-by: Jiahao XU --- src/drivers/version.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/drivers/version.rs b/src/drivers/version.rs index 35e11299..abddbc61 100644 --- a/src/drivers/version.rs +++ b/src/drivers/version.rs @@ -28,15 +28,9 @@ impl Version for crates_io_api::Version { } pub(super) fn find_version>( - requirement: &str, + version_req: &VersionReq, version_iter: VersionIter, ) -> Result<(Item, semver::Version), BinstallError> { - // Parse version requirement - let version_req = VersionReq::parse(requirement).map_err(|err| BinstallError::VersionReq { - req: requirement.into(), - err, - })?; - version_iter // Filter for matching versions .filter_map(|item| { @@ -52,5 +46,7 @@ pub(super) fn find_version>( }) // Return highest version .max_by_key(|(_item, ver)| ver.clone()) - .ok_or(BinstallError::VersionMismatch { req: version_req }) + .ok_or(BinstallError::VersionMismatch { + req: version_req.clone(), + }) } From 686cae6ae835567588e96c0cba9a1200b8aed225 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 18:41:23 +1000 Subject: [PATCH 13/23] Use `&VersionReq` for param `version_req` of `fetch_crate_cratesio` Signed-off-by: Jiahao XU --- src/drivers/crates_io.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/drivers/crates_io.rs b/src/drivers/crates_io.rs index 5d297d40..7dda6fa6 100644 --- a/src/drivers/crates_io.rs +++ b/src/drivers/crates_io.rs @@ -4,6 +4,7 @@ use cargo_toml::Manifest; use crates_io_api::AsyncClient; use log::debug; use reqwest::Client; +use semver::VersionReq; use url::Url; use super::find_version; @@ -19,7 +20,7 @@ pub async fn fetch_crate_cratesio( client: &Client, crates_io_api_client: &AsyncClient, name: &str, - version_req: &str, + version_req: &VersionReq, ) -> Result, BinstallError> { // Fetch / update index debug!("Looking up crate information"); From 065f62a6255c6119c01eed8de63e000c9e9a94f1 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 18:42:59 +1000 Subject: [PATCH 14/23] Use `VersionReq` for `Options::version_req` and update usage of `CrateName` in `binstall::install` Signed-off-by: Jiahao XU --- src/binstall.rs | 4 ++-- src/binstall/install.rs | 4 ++-- src/binstall/resolve.rs | 29 +++++++++++++---------------- src/main.rs | 8 ++++---- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/binstall.rs b/src/binstall.rs index 73c474c3..20b582b7 100644 --- a/src/binstall.rs +++ b/src/binstall.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use compact_str::CompactString; +use semver::VersionReq; use crate::{metafiles::binstall_v1::MetaData, DesiredTargets, PkgOverride}; @@ -14,7 +14,7 @@ pub struct Options { pub no_symlinks: bool, pub dry_run: bool, pub force: bool, - pub version: Option, + pub version_req: Option, pub manifest_path: Option, pub cli_overrides: PkgOverride, pub desired_targets: DesiredTargets, diff --git a/src/binstall/install.rs b/src/binstall/install.rs index 21015333..0bf5c0e7 100644 --- a/src/binstall/install.rs +++ b/src/binstall/install.rs @@ -19,7 +19,7 @@ pub async fn install( fetcher, package, name, - version, + version_req, bin_path, bin_files, } => { @@ -31,7 +31,7 @@ pub async fn install( .map(|option| { option.map(|bins| MetaData { name, - version_req: version, + version_req, current_version, source: Source::cratesio_registry(), target, diff --git a/src/binstall/resolve.rs b/src/binstall/resolve.rs index 6290deef..d17747b4 100644 --- a/src/binstall/resolve.rs +++ b/src/binstall/resolve.rs @@ -4,10 +4,11 @@ use std::{ }; use cargo_toml::{Package, Product}; -use compact_str::{format_compact, CompactString}; +use compact_str::{CompactString, ToCompactString}; use log::{debug, error, info, warn}; use miette::{miette, Result}; use reqwest::Client; +use semver::VersionReq; use super::Options; use crate::{ @@ -21,7 +22,7 @@ pub enum Resolution { fetcher: Arc, package: Package, name: CompactString, - version: CompactString, + version_req: CompactString, bin_path: PathBuf, bin_files: Vec, }, @@ -83,30 +84,26 @@ pub async fn resolve( ) -> Result { info!("Installing package: '{}'", crate_name); - let mut version: CompactString = match (&crate_name.version, &opts.version) { + let version_req: VersionReq = match (&crate_name.version_req, &opts.version_req) { (Some(version), None) => version.clone(), (None, Some(version)) => version.clone(), (Some(_), Some(_)) => Err(BinstallError::SuperfluousVersionOption)?, - (None, None) => "*".into(), + (None, None) => VersionReq::STAR, }; - // Treat 0.1.2 as =0.1.2 - if version - .chars() - .next() - .map(|ch| ch.is_ascii_digit()) - .unwrap_or(false) - { - version = format_compact!("={version}"); - } - // Fetch crate via crates.io, git, or use a local manifest path // TODO: work out which of these to do based on `opts.name` // TODO: support git-based fetches (whole repo name rather than just crate name) let manifest = match opts.manifest_path.clone() { Some(manifest_path) => load_manifest_path(manifest_path)?, None => { - fetch_crate_cratesio(&client, &crates_io_api_client, &crate_name.name, &version).await? + fetch_crate_cratesio( + &client, + &crates_io_api_client, + &crate_name.name, + &version_req, + ) + .await? } }; @@ -175,7 +172,7 @@ pub async fn resolve( fetcher, package, name: crate_name.name, - version, + version_req: version_req.to_compact_string(), bin_path, bin_files, } diff --git a/src/main.rs b/src/main.rs index da87fc49..2aa7e58f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,9 +9,9 @@ use std::{ }; use clap::{builder::PossibleValue, AppSettings, Parser}; -use compact_str::CompactString; use log::{debug, error, info, warn, LevelFilter}; use miette::{miette, Result, WrapErr}; +use semver::VersionReq; use simplelog::{ColorChoice, ConfigBuilder, TermLogger, TerminalMode}; use tokio::{runtime::Runtime, task::block_in_place}; @@ -46,8 +46,8 @@ struct Options { /// /// Cannot be used when multiple packages are installed at once, use the attached version /// syntax in that case. - #[clap(help_heading = "Package selection", long = "version")] - version_req: Option, + #[clap(help_heading = "Package selection", long = "version", parse(try_from_str = parse_version))] + version_req: Option, /// Override binary target set. /// @@ -371,7 +371,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { no_symlinks: opts.no_symlinks, dry_run: opts.dry_run, force: opts.force, - version: opts.version_req.take(), + version_req: opts.version_req.take(), manifest_path: opts.manifest_path.take(), cli_overrides, desired_targets, From 431992dab0760034502db105da85ca34e0356290 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 18:45:53 +1000 Subject: [PATCH 15/23] Test `--version` and `$crate_name@$version` in `tests.sh` Signed-off-by: Jiahao XU --- ci-scripts/tests.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ci-scripts/tests.sh b/ci-scripts/tests.sh index 41d10b59..64ed0f9a 100755 --- a/ci-scripts/tests.sh +++ b/ci-scripts/tests.sh @@ -36,3 +36,13 @@ min_tls=1.3 cargo-binstall # Test that the installed binaries can be run cargo binstall --help >/dev/null + +# Test --version +"./$1" binstall --force --log-level debug --no-confirm --version 0.11.1 cargo-binstall +# Test that the installed binaries can be run +cargo binstall --help >/dev/null + +# Test "$crate_name@$version" +"./$1" binstall --force --log-level debug --no-confirm cargo-binstall@0.11.1 +# Test that the installed binaries can be run +cargo binstall --help >/dev/null From 2c2b3c070f9f37737c00342c2ee14fc07cef928f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 19:11:25 +1000 Subject: [PATCH 16/23] Skip only if `version_req` is satisfied Signed-off-by: Jiahao XU --- src/main.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 2aa7e58f..ae340ce2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -358,7 +358,14 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { if opts.force { true } else if let Some(records) = &metadata { - !records.contains(&crate_name.name) + if let Some(version_req) = &crate_name.version_req { + records + .get(&crate_name.name) + .map(|metadata| !version_req.matches(&metadata.current_version)) + .unwrap_or(true) + } else { + !records.contains(&crate_name.name) + } } else { true } From 7f4edfd9f24fd43ee65776b1aa4ba27f4cb0429b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 19:15:13 +1000 Subject: [PATCH 17/23] `log::info!` if a crate is skipped due to already installed Signed-off-by: Jiahao XU --- src/main.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index ae340ce2..ecc92f04 100644 --- a/src/main.rs +++ b/src/main.rs @@ -358,14 +358,20 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { if opts.force { true } else if let Some(records) = &metadata { - if let Some(version_req) = &crate_name.version_req { + let keep = if let Some(version_req) = &crate_name.version_req { records .get(&crate_name.name) .map(|metadata| !version_req.matches(&metadata.current_version)) .unwrap_or(true) } else { !records.contains(&crate_name.name) + }; + + if !keep { + info!("package {crate_name} is already installed, use --force to override") } + + keep } else { true } From 4a174602525de3c5fcd8b5f116f676fb09de9e21 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 19:17:52 +1000 Subject: [PATCH 18/23] Test skip when installed in `ci-scripts/tests.sh` Signed-off-by: Jiahao XU --- ci-scripts/tests.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci-scripts/tests.sh b/ci-scripts/tests.sh index 64ed0f9a..d72c548c 100755 --- a/ci-scripts/tests.sh +++ b/ci-scripts/tests.sh @@ -46,3 +46,9 @@ cargo binstall --help >/dev/null "./$1" binstall --force --log-level debug --no-confirm cargo-binstall@0.11.1 # Test that the installed binaries can be run cargo binstall --help >/dev/null + +# Test skip when installed +"./$1" binstall --no-confirm cargo-binstall | grep -q 'package cargo-binstall is already installed' +"./$1" binstall --no-confirm cargo-binstall@0.11.1 | grep -q 'package cargo-binstall is already installed' + +"./$1" binstall --no-confirm cargo-binstall@0.10.0 | grep -q -v 'package cargo-binstall is already installed' From d932f1c262999f698346fd3f68046a1329a0b9dd Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 7 Aug 2022 19:27:30 +1000 Subject: [PATCH 19/23] Fix testing skip if installed in `ci-scripts/tests.sh` Signed-off-by: Jiahao XU --- ci-scripts/tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci-scripts/tests.sh b/ci-scripts/tests.sh index d72c548c..a9c478b6 100755 --- a/ci-scripts/tests.sh +++ b/ci-scripts/tests.sh @@ -49,6 +49,6 @@ cargo binstall --help >/dev/null # Test skip when installed "./$1" binstall --no-confirm cargo-binstall | grep -q 'package cargo-binstall is already installed' -"./$1" binstall --no-confirm cargo-binstall@0.11.1 | grep -q 'package cargo-binstall is already installed' +"./$1" binstall --no-confirm cargo-binstall@0.11.1 | grep -q 'package cargo-binstall@=0.11.1 is already installed' -"./$1" binstall --no-confirm cargo-binstall@0.10.0 | grep -q -v 'package cargo-binstall is already installed' +"./$1" binstall --no-confirm cargo-binstall@0.10.0 | grep -q -v 'package cargo-binstall@=0.10.0 is already installed' From b94dc979acc5585bb886cdadd7c7adabf08b5cbe Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 8 Aug 2022 17:59:39 +1000 Subject: [PATCH 20/23] Add new trait `VersionReqExt` and impl it for `VersionReq` Signed-off-by: Jiahao XU --- src/helpers.rs | 3 ++ src/helpers/version.rs | 78 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 src/helpers/version.rs diff --git a/src/helpers.rs b/src/helpers.rs index b311f75d..b1b67ddf 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -51,6 +51,9 @@ pub use flock::FileLock; mod signal; pub use signal::cancel_on_user_sig_term; +mod version; +pub use version::VersionReqExt; + pub fn cargo_home() -> Result<&'static Path, io::Error> { static CARGO_HOME: OnceCell = OnceCell::new(); diff --git a/src/helpers/version.rs b/src/helpers/version.rs new file mode 100644 index 00000000..f994dc93 --- /dev/null +++ b/src/helpers/version.rs @@ -0,0 +1,78 @@ +use compact_str::format_compact; +use semver::{Prerelease, Version, VersionReq}; + +/// Extension trait for [`VersionReq`]. +pub trait VersionReqExt { + /// Return `true` if `self.matches(version)` returns `true` + /// and the `version` is the latest one acceptable by `self`. + fn is_latest_compatible(&self, version: &Version) -> bool; +} + +impl VersionReqExt for VersionReq { + fn is_latest_compatible(&self, version: &Version) -> bool { + if !self.matches(version) { + return false; + } + + // Test if bumping patch will be accepted + let bumped_version = Version::new(version.major, version.minor, version.patch + 1); + + if self.matches(&bumped_version) { + return false; + } + + // Test if bumping prerelease will be accepted if version has one. + let pre = &version.pre; + if !pre.is_empty() { + // Bump pre by appending random number to the end. + let bumped_pre = format_compact!("{}.1", pre.as_str()); + + let bumped_version = Version { + major: version.major, + minor: version.minor, + patch: version.patch, + pre: Prerelease::new(&bumped_pre).unwrap(), + build: Default::default(), + }; + + if self.matches(&bumped_version) { + return false; + } + } + + true + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test() { + // Test star + assert!(!VersionReq::STAR.is_latest_compatible(&Version::parse("0.0.1").unwrap())); + assert!(!VersionReq::STAR.is_latest_compatible(&Version::parse("0.1.1").unwrap())); + assert!(!VersionReq::STAR.is_latest_compatible(&Version::parse("0.1.1-alpha").unwrap())); + + // Test ^x.y.z + assert!(!VersionReq::parse("^0.1") + .unwrap() + .is_latest_compatible(&Version::parse("0.1.99").unwrap())); + + // Test =x.y.z + assert!(VersionReq::parse("=0.1.0") + .unwrap() + .is_latest_compatible(&Version::parse("0.1.0").unwrap())); + + // Test =x.y.z-alpha + assert!(VersionReq::parse("=0.1.0-alpha") + .unwrap() + .is_latest_compatible(&Version::parse("0.1.0-alpha").unwrap())); + + // Test >=x.y.z-alpha + assert!(!VersionReq::parse(">=0.1.0-alpha") + .unwrap() + .is_latest_compatible(&Version::parse("0.1.0-alpha").unwrap())); + } +} From 36926518cfa835a2339e965bb0871ddbf2431712 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 8 Aug 2022 18:01:08 +1000 Subject: [PATCH 21/23] Use `VersionReqExt::is_latest_compatible` in `entry:367` 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 ecc92f04..687fc906 100644 --- a/src/main.rs +++ b/src/main.rs @@ -361,7 +361,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { let keep = if let Some(version_req) = &crate_name.version_req { records .get(&crate_name.name) - .map(|metadata| !version_req.matches(&metadata.current_version)) + .map(|metadata| !version_req.is_latest_compatible(&metadata.current_version)) .unwrap_or(true) } else { !records.contains(&crate_name.name) From b69d7e7c0664689676ad760834c12f4ac20795a1 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 8 Aug 2022 18:03:59 +1000 Subject: [PATCH 22/23] Test When 0.11.0 is installed but can be upgraded in `tests.sh` Signed-off-by: Jiahao XU --- ci-scripts/tests.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci-scripts/tests.sh b/ci-scripts/tests.sh index a9c478b6..e38259b6 100755 --- a/ci-scripts/tests.sh +++ b/ci-scripts/tests.sh @@ -52,3 +52,7 @@ cargo binstall --help >/dev/null "./$1" binstall --no-confirm cargo-binstall@0.11.1 | grep -q 'package cargo-binstall@=0.11.1 is already installed' "./$1" binstall --no-confirm cargo-binstall@0.10.0 | grep -q -v 'package cargo-binstall@=0.10.0 is already installed' + +## Test When 0.11.0 is installed but can be upgraded. +"./$1" binstall --force --log-level debug --no-confirm cargo-binstall@0.11.0 +"./$1" binstall --no-confirm cargo-binstall@^0.11.0 | grep -q -v 'package cargo-binstall@^0.11.0 is already installed' From a271e695a560af202c39421818af042afcfb3bfd Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 8 Aug 2022 19:18:30 +1000 Subject: [PATCH 23/23] Ignore if package already up-to-date Signed-off-by: Jiahao XU --- src/binstall/install.rs | 1 + src/binstall/resolve.rs | 18 +++++++++++++++++- src/main.rs | 39 ++++++++++++++++++++++----------------- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/binstall/install.rs b/src/binstall/install.rs index 0bf5c0e7..9e57d6c1 100644 --- a/src/binstall/install.rs +++ b/src/binstall/install.rs @@ -15,6 +15,7 @@ pub async fn install( jobserver_client: LazyJobserverClient, ) -> Result> { match resolution { + Resolution::AlreadyUpToDate => Ok(None), Resolution::Fetch { fetcher, package, diff --git a/src/binstall/resolve.rs b/src/binstall/resolve.rs index d17747b4..0869106a 100644 --- a/src/binstall/resolve.rs +++ b/src/binstall/resolve.rs @@ -8,7 +8,7 @@ use compact_str::{CompactString, ToCompactString}; use log::{debug, error, info, warn}; use miette::{miette, Result}; use reqwest::Client; -use semver::VersionReq; +use semver::{Version, VersionReq}; use super::Options; use crate::{ @@ -29,6 +29,7 @@ pub enum Resolution { InstallFromSource { package: Package, }, + AlreadyUpToDate, } impl Resolution { fn print(&self, opts: &Options) { @@ -70,6 +71,7 @@ impl Resolution { Resolution::InstallFromSource { .. } => { warn!("The package will be installed from source (with cargo)",) } + Resolution::AlreadyUpToDate => (), } } } @@ -77,6 +79,7 @@ impl Resolution { pub async fn resolve( opts: Arc, crate_name: CrateName, + curr_version: Option, temp_dir: Arc, install_path: Arc, client: Client, @@ -109,6 +112,19 @@ pub async fn resolve( let package = manifest.package.unwrap(); + if let Some(curr_version) = curr_version { + let new_version = + Version::parse(&package.version).map_err(|err| BinstallError::VersionParse { + v: package.version.clone(), + err, + })?; + + if new_version == curr_version { + info!("package {crate_name} is already up to date {curr_version}"); + return Ok(Resolution::AlreadyUpToDate); + } + } + let (mut meta, binaries) = ( package .metadata diff --git a/src/main.rs b/src/main.rs index 687fc906..55628a22 100644 --- a/src/main.rs +++ b/src/main.rs @@ -354,26 +354,29 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { })?; // Remove installed crates - let crate_names = CrateName::dedup(crate_names).filter(|crate_name| { + let crate_names = CrateName::dedup(crate_names).filter_map(|crate_name| { if opts.force { - true + Some((crate_name, None)) } else if let Some(records) = &metadata { - let keep = if let Some(version_req) = &crate_name.version_req { - records - .get(&crate_name.name) - .map(|metadata| !version_req.is_latest_compatible(&metadata.current_version)) - .unwrap_or(true) + if let Some(metadata) = records.get(&crate_name.name) { + if let Some(version_req) = &crate_name.version_req { + if version_req.is_latest_compatible(&metadata.current_version) { + info!( + "package {crate_name} is already installed and cannot be upgraded, use --force to override" + ); + None + } else { + Some((crate_name, Some(metadata.current_version.clone()))) + } + } else { + info!("package {crate_name} is already installed, use --force to override"); + None + } } else { - !records.contains(&crate_name.name) - }; - - if !keep { - info!("package {crate_name} is already installed, use --force to override") + Some((crate_name, None)) } - - keep } else { - true + Some((crate_name, None)) } }); @@ -395,10 +398,11 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { // Resolve crates let tasks: Vec<_> = crate_names .into_iter() - .map(|crate_name| { + .map(|(crate_name, current_version)| { AutoAbortJoinHandle::spawn(binstall::resolve( binstall_opts.clone(), crate_name, + current_version, temp_dir_path.clone(), install_path.clone(), client.clone(), @@ -430,7 +434,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { // Resolve crates and install without confirmation crate_names .into_iter() - .map(|crate_name| { + .map(|(crate_name, current_version)| { let opts = binstall_opts.clone(); let temp_dir_path = temp_dir_path.clone(); let jobserver_client = jobserver_client.clone(); @@ -442,6 +446,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> { let resolution = binstall::resolve( opts.clone(), crate_name, + current_version, temp_dir_path, install_path, client,