From a721e6278182f79ba97eb114061be05cf766c9c3 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 19 Mar 2020 23:29:31 +0000 Subject: [PATCH] Try installing exact versions before updating When an exact version is being installed, if we already have that version from the index, we don't need to update the index before installing it. Don't do this if it's not an exact version, because the update may find us a newer version. This is particularly useful for scripts which unconditionally run `cargo install some-crate --version=1.2.3`. Before install-update, I wrote a crate to do this (https://crates.io/crates/cargo-ensure-installed) which I'm trying to replace with just `cargo install`, but the extra latency of updating the index for a no-op is noticeable. This introduces an interesting edge-case around yanked crates; the yanked-ness of crates will no longer change on install for exact version matches which were already downloaded. This feels niche enough to hopefully not matter... --- src/cargo/ops/cargo_install.rs | 10 +++++-- src/cargo/ops/cargo_uninstall.rs | 2 +- .../ops/common_for_install_and_uninstall.rs | 28 ++++++++++++++++--- tests/testsuite/install_upgrade.rs | 3 +- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 0ff2edbc6..b2759366e 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -151,7 +151,7 @@ fn install_one( krate, vers, config, - true, + NeedsUpdate::True, &mut |git| git.read_packages(), )? } else if source_id.is_path() { @@ -180,7 +180,7 @@ fn install_one( } } src.update()?; - select_pkg(src, krate, vers, config, false, &mut |path| { + select_pkg(src, krate, vers, config, NeedsUpdate::False, &mut |path| { path.read_packages() })? } else { @@ -189,7 +189,11 @@ fn install_one( krate, vers, config, - is_first_install, + if is_first_install { + NeedsUpdate::TryWithoutFirst + } else { + NeedsUpdate::False + }, &mut |_| { bail!( "must specify a crate to install from \ diff --git a/src/cargo/ops/cargo_uninstall.rs b/src/cargo/ops/cargo_uninstall.rs index 2d69eabd0..612db0129 100644 --- a/src/cargo/ops/cargo_uninstall.rs +++ b/src/cargo/ops/cargo_uninstall.rs @@ -85,7 +85,7 @@ fn uninstall_cwd(root: &Filesystem, bins: &[String], config: &Config) -> CargoRe let tracker = InstallTracker::load(config, root)?; let source_id = SourceId::for_path(config.cwd())?; let src = path_source(source_id, config)?; - let pkg = select_pkg(src, None, None, config, true, &mut |path| { + let pkg = select_pkg(src, None, None, config, NeedsUpdate::True, &mut |path| { path.read_packages() })?; let pkgid = pkg.package_id(); diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index aa5c4ee18..b61ebc5c8 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -525,13 +525,20 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult( mut source: T, name: Option<&str>, vers: Option<&str>, config: &Config, - needs_update: bool, + needs_update: NeedsUpdate, list_all: &mut dyn FnMut(&mut T) -> CargoResult>, ) -> CargoResult where @@ -542,7 +549,7 @@ where // with other global Cargos let _lock = config.acquire_package_cache_lock()?; - if needs_update { + if let NeedsUpdate::True = needs_update { source.update()?; } @@ -603,8 +610,21 @@ where vers }; let dep = Dependency::parse_no_deprecated(name, vers_spec, source.source_id())?; - let deps = source.query_vec(&dep)?; - match deps.iter().map(|p| p.package_id()).max() { + let deps = (|| { + if let Some(vers_spec) = vers_spec { + if semver::VersionReq::parse(vers_spec).unwrap().is_exact() { + let deps = source.query_vec(&dep)?; + if deps.len() == 1 { + return Ok(deps); + } + } + } + if needs_update != NeedsUpdate::False { + source.update()?; + } + source.query_vec(&dep) + })(); + match deps?.iter().map(|p| p.package_id()).max() { Some(pkgid) => { let pkg = Box::new(&mut source).download_now(pkgid, config)?; Ok(pkg) diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index ee69bd85b..3d51ad625 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -225,8 +225,7 @@ fn ambiguous_version_no_longer_allowed() { cargo_process("install foo --version=1.0") .with_stderr( "\ -[UPDATING] `[..]` index -[ERROR] the `--vers` provided, `1.0`, is not a valid semver version: cannot parse '1.0' as a semver +error: the `--vers` provided, `1.0`, is not a valid semver version: cannot parse '1.0' as a semver if you want to specify semver range, add an explicit qualifier, like ^1.0 ",