Auto merge of #3338 - alexcrichton:correct-vers-flag, r=brson

bors · bors · commit 787591c4e45d · 2016-12-02T13:23:38.000Z
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
diff --git 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,13 +252,34 @@ 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<Vec<Package>>)
                      -> CargoResult<(Package, Box<Source + 'a>)>
     where T: Source + 'a
 {
     source.update()?;
     match name {
         Some(name) => {
+            let vers = match vers {
+                Some(v) => {
+                    match v.parse::<Version>() {
+                        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
@@ -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
+"));
+}
