Merge pull request #282 from cargo-bins/skip-if-already-installed

Skip if already installed and add new cmdline option `--force`

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 \
@@ -35,3 +36,23 @@ 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
+
+# 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@=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'

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};
 
@@ -13,7 +13,8 @@ pub use install::*;
 pub struct Options {
     pub no_symlinks: bool,
     pub dry_run: bool,
-    pub version: Option<CompactString>,
+    pub force: bool,
+    pub version_req: Option<VersionReq>,
     pub manifest_path: Option<PathBuf>,
     pub cli_overrides: PkgOverride,
     pub desired_targets: DesiredTargets,

src/binstall/install.rs

@@ -15,11 +15,12 @@ pub async fn install(
     jobserver_client: LazyJobserverClient,
 ) -> Result<Option<MetaData>> {
     match resolution {
+        Resolution::AlreadyUpToDate => Ok(None),
         Resolution::Fetch {
             fetcher,
             package,
             name,
-            version,
+            version_req,
             bin_path,
             bin_files,
         } => {
@@ -31,7 +32,7 @@ pub async fn install(
                 .map(|option| {
                     option.map(|bins| MetaData {
                         name,
-                        version_req: version,
+                        version_req,
                         current_version,
                         source: Source::cratesio_registry(),
                         target,
@@ -47,7 +48,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 +128,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 +152,10 @@ async fn install_from_source(
         cmd.arg("--quiet");
     }
 
+    if force {
+        cmd.arg("--force");
+    }
+
     let mut child = cmd
         .spawn()
         .into_diagnostic()

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::{Version, VersionReq};
 
 use super::Options;
 use crate::{
@@ -21,13 +22,14 @@ pub enum Resolution {
         fetcher: Arc<dyn Fetcher>,
         package: Package<Meta>,
         name: CompactString,
-        version: CompactString,
+        version_req: CompactString,
         bin_path: PathBuf,
         bin_files: Vec<bins::BinFile>,
     },
     InstallFromSource {
         package: Package<Meta>,
     },
+    AlreadyUpToDate,
 }
 impl Resolution {
     fn print(&self, opts: &Options) {
@@ -69,6 +71,7 @@ impl Resolution {
             Resolution::InstallFromSource { .. } => {
                 warn!("The package will be installed from source (with cargo)",)
             }
+            Resolution::AlreadyUpToDate => (),
         }
     }
 }
@@ -76,6 +79,7 @@ impl Resolution {
 pub async fn resolve(
     opts: Arc<Options>,
     crate_name: CrateName,
+    curr_version: Option<Version>,
     temp_dir: Arc<Path>,
     install_path: Arc<Path>,
     client: Client,
@@ -83,35 +87,44 @@ pub async fn resolve(
 ) -> Result<Resolution> {
     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?
         }
     };
 
     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
@@ -175,7 +188,7 @@ pub async fn resolve(
                 fetcher,
                 package,
                 name: crate_name.name,
-                version,
+                version_req: version_req.to_compact_string(),
                 bin_path,
                 bin_files,
             }

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<Manifest<Meta>, BinstallError> {
     // Fetch / update index
     debug!("Looking up crate information");

src/drivers/version.rs

@@ -28,15 +28,9 @@ impl Version for crates_io_api::Version {
 }
 
 pub(super) fn find_version<Item: Version, VersionIter: Iterator<Item = Item>>(
-    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<Item: Version, VersionIter: Iterator<Item = Item>>(
         })
         // 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(),
+        })
 }

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};
@@ -50,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<PathBuf> = OnceCell::new();
 
@@ -86,6 +90,20 @@ pub async fn await_task<T>(task: tokio::task::JoinHandle<miette::Result<T>>) ->
     }
 }
 
+pub fn parse_version(version: &str) -> Result<semver::VersionReq, semver::Error> {
+    // 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<P: AsRef<Path>>(
     manifest_path: P,

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<CompactString>,
+    pub version_req: Option<VersionReq>,
 }
 
 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<Self, Self::Err> {
         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<CrateName> = 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));
 

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()));
+    }
+}

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")]
+    #[clap(help_heading = "Package selection", long = "version", parse(try_from_str = parse_version))]
-    version_req: Option<CompactString>,
+    version_req: Option<VersionReq>,
 
     /// Override binary target set.
     ///
@@ -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
@@ -281,15 +285,15 @@ 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(),
         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()))?;
 
@@ -317,28 +321,64 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
     // Initialize UI thread
     let mut uithread = UIThread::new(!opts.no_confirm);
 
-    // Launch target detection
+    let (install_path, metadata, temp_dir) = block_in_place(|| -> Result<_> {
-    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(|| {
+            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());
 
-    // Compute install directory
+        // Load metadata
-    let (install_path, custom_install_path) = get_install_path(opts.install_path.as_deref());
+        let metadata = if !custom_install_path {
-    let install_path = install_path.ok_or_else(|| {
+            debug!("Reading binstall/crates-v1.json");
-        error!("No viable install path found of specified, try `--install-path`");
+            Some(metafiles::binstall_v1::Records::load()?)
-        miette!("No install path found or specified")
+        } else {
+            None
+        };
+
+        // 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))
     })?;
-    fs::create_dir_all(&install_path).map_err(BinstallError::Io)?;
-    debug!("Using install path: {}", install_path.display());
 
-    // Create a temporary directory for downloads etc.
+    // Remove installed crates
-    //
+    let crate_names = CrateName::dedup(crate_names).filter_map(|crate_name| {
-    // Put all binaries to a temporary directory under `dst` first, catching
+        if opts.force {
-    // some failure modes (e.g., out of space) before touching the existing
+            Some((crate_name, None))
-    // binaries. This directory will get cleaned up via RAII.
+        } else if let Some(records) = &metadata {
-    let temp_dir = tempfile::Builder::new()
+            if let Some(metadata) = records.get(&crate_name.name) {
-        .prefix("cargo-binstall")
+                if let Some(version_req) = &crate_name.version_req {
-        .tempdir_in(&install_path)
+                    if version_req.is_latest_compatible(&metadata.current_version) {
-        .map_err(BinstallError::from)
+                        info!(
-        .wrap_err("Creating a temporary directory failed.")?;
+                            "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 {
+                Some((crate_name, None))
+            }
+        } else {
+            Some((crate_name, None))
+        }
+    });
 
     let temp_dir_path: Arc<Path> = Arc::from(temp_dir.path());
 
@@ -346,7 +386,8 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
     let binstall_opts = Arc::new(binstall::Options {
         no_symlinks: opts.no_symlinks,
         dry_run: opts.dry_run,
-        version: opts.version_req.take(),
+        force: opts.force,
+        version_req: opts.version_req.take(),
         manifest_path: opts.manifest_path.take(),
         cli_overrides,
         desired_targets,
@@ -357,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(),
@@ -392,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();
@@ -404,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,
@@ -425,7 +468,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.
@@ -434,7 +477,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 {