Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix metabuild compile errors with --message-format=json. #6432

Merged
merged 3 commits into from
Dec 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,10 +689,9 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
// second is the cwd that rustc should operate in.
fn path_args(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>) -> (PathBuf, PathBuf) {
let ws_root = bcx.ws.root();
let src = if unit.target.is_custom_build() && unit.pkg.manifest().metabuild().is_some() {
unit.pkg.manifest().metabuild_path(bcx.ws.target_dir())
} else {
unit.target.src_path().path().to_path_buf()
let src = match unit.target.src_path() {
TargetSourcePath::Path(path) => path.to_path_buf(),
TargetSourcePath::Metabuild => unit.pkg.manifest().metabuild_path(bcx.ws.target_dir()),
};
assert!(src.is_absolute());
if unit.pkg.package_id().source_id().is_path() {
Expand Down
18 changes: 12 additions & 6 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,10 @@ pub enum TargetSourcePath {
}

impl TargetSourcePath {
pub fn path(&self) -> &Path {
pub fn path(&self) -> Option<&Path> {
match self {
TargetSourcePath::Path(path) => path.as_ref(),
TargetSourcePath::Metabuild => panic!("metabuild not expected"),
TargetSourcePath::Path(path) => Some(path.as_ref()),
TargetSourcePath::Metabuild => None,
}
}

Expand Down Expand Up @@ -268,19 +268,25 @@ struct SerializedTarget<'a> {
/// See https://doc.rust-lang.org/reference/linkage.html
crate_types: Vec<&'a str>,
name: &'a str,
src_path: &'a PathBuf,
src_path: Option<&'a PathBuf>,
edition: &'a str,
#[serde(rename = "required-features", skip_serializing_if = "Option::is_none")]
required_features: Option<Vec<&'a str>>,
}

impl ser::Serialize for Target {
fn serialize<S: ser::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
let src_path = match &self.src_path {
TargetSourcePath::Path(p) => Some(p),
// Unfortunately getting the correct path would require access to
// target_dir, which is not available here.
TargetSourcePath::Metabuild => None,
};
SerializedTarget {
kind: &self.kind,
crate_types: self.rustc_crate_types(),
name: &self.name,
src_path: &self.src_path.path().to_path_buf(),
src_path: src_path,
edition: &self.edition.to_string(),
required_features: self
.required_features
Expand All @@ -301,7 +307,7 @@ compact_debug! {
Target::lib_target(
&self.name,
kinds.clone(),
self.src_path().path().to_path_buf(),
self.src_path().path().unwrap().to_path_buf(),
self.edition,
),
format!("lib_target({:?}, {:?}, {:?}, {:?})",
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ fn run_doc_tests(
config.shell().status("Doc-tests", target.name())?;
let mut p = compilation.rustdoc_process(package, target)?;
p.arg("--test")
.arg(target.src_path().path())
.arg(target.src_path().path().unwrap())
.arg("--crate-name")
.arg(&target.crate_name());

Expand Down
73 changes: 73 additions & 0 deletions tests/testsuite/metabuild.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,5 +699,78 @@ fn metabuild_json_artifact() {
let p = basic_project();
p.cargo("build --message-format=json")
.masquerade_as_nightly_cargo()
.with_json_contains_unordered(
r#"
{
"executable": null,
"features": [],
"filenames": [
"[..]/foo/target/debug/build/foo-[..]/metabuild-foo[EXE]"
],
"fresh": false,
"package_id": "foo [..]",
"profile": "{...}",
"reason": "compiler-artifact",
"target": {
"crate_types": [
"bin"
],
"edition": "2018",
"kind": [
"custom-build"
],
"name": "metabuild-foo",
"src_path": "[..]/foo/target/.metabuild/metabuild-foo-[..].rs"
}
}

{
"cfgs": [],
"env": [],
"linked_libs": [],
"linked_paths": [],
"package_id": "foo [..]",
"reason": "build-script-executed"
}
"#,
)
.run();
}

#[test]
fn metabuild_failed_build_json() {
let p = basic_project();
// Modify the metabuild dep so that it fails to compile.
p.change_file("mb/src/lib.rs", "");
p.cargo("build --message-format=json")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_json_contains_unordered(
r#"
{
"message": {
"children": "{...}",
"code": "{...}",
"level": "error",
"message": "cannot find function `metabuild` in module `mb`",
"rendered": "[..]",
"spans": "{...}"
},
"package_id": "foo [..]",
"reason": "compiler-message",
"target": {
"crate_types": [
"bin"
],
"edition": "2018",
"kind": [
"custom-build"
],
"name": "metabuild-foo",
"src_path": null
}
}
"#,
)
.run();
}
47 changes: 47 additions & 0 deletions tests/testsuite/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ pub struct Execs {
expect_stderr_unordered: Vec<String>,
expect_neither_contains: Vec<String>,
expect_json: Option<Vec<Value>>,
expect_json_contains_unordered: Vec<Value>,
stream_output: bool,
}

Expand Down Expand Up @@ -689,6 +690,24 @@ impl Execs {
self
}

/// Verify JSON output contains the given objects (in any order) somewhere
/// in its output.
///
/// CAUTION: Be very careful when using this. Make sure every object is
/// unique (not a subset of one another). Also avoid using objects that
/// could possibly match multiple output lines unless you're very sure of
/// what you are doing.
///
/// See `with_json` for more detail.
pub fn with_json_contains_unordered(&mut self, expected: &str) -> &mut Self {
self.expect_json_contains_unordered.extend(
expected
.split("\n\n")
.map(|line| line.parse().expect("line to be a valid JSON value")),
);
self
}

/// Forward subordinate process stdout/stderr to the terminal.
/// Useful for printf debugging of the tests.
/// CAUTION: CI will fail if you leave this in your test!
Expand Down Expand Up @@ -941,6 +960,33 @@ impl Execs {
self.match_json(obj, line)?;
}
}

if !self.expect_json_contains_unordered.is_empty() {
let stdout = str::from_utf8(&actual.stdout)
.map_err(|_| "stdout was not utf8 encoded".to_owned())?;
let mut lines = stdout
.lines()
.filter(|line| line.starts_with('{'))
.collect::<Vec<_>>();
for obj in &self.expect_json_contains_unordered {
match lines
.iter()
.position(|line| self.match_json(obj, line).is_ok())
{
Some(index) => lines.remove(index),
None => {
return Err(format!(
"Did not find expected JSON:\n\
{}\n\
Remaining available output:\n\
{}\n",
serde_json::to_string_pretty(obj).unwrap(),
lines.join("\n")
));
}
};
}
}
Ok(())
}

Expand Down Expand Up @@ -1322,6 +1368,7 @@ pub fn execs() -> Execs {
expect_stderr_unordered: Vec::new(),
expect_neither_contains: Vec::new(),
expect_json: None,
expect_json_contains_unordered: Vec::new(),
stream_output: false,
}
}
Expand Down