Skip to content

Commit

Permalink
Try installing exact versions before updating
Browse files Browse the repository at this point in the history
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...
  • Loading branch information
illicitonion committed Mar 20, 2020
1 parent e8b0344 commit a721e62
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 10 deletions.
10 changes: 7 additions & 3 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn install_one(
krate,
vers,
config,
true,
NeedsUpdate::True,
&mut |git| git.read_packages(),
)?
} else if source_id.is_path() {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
28 changes: 24 additions & 4 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,13 +525,20 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult<PathSour
Ok(PathSource::new(&path, source_id, config))
}

#[derive(Clone, Copy, PartialEq, Eq)]
pub enum NeedsUpdate {
True,
False,
TryWithoutFirst,
}

/// Gets a Package based on command-line requirements.
pub fn select_pkg<'a, T>(
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<Vec<Package>>,
) -> CargoResult<Package>
where
Expand All @@ -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()?;
}

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/install_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
",
Expand Down

0 comments on commit a721e62

Please sign in to comment.