From d91c9b405777cb6a70dd6d0393511dd4917cfefd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 28 Nov 2016 09:48:14 -0800 Subject: [PATCH] Require `cargo install --vers` takes a semver version Historically Cargo accidentally took a semver version *requirement*, so let's start issuing warnings about how this is now legacy behavior. Closes #3321 --- src/cargo/ops/cargo_install.rs | 28 +++++++++++++++++++++++++--- tests/install.rs | 26 +++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index c2a348e2e99..f6715992fd3 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -7,6 +7,7 @@ use std::io::prelude::*; use std::io::SeekFrom; use std::path::{Path, PathBuf}; +use semver::Version; use tempdir::TempDir; use toml; @@ -57,7 +58,7 @@ pub fn install(root: Option<&str>, let map = SourceConfigMap::new(config)?; let (pkg, source) = if source_id.is_git() { select_pkg(GitSource::new(source_id, config), source_id, - krate, vers, &mut |git| git.read_packages())? + krate, vers, config, &mut |git| git.read_packages())? } else if source_id.is_path() { let path = source_id.url().to_file_path().ok() .expect("path sources must have a valid path"); @@ -68,11 +69,11 @@ pub fn install(root: Option<&str>, specify an alternate source", path.display())) })?; select_pkg(PathSource::new(&path, source_id, config), - source_id, krate, vers, + source_id, krate, vers, config, &mut |path| path.read_packages())? } else { select_pkg(map.load(source_id)?, - source_id, krate, vers, + source_id, krate, vers, config, &mut |_| Err(human("must specify a crate to install from \ crates.io, or use --path or --git to \ specify alternate source")))? @@ -251,6 +252,7 @@ fn select_pkg<'a, T>(mut source: T, source_id: &SourceId, name: Option<&str>, vers: Option<&str>, + config: &Config, list_all: &mut FnMut(&mut T) -> CargoResult>) -> CargoResult<(Package, Box)> where T: Source + 'a @@ -258,6 +260,26 @@ fn select_pkg<'a, T>(mut source: T, source.update()?; match name { Some(name) => { + let vers = match vers { + Some(v) => { + match v.parse::() { + Ok(v) => Some(format!("={}", v)), + Err(_) => { + let msg = format!("the `--vers` provided, `{}`, is \ + not a valid semver version\n\n\ + historically Cargo treated this \ + as a semver version requirement \ + accidentally\nand will continue \ + to do so, but this behavior \ + will be removed eventually", v); + config.shell().warn(&msg)?; + Some(v.to_string()) + } + } + } + None => None, + }; + let vers = vers.as_ref().map(|s| &**s); let dep = Dependency::parse_no_deprecated(name, vers, source_id)?; let deps = source.query(&dep)?; match deps.iter().map(|p| p.package_id()).max() { diff --git a/tests/install.rs b/tests/install.rs index 6d064703fe2..eb77c0a2e41 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -87,7 +87,7 @@ fn bad_version() { assert_that(cargo_process("install").arg("foo").arg("--vers=0.2.0"), execs().with_status(101).with_stderr("\ [UPDATING] registry [..] -[ERROR] could not find `foo` in `registry [..]` with version `0.2.0` +[ERROR] could not find `foo` in `registry [..]` with version `=0.2.0` ")); } @@ -826,3 +826,27 @@ fn use_path_workspace() { let lock2 = p.read_lockfile(); assert!(lock == lock2, "different lockfiles"); } + +#[test] +fn vers_precise() { + pkg("foo", "0.1.1"); + pkg("foo", "0.1.2"); + + assert_that(cargo_process("install").arg("foo").arg("--vers").arg("0.1.1"), + execs().with_status(0).with_stderr_contains("\ +[DOWNLOADING] foo v0.1.1 (registry [..]) +")); +} + +#[test] +fn legacy_version_requirement() { + pkg("foo", "0.1.1"); + + assert_that(cargo_process("install").arg("foo").arg("--vers").arg("0.1"), + execs().with_status(0).with_stderr_contains("\ +warning: the `--vers` provided, `0.1`, is not a valid semver version + +historically Cargo treated this as a semver version requirement accidentally +and will continue to do so, but this behavior will be removed eventually +")); +}