From 3e0603395c2858d8415bd8b0619f090ee72d964f Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sun, 17 Dec 2017 22:58:51 +0100 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20swallow=20virtual=20manifest=20?= =?UTF-8?q?parsing=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 40 +++++++++++++++++--------------------- tests/build.rs | 2 +- tests/metadata.rs | 2 +- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index c5d0ddf0178..58445a0c2c5 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -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) { diff --git a/tests/build.rs b/tests/build.rs index 51f837aa0f2..101adc5fceb 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -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] ")) } diff --git a/tests/metadata.rs b/tests/metadata.rs index 4805b67415f..214975c065e 100644 --- a/tests/metadata.rs +++ b/tests/metadata.rs @@ -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=