Auto merge of #4229 - azerupi:install-version-range, r=matklad

bors · bors · commit 64c3217eb459 · 2017-06-28T06:39:50.000Z
Implement semver ranges for install --vers This implements the design discussed in #4207 It allows to specify semver ranges on `cargo install ... --vers` 1. The first character of the `--vers` value is checked, if there is none we return an error. 2. If it is one of `<`, `>`, `=`, `^`, `~` we parse the value as a `VersionReq`, otherwise we parse it as a `Version`. 3. If the parsing as a `Version` fails but parsing as `VersionReq` succeeds, we add a note to the warning mentioning that a qualifier should be used to specify a semver range. This catches versions with less than tree digits. Otherwise, the previous behaviour is preserved with the warning of backwards compatibility. This means that - `cargo install ... --vers "^1.2.3"` will be parsed as a range - `cargo install ... --vers 1.2.3` will be parsed as a version - `cargo install ... --vers 1.2` will be parsed as a version for backwards compatibility reasons, fail and be passed through as is,**but** we will add a note `if you want to specify semver range, add an explicit qualifier, like ^1.2` - `cargo install ... --vers blah` will be parsed as a version for backwards compatibility reasons (which is weird because it is not even a valid semver range) and produce an `unknown error` down the line. I have left this behaviour untouched because it worked like that before, but I can easily make it error sooner with a better message.
diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs
@@ -8,7 +8,7 @@ use std::io::SeekFrom;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-use semver::Version;
+use semver::{Version, VersionReq};
 use tempdir::TempDir;
 use toml;
 
@@ -276,18 +276,45 @@ fn select_pkg<'a, T>(mut source: T,
         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())
+
+                    // If the version begins with character <, >, =, ^, ~ parse it as a
+                    // version range, otherwise parse it as a specific version
+                    let first = v.chars()
+                        .nth(0)
+                        .ok_or("no version provided for the `--vers` flag")?;
+
+                    match first {
+                        '<' | '>' | '=' | '^' | '~' => match v.parse::<VersionReq>() {
+                            Ok(v) => Some(v.to_string()),
+                            Err(_) => {
+                                let msg = format!("the `--vers` provided, `{}`, is \
+                                                   not a valid semver version requirement\n\n
+                                                   Please have a look at \
+                                                   http://doc.crates.io/specifying-dependencies.html \
+                                                   for the correct format", v);
+                                return Err(msg.into());
+                            }
+                        },
+                        _ => match v.parse::<Version>() {
+                            Ok(v) => Some(format!("={}", v)),
+                            Err(_) => {
+                                let mut 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);
+
+                                // If it is not a valid version but it is a valid version
+                                // requirement, add a note to the warning
+                                if v.parse::<VersionReq>().is_ok() {
+                                    msg.push_str(&format!("\nif you want to specify semver range, \
+                                                  add an explicit qualifier, like ^{}", v));
+                                }
+                                config.shell().warn(&msg)?;
+                                Some(v.to_string())
+                            }
                         }
                     }
                 }
