Don’t swallow virtual manifest parsing errors
authorSimon Sapin <simon.sapin@exyr.org>
Sun, 17 Dec 2017 21:58:51 +0000 (22:58 +0100)
committerSimon Sapin <simon.sapin@exyr.org>
Sun, 17 Dec 2017 22:09:05 +0000 (23:09 +0100)
Before this change, `cargo::util::toml::do_read_manifest` ended like this:

```rust
    return match TomlManifest::to_real_manifest(/* … */) {
        Ok(/* … */) => /* … */,
        Err(e) => match TomlManifest::to_virtual_manifest(/* … */) {
            Ok(/* … */) => /* … */,
            Err(..) => Err(e),
        }
    };
```

Errors returned by `to_virtual_manifest` were always ignored.
As a result, when something was wrong in a virtual manifest,
Cargo would unhelpfully exit with no more output than:

```
error: failed to parse manifest at `/tmp/a/Cargo.toml`

Caused by:
  no `package` section found.
```

http://doc.crates.io/manifest.html#virtual-manifest defines
a virtual manifest as “the `package` table is not present”,
so let’s first determine if a manifest is virtual based
on that criteria, and then only call one of the two methods.

Although it is not mentioned in the documentation,
`[project]` seems to be in the code an alias for `[package]`.
So let’s preserve that here too.

src/cargo/util/toml/mod.rs
tests/build.rs
tests/metadata.rs

index c5d0ddf017805b21c480ebcdff68a96fb632fd8a..58445a0c2c50bd9fef39538d16c09b54685ea440 100644 (file)
@@ -56,30 +56,26 @@ fn do_read_manifest(contents: &str,
     })?;
 
     let manifest = Rc::new(manifest);
-    return match TomlManifest::to_real_manifest(&manifest,
-                                                source_id,
-                                                package_root,
-                                                config) {
-        Ok((mut manifest, paths)) => {
-            for key in unused {
-                manifest.add_warning(format!("unused manifest key: {}", key));
-            }
-            if !manifest.targets().iter().any(|t| !t.is_custom_build()) {
-                bail!("no targets specified in the manifest\n  \
-                       either src/lib.rs, src/main.rs, a [lib] section, or \
-                       [[bin]] section must be present")
-            }
-            Ok((EitherManifest::Real(manifest), paths))
+    return if manifest.project.is_some() || manifest.package.is_some() {
+        let (mut manifest, paths) = TomlManifest::to_real_manifest(&manifest,
+                                                                   source_id,
+                                                                   package_root,
+                                                                   config)?;
+        for key in unused {
+            manifest.add_warning(format!("unused manifest key: {}", key));
         }
-        Err(e) => {
-            match TomlManifest::to_virtual_manifest(&manifest,
-                                                    source_id,
-                                                    package_root,
-                                                    config) {
-                Ok((m, paths)) => Ok((EitherManifest::Virtual(m), paths)),
-                Err(..) => Err(e),
-            }
+        if !manifest.targets().iter().any(|t| !t.is_custom_build()) {
+            bail!("no targets specified in the manifest\n  \
+                   either src/lib.rs, src/main.rs, a [lib] section, or \
+                   [[bin]] section must be present")
         }
+        Ok((EitherManifest::Real(manifest), paths))
+    } else {
+        let (m, paths) = TomlManifest::to_virtual_manifest(&manifest,
+                                                           source_id,
+                                                           package_root,
+                                                           config)?;
+        Ok((EitherManifest::Virtual(m), paths))
     };
 
     fn stringify(dst: &mut String, path: &serde_ignored::Path) {
index 51f837aa0f2cac924a713aa0a6788b3e18aaefaf..101adc5fcebd98146c8821badaafd06c534bdcec 100644 (file)
@@ -95,7 +95,7 @@ fn cargo_compile_with_invalid_manifest() {
 [ERROR] failed to parse manifest at `[..]`
 
 Caused by:
-  no `package` section found.
+  virtual manifests must be configured with [workspace]
 "))
 }
 
index 4805b67415fe03e1fcfe173586ab0415fe0ad059..214975c065e520fcee5e9e1b654dbb237f4565f1 100644 (file)
@@ -555,7 +555,7 @@ fn cargo_metadata_with_invalid_manifest() {
 [ERROR] failed to parse manifest at `[..]`
 
 Caused by:
-  no `package` section found."))
+  virtual manifests must be configured with [workspace]"))
 }
 
 const MANIFEST_OUTPUT: &'static str=