Auto merge of #3154 - steveklabnik:gh3124, r=alexcrichton

upgrade semver

This is a spike at fixing #3124.

@alexcrichton I'm not sure how to actually emit the error message here. In the TOML example you linked me: https://github.com/rust-lang/cargo/blob/5593045ddef2744c1042dee0c0037c2ebcc1834e/src/cargo/util/toml.rs#L166

That takes a Config as an argument. This function does not. What's the right approach here?

Also, this code is messy. I am 99% sure I can write it nicer with combinators. This is just to get the discussion going.

Cargo.toml

@@ -38,7 +38,7 @@ num_cpus = "1.0"
 psapi-sys = "0.1"
 regex = "0.1"
 rustc-serialize = "0.3"
-semver = "0.2.3"
+semver = "0.5.0"
 tar = { version = "0.4", default-features = false }
 tempdir = "0.3"
 term = "0.4.4"

src/cargo/core/dependency.rs

@@ -3,10 +3,11 @@ use std::rc::Rc;
 use std::str::FromStr;
 
 use semver::VersionReq;
+use semver::ReqParseError;
 use rustc_serialize::{Encoder, Encodable};
 
 use core::{SourceId, Summary, PackageId};
-use util::{CargoError, CargoResult, Cfg, CfgExpr, ChainError, human};
+use util::{CargoError, CargoResult, Cfg, CfgExpr, ChainError, human, Config};
 
 /// Information about a dependency requested by a Cargo manifest.
 /// Cheap to copy.
@@ -87,11 +88,21 @@ impl Encodable for Kind {
 
 impl DependencyInner {
     /// Attempt to create a `Dependency` from an entry in the manifest.
+    ///
+    /// The `deprecated_extra` information is set to `Some` when this method
+    /// should *also* parse historically deprecated semver requirements. This
+    /// information allows providing diagnostic information about what's going
+    /// on.
+    ///
+    /// If set to `None`, then historically deprecated semver requirements will
+    /// all be rejected.
     pub fn parse(name: &str,
                  version: Option<&str>,
-                 source_id: &SourceId) -> CargoResult<DependencyInner> {
+                 source_id: &SourceId,
+                 deprecated_extra: Option<(&PackageId, &Config)>)
+                 -> CargoResult<DependencyInner> {
         let (specified_req, version_req) = match version {
-            Some(v) => (true, try!(VersionReq::parse(v))),
+            Some(v) => (true, try!(DependencyInner::parse_with_deprecated(v, deprecated_extra))),
             None => (false, VersionReq::any())
         };
 
@@ -103,6 +114,40 @@ impl DependencyInner {
         })
     }
 
+    fn parse_with_deprecated(req: &str,
+                             extra: Option<(&PackageId, &Config)>)
+                             -> CargoResult<VersionReq> {
+        match VersionReq::parse(req) {
+            Err(e) => {
+                let (inside, config) = match extra {
+                    Some(pair) => pair,
+                    None => return Err(e.into()),
+                };
+                match e {
+                    ReqParseError::DeprecatedVersionRequirement(requirement) => {
+                        let msg = format!("\
+parsed version requirement `{}` is no longer valid
+
+Previous versions of Cargo accepted this malformed requirement,
+but it is being deprecated. This was found when parsing the manifest
+of {} {}, and the correct version requirement is `{}`.
+
+This will soon become a hard error, so it's either recommended to
+update to a fixed version or contact the upstream maintainer about
+this warning.
+",
+	req, inside.name(), inside.version(), requirement);
+                        try!(config.shell().warn(&msg));
+
+                        Ok(requirement)
+                    }
+                    e => Err(From::from(e)),
+                }
+            },
+            Ok(v) => Ok(v),
+        }
+    }
+
     pub fn new_override(name: &str, source_id: &SourceId) -> DependencyInner {
         DependencyInner {
             name: name.to_string(),
@@ -216,8 +261,20 @@ impl Dependency {
     /// Attempt to create a `Dependency` from an entry in the manifest.
     pub fn parse(name: &str,
                  version: Option<&str>,
-                 source_id: &SourceId) -> CargoResult<Dependency> {
-        DependencyInner::parse(name, version, source_id).map(|di| {
+                 source_id: &SourceId,
+                 inside: &PackageId,
+                 config: &Config) -> CargoResult<Dependency> {
+        let arg = Some((inside, config));
+        DependencyInner::parse(name, version, source_id, arg).map(|di| {
+            di.into_dependency()
+        })
+    }
+
+    /// Attempt to create a `Dependency` from an entry in the manifest.
+    pub fn parse_no_deprecated(name: &str,
+                               version: Option<&str>,
+                               source_id: &SourceId) -> CargoResult<Dependency> {
+        DependencyInner::parse(name, version, source_id, None).map(|di| {
             di.into_dependency()
         })
     }

src/cargo/core/registry.rs

@@ -364,7 +364,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
 #[cfg(test)]
 pub mod test {
     use core::{Summary, Registry, Dependency};
-    use util::{CargoResult};
+    use util::CargoResult;
 
     pub struct RegistryBuilder {
         summaries: Vec<Summary>,

src/cargo/ops/cargo_install.rs

@@ -258,7 +258,7 @@ fn select_pkg<'a, T>(mut source: T,
     try!(source.update());
     match name {
         Some(name) => {
-            let dep = try!(Dependency::parse(name, vers, source_id));
+            let dep = try!(Dependency::parse_no_deprecated(name, vers, source_id));
             let deps = try!(source.query(&dep));
             match deps.iter().map(|p| p.package_id()).max() {
                 Some(pkgid) => {

src/cargo/sources/registry/index.rs

@@ -135,8 +135,7 @@ impl<'cfg> RegistryIndex<'cfg> {
             name, req, features, optional, default_features, target, kind
         } = dep;
 
-        let dep = try!(DependencyInner::parse(&name, Some(&req),
-                                              &self.source_id));
+        let dep = try!(DependencyInner::parse(&name, Some(&req), &self.source_id, None));
         let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") {
             "dev" => Kind::Development,
             "build" => Kind::Build,

src/cargo/util/toml.rs

@@ -336,6 +336,7 @@ impl TomlProject {
 }
 
 struct Context<'a, 'b> {
+    pkgid: Option<&'a PackageId>,
     deps: &'a mut Vec<Dependency>,
     source_id: &'a SourceId,
     nested_paths: &'a mut Vec<PathBuf>,
@@ -566,6 +567,7 @@ impl TomlManifest {
         {
 
             let mut cx = Context {
+                pkgid: Some(&pkgid),
                 deps: &mut deps,
                 source_id: source_id,
                 nested_paths: &mut nested_paths,
@@ -714,6 +716,7 @@ impl TomlManifest {
         let mut warnings = Vec::new();
         let mut deps = Vec::new();
         let replace = try!(self.replace(&mut Context {
+            pkgid: None,
             deps: &mut deps,
             source_id: source_id,
             nested_paths: &mut nested_paths,
@@ -883,7 +886,13 @@ impl TomlDependency {
         };
 
         let version = details.version.as_ref().map(|v| &v[..]);
-        let mut dep = try!(DependencyInner::parse(name, version, &new_source_id));
+        let mut dep = match cx.pkgid {
+            Some(id) => {
+                try!(DependencyInner::parse(name, version, &new_source_id,
+                                            Some((id, cx.config))))
+            }
+            None => try!(DependencyInner::parse(name, version, &new_source_id, None)),
+        };
         dep = dep.set_features(details.features.unwrap_or(Vec::new()))
                  .set_default_features(details.default_features.unwrap_or(true))
                  .set_optional(details.optional.unwrap_or(false))

tests/registry.rs

@@ -1242,3 +1242,101 @@ fn bump_version_dont_update_registry() {
 [FINISHED] [..]
 "));
 }
+
+#[test]
+fn old_version_req() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "bar"
+            version = "0.5.0"
+            authors = []
+
+            [dependencies]
+            remote = "0.2*"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    Package::new("remote", "0.2.0").publish();
+
+    assert_that(p.cargo("build"),
+                execs().with_status(0)
+                       .with_stderr("\
+warning: parsed version requirement `0.2*` is no longer valid
+
+Previous versions of Cargo accepted this malformed requirement,
+but it is being deprecated. This was found when parsing the manifest
+of bar 0.5.0, and the correct version requirement is `0.2.*`.
+
+This will soon become a hard error, so it's either recommended to
+update to a fixed version or contact the upstream maintainer about
+this warning.
+
+warning: parsed version requirement `0.2*` is no longer valid
+
+Previous versions of Cargo accepted this malformed requirement,
+but it is being deprecated. This was found when parsing the manifest
+of bar 0.5.0, and the correct version requirement is `0.2.*`.
+
+This will soon become a hard error, so it's either recommended to
+update to a fixed version or contact the upstream maintainer about
+this warning.
+
+[UPDATING] [..]
+[DOWNLOADING] [..]
+[COMPILING] [..]
+[COMPILING] [..]
+[FINISHED] [..]
+"));
+}
+
+#[test]
+fn old_version_req_upstream() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "bar"
+            version = "0.5.0"
+            authors = []
+
+            [dependencies]
+            remote = "0.3"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    Package::new("remote", "0.3.0")
+			.file("Cargo.toml", r#"
+				[project]
+                name = "remote"
+                version = "0.3.0"
+                authors = []
+
+                [dependencies]
+                bar = "0.2*"
+			"#)
+            .file("src/lib.rs", "")
+			.publish();
+    Package::new("bar", "0.2.0").publish();
+
+    assert_that(p.cargo("build"),
+                execs().with_status(0)
+                       .with_stderr("\
+[UPDATING] [..]
+[DOWNLOADING] [..]
+warning: parsed version requirement `0.2*` is no longer valid
+
+Previous versions of Cargo accepted this malformed requirement,
+but it is being deprecated. This was found when parsing the manifest
+of remote 0.3.0, and the correct version requirement is `0.2.*`.
+
+This will soon become a hard error, so it's either recommended to
+update to a fixed version or contact the upstream maintainer about
+this warning.
+
+[COMPILING] [..]
+[COMPILING] [..]
+[FINISHED] [..]
+"));
+}

tests/resolve.rs

@@ -33,7 +33,7 @@ impl ToDep for &'static str {
     fn to_dep(self) -> Dependency {
         let url = "http://example.com".to_url().unwrap();
         let source_id = SourceId::for_registry(&url);
-        Dependency::parse(self, Some("1.0.0"), &source_id).unwrap()
+        Dependency::parse_no_deprecated(self, Some("1.0.0"), &source_id).unwrap()
     }
 }
 
@@ -101,14 +101,14 @@ fn dep(name: &str) -> Dependency { dep_req(name, "1.0.0") }
 fn dep_req(name: &str, req: &str) -> Dependency {
     let url = "http://example.com".to_url().unwrap();
     let source_id = SourceId::for_registry(&url);
-    Dependency::parse(name, Some(req), &source_id).unwrap()
+    Dependency::parse_no_deprecated(name, Some(req), &source_id).unwrap()
 }
 
 fn dep_loc(name: &str, location: &str) -> Dependency {
     let url = location.to_url().unwrap();
     let master = GitReference::Branch("master".to_string());
     let source_id = SourceId::for_git(&url, master);
-    Dependency::parse(name, Some("1.0.0"), &source_id).unwrap()
+    Dependency::parse_no_deprecated(name, Some("1.0.0"), &source_id).unwrap()
 }
 fn dep_kind(name: &str, kind: Kind) -> Dependency {
     dep(name).clone_inner().set_kind(kind).into_dependency()