Auto merge of #4340 - debris:fixed_4310, r=alexcrichton

Fixed #4310, print useful message when dependency contains a malformed Cargo.toml

fixed #4310

Author: bors

src/cargo/ops/cargo_read_manifest.rs

@@ -5,7 +5,7 @@ use std::path::{Path, PathBuf};
 
 use core::{Package, SourceId, PackageId, EitherManifest};
 use util::{self, Config};
-use util::errors::{CargoResult, CargoResultExt};
+use util::errors::{CargoResult, CargoResultExt, CargoError};
 use util::important_paths::find_project_manifest_exact;
 use util::toml::read_manifest;
 
@@ -28,6 +28,7 @@ pub fn read_packages(path: &Path, source_id: &SourceId, config: &Config)
                      -> CargoResult<Vec<Package>> {
     let mut all_packages = HashMap::new();
     let mut visited = HashSet::<PathBuf>::new();
+    let mut errors = Vec::<CargoError>::new();
 
     trace!("looking for root package: {}, source_id={}", path.display(), source_id);
 
@@ -55,13 +56,16 @@ pub fn read_packages(path: &Path, source_id: &SourceId, config: &Config)
 
         if has_manifest(dir) {
             read_nested_packages(dir, &mut all_packages, source_id, config,
-                                      &mut visited)?;
+                                      &mut visited, &mut errors)?;
         }
         Ok(true)
     })?;
 
     if all_packages.is_empty() {
-        Err(format!("Could not find Cargo.toml in `{}`", path.display()).into())
+        match errors.pop() {
+            Some(err) => Err(err),
+            None => Err(format!("Could not find Cargo.toml in `{}`", path.display()).into()),
+        }
     } else {
         Ok(all_packages.into_iter().map(|(_, v)| v).collect())
     }
@@ -104,13 +108,14 @@ fn read_nested_packages(path: &Path,
                         all_packages: &mut HashMap<PackageId, Package>,
                         source_id: &SourceId,
                         config: &Config,
-                        visited: &mut HashSet<PathBuf>) -> CargoResult<()> {
+                        visited: &mut HashSet<PathBuf>,
+                        errors: &mut Vec<CargoError>) -> CargoResult<()> {
     if !visited.insert(path.to_path_buf()) { return Ok(()) }
 
     let manifest_path = find_project_manifest_exact(path, "Cargo.toml")?;
 
     let (manifest, nested) = match read_manifest(&manifest_path, source_id, config) {
-        Err(_) => {
+        Err(err) => {
             // Ignore malformed manifests found on git repositories
             //
             // git source try to find and read all manifests from the repository
@@ -120,6 +125,7 @@ fn read_nested_packages(path: &Path,
             // TODO: Add a way to exclude folders?
             info!("skipping malformed package found at `{}`",
                   path.to_string_lossy());
+            errors.push(err);
             return Ok(());
         }
         Ok(tuple) => tuple
@@ -151,7 +157,7 @@ fn read_nested_packages(path: &Path,
         for p in nested.iter() {
             let path = util::normalize_path(&path.join(p));
             read_nested_packages(&path, all_packages, source_id,
-                                      config, visited)?;
+                                      config, visited, errors)?;
         }
     }
 

tests/git.rs

@@ -2083,3 +2083,65 @@ fn include_overrides_gitignore() {
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
 "));
 }
+
+#[test]
+fn invalid_git_dependency_manifest() {
+    let project = project("foo");
+    let git_project = git::new("dep1", |project| {
+        project
+            .file("Cargo.toml", r#"
+                [project]
+
+                name = "dep1"
+                version = "0.5.0"
+                authors = ["[email protected]"]
+                categories = ["algorithms"]
+                categories = ["algorithms"]
+
+                [lib]
+
+                name = "dep1"
+            "#)
+            .file("src/dep1.rs", r#"
+                pub fn hello() -> &'static str {
+                    "hello world"
+                }
+            "#)
+    }).unwrap();
+
+    let project = project
+        .file("Cargo.toml", &format!(r#"
+            [project]
+
+            name = "foo"
+            version = "0.5.0"
+            authors = ["[email protected]"]
+
+            [dependencies.dep1]
+
+            git = '{}'
+        "#, git_project.url()))
+        .file("src/main.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"]));
+
+    let git_root = git_project.root();
+
+    assert_that(project.cargo_process("build"),
+        execs()
+        .with_stderr(&format!("[UPDATING] git repository `{}`\n\
+                              error: failed to load source for a dependency on `dep1`\n\
+                              \n\
+                              Caused by:\n  \
+                              Unable to update {}\n\
+                              \n\
+                              Caused by:\n  \
+                              failed to parse manifest at `[..]`\n\
+                              \n\
+                              Caused by:\n  \
+                              could not parse input as TOML\n\
+                              \n\
+                              Caused by:\n  \
+                              duplicate key: `categories` for key `project`",
+                             path2url(git_root.clone()),
+                             path2url(git_root),
+                             )));
+}