Skip to content

Commit 510cf5e

Browse files
committed
Auto merge of #4828 - SimonSapin:virtual-err, r=alexcrichton
Don’t swallow virtual manifest parsing errors 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.
2 parents 4f6b265 + 3e06033 commit 510cf5e

File tree

3 files changed

+20
-24
lines changed

3 files changed

+20
-24
lines changed

src/cargo/util/toml/mod.rs

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,30 +56,26 @@ fn do_read_manifest(contents: &str,
5656
})?;
5757

5858
let manifest = Rc::new(manifest);
59-
return match TomlManifest::to_real_manifest(&manifest,
60-
source_id,
61-
package_root,
62-
config) {
63-
Ok((mut manifest, paths)) => {
64-
for key in unused {
65-
manifest.add_warning(format!("unused manifest key: {}", key));
66-
}
67-
if !manifest.targets().iter().any(|t| !t.is_custom_build()) {
68-
bail!("no targets specified in the manifest\n \
69-
either src/lib.rs, src/main.rs, a [lib] section, or \
70-
[[bin]] section must be present")
71-
}
72-
Ok((EitherManifest::Real(manifest), paths))
59+
return if manifest.project.is_some() || manifest.package.is_some() {
60+
let (mut manifest, paths) = TomlManifest::to_real_manifest(&manifest,
61+
source_id,
62+
package_root,
63+
config)?;
64+
for key in unused {
65+
manifest.add_warning(format!("unused manifest key: {}", key));
7366
}
74-
Err(e) => {
75-
match TomlManifest::to_virtual_manifest(&manifest,
76-
source_id,
77-
package_root,
78-
config) {
79-
Ok((m, paths)) => Ok((EitherManifest::Virtual(m), paths)),
80-
Err(..) => Err(e),
81-
}
67+
if !manifest.targets().iter().any(|t| !t.is_custom_build()) {
68+
bail!("no targets specified in the manifest\n \
69+
either src/lib.rs, src/main.rs, a [lib] section, or \
70+
[[bin]] section must be present")
8271
}
72+
Ok((EitherManifest::Real(manifest), paths))
73+
} else {
74+
let (m, paths) = TomlManifest::to_virtual_manifest(&manifest,
75+
source_id,
76+
package_root,
77+
config)?;
78+
Ok((EitherManifest::Virtual(m), paths))
8379
};
8480

8581
fn stringify(dst: &mut String, path: &serde_ignored::Path) {

tests/build.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ fn cargo_compile_with_invalid_manifest() {
9595
[ERROR] failed to parse manifest at `[..]`
9696
9797
Caused by:
98-
no `package` section found.
98+
virtual manifests must be configured with [workspace]
9999
"))
100100
}
101101

tests/metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ fn cargo_metadata_with_invalid_manifest() {
555555
[ERROR] failed to parse manifest at `[..]`
556556
557557
Caused by:
558-
no `package` section found."))
558+
virtual manifests must be configured with [workspace]"))
559559
}
560560

561561
const MANIFEST_OUTPUT: &'static str=

0 commit comments

Comments
 (0)