Tweak message in Dependency::parse

Provide some contextual information about why a dependency failed to parse.

Author: alexcrichton

src/cargo/core/dependency.rs

@@ -88,12 +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,
-                 config: &Config) -> CargoResult<DependencyInner> {
+                 deprecated_extra: Option<(&PackageId, &Config)>)
+                 -> CargoResult<DependencyInner> {
         let (specified_req, version_req) = match version {
-            Some(v) => (true, try!(DependencyInner::parse_with_deprecated(v, config))),
+            Some(v) => (true, try!(DependencyInner::parse_with_deprecated(v, deprecated_extra))),
             None => (false, VersionReq::any())
         };
 
@@ -105,16 +114,31 @@ impl DependencyInner {
         })
     }
 
-    fn parse_with_deprecated(req: &str, config: &Config) -> CargoResult<VersionReq> {
+    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!("One of your version requirements ({}) is invalid. \
-Previous versions of Cargo accepted this malformed requirement, but it is being deprecated. Please \
-use {} instead.", req, 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)),
@@ -238,8 +262,19 @@ impl Dependency {
     pub fn parse(name: &str,
                  version: Option<&str>,
                  source_id: &SourceId,
+                 inside: &PackageId,
                  config: &Config) -> CargoResult<Dependency> {
-        DependencyInner::parse(name, version, source_id, config).map(|di| {
+        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, Config};
+    use util::CargoResult;
 
     pub struct RegistryBuilder {
         summaries: Vec<Summary>,
@@ -405,13 +405,13 @@ pub mod test {
     }
 
     impl Registry for RegistryBuilder {
-        fn query(&mut self, dep: &Dependency, config: &Config) -> CargoResult<Vec<Summary>> {
+        fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
             debug!("querying; dep={:?}", dep);
 
             let overrides = self.query_overrides(dep);
 
             if overrides.is_empty() {
-                self.summaries.query(dep, config)
+                self.summaries.query(dep)
             } else {
                 Ok(overrides)
             }

src/cargo/ops/cargo_install.rs

@@ -57,7 +57,7 @@ pub fn install(root: Option<&str>,
     let map = try!(SourceConfigMap::new(config));
     let (pkg, source) = if source_id.is_git() {
         try!(select_pkg(GitSource::new(source_id, config), source_id,
-                        krate, vers, config, &mut |git| git.read_packages()))
+                        krate, vers, &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 +68,11 @@ pub fn install(root: Option<&str>,
                            specify an alternate source", path.display()))
         }));
         try!(select_pkg(PathSource::new(&path, source_id, config),
-                        source_id, krate, vers, config,
+                        source_id, krate, vers,
                         &mut |path| path.read_packages()))
     } else {
         try!(select_pkg(try!(map.load(source_id)),
-                        source_id, krate, vers, config,
+                        source_id, krate, vers,
                         &mut |_| Err(human("must specify a crate to install from \
                                             crates.io, or use --path or --git to \
                                             specify alternate source"))))
@@ -251,15 +251,14 @@ 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
 {
     try!(source.update());
     match name {
         Some(name) => {
-            let dep = try!(Dependency::parse(name, vers, source_id, config));
+            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,9 +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,
-                                              self.config));
+        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, cx.config));
+        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/resolve.rs

@@ -10,7 +10,7 @@ use hamcrest::{assert_that, equal_to, contains};
 use cargo::core::source::{SourceId, GitReference};
 use cargo::core::dependency::Kind::{self, Development};
 use cargo::core::{Dependency, PackageId, Summary, Registry};
-use cargo::util::{CargoResult, ToUrl, Config};
+use cargo::util::{CargoResult, ToUrl};
 use cargo::core::resolver::{self, Method};
 
 fn resolve<R: Registry>(pkg: PackageId, deps: Vec<Dependency>,
@@ -33,8 +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);
-        let config = Config::default().unwrap();
-        Dependency::parse(self, Some("1.0.0"), &source_id, &config).unwrap()
+        Dependency::parse_no_deprecated(self, Some("1.0.0"), &source_id).unwrap()
     }
 }
 
@@ -102,16 +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);
-    let config = Config::default().unwrap();
-    Dependency::parse(name, Some(req), &source_id, &config).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);
-    let config = Config::default().unwrap();
-    Dependency::parse(name, Some("1.0.0"), &source_id, &config).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()