Skip to content

Commit

Permalink
Add the path to the manifest in json output
Browse files Browse the repository at this point in the history
This allows consumers of the json messages to avoid guessing where
exactly the package root is. Having access to the package root is
difficult by virtue of requiring logic to guess its location by e.g.
walking filesystem from the source file.

This guessing logic becomes further complicated in presence of
workspaces and nigh impossible to implement correctly in instances where
artifacts end up produced from paths above the package root (e.g.
`../foo.rs`).

Since Cargo has access to this data in the first place, there doesn't
seem to be much reason to force consumers to invent their own, possibly
flawed, logic.
  • Loading branch information
nagisa committed Mar 4, 2021
1 parent 61a31bc commit 548300b
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 5 deletions.
43 changes: 38 additions & 5 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ fn compile<'cfg>(
let work = if unit.show_warnings(bcx.config) {
replay_output_cache(
unit.pkg.package_id(),
PathBuf::from(unit.pkg.manifest_path()),
&unit.target,
cx.files().message_cache_path(unit),
cx.bcx.build_config.message_format,
Expand Down Expand Up @@ -219,6 +220,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
// Prepare the native lib state (extra `-L` and `-l` flags).
let build_script_outputs = Arc::clone(&cx.build_script_outputs);
let current_id = unit.pkg.package_id();
let manifest_path = PathBuf::from(unit.pkg.manifest_path());
let build_scripts = cx.build_scripts.get(unit).cloned();

// If we are a binary and the package also contains a library, then we
Expand Down Expand Up @@ -316,7 +318,16 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
&target,
mode,
&mut |line| on_stdout_line(state, line, package_id, &target),
&mut |line| on_stderr_line(state, line, package_id, &target, &mut output_options),
&mut |line| {
on_stderr_line(
state,
line,
package_id,
&manifest_path,
&target,
&mut output_options,
)
},
)
.map_err(verbose_if_simple_exit_code)
.chain_err(|| format!("could not compile `{}`", name))?;
Expand Down Expand Up @@ -414,6 +425,7 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu
let outputs = cx.outputs(unit)?;
let export_dir = cx.files().export_dir();
let package_id = unit.pkg.package_id();
let manifest_path = PathBuf::from(unit.pkg.manifest_path());
let profile = unit.profile;
let unit_mode = unit.mode;
let features = unit.features.iter().map(|s| s.to_string()).collect();
Expand Down Expand Up @@ -467,6 +479,7 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu

let msg = machine_message::Artifact {
package_id,
manifest_path,
target: &target,
profile: art_profile,
features,
Expand Down Expand Up @@ -618,10 +631,10 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
let name = unit.pkg.name().to_string();
let build_script_outputs = Arc::clone(&cx.build_script_outputs);
let package_id = unit.pkg.package_id();
let manifest_path = PathBuf::from(unit.pkg.manifest_path());
let target = Target::clone(&unit.target);
let mut output_options = OutputOptions::new(cx, unit);
let script_metadata = cx.find_build_script_metadata(unit);

Ok(Work::new(move |state| {
if let Some(script_metadata) = script_metadata {
if let Some(output) = build_script_outputs.lock().unwrap().get(script_metadata) {
Expand All @@ -638,7 +651,16 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
rustdoc
.exec_with_streaming(
&mut |line| on_stdout_line(state, line, package_id, &target),
&mut |line| on_stderr_line(state, line, package_id, &target, &mut output_options),
&mut |line| {
on_stderr_line(
state,
line,
package_id,
&manifest_path,
&target,
&mut output_options,
)
},
false,
)
.chain_err(|| format!("could not document `{}`", name))?;
Expand Down Expand Up @@ -1139,10 +1161,11 @@ fn on_stderr_line(
state: &JobState<'_>,
line: &str,
package_id: PackageId,
manifest_path: &std::path::Path,
target: &Target,
options: &mut OutputOptions,
) -> CargoResult<()> {
if on_stderr_line_inner(state, line, package_id, target, options)? {
if on_stderr_line_inner(state, line, package_id, manifest_path, target, options)? {
// Check if caching is enabled.
if let Some((path, cell)) = &mut options.cache_cell {
// Cache the output, which will be replayed later when Fresh.
Expand All @@ -1160,6 +1183,7 @@ fn on_stderr_line_inner(
state: &JobState<'_>,
line: &str,
package_id: PackageId,
manifest_path: &std::path::Path,
target: &Target,
options: &mut OutputOptions,
) -> CargoResult<bool> {
Expand Down Expand Up @@ -1300,6 +1324,7 @@ fn on_stderr_line_inner(
// indicating which package it came from and then emit it.
let msg = machine_message::FromCompiler {
package_id,
manifest_path,
target,
message: compiler_message,
}
Expand All @@ -1314,6 +1339,7 @@ fn on_stderr_line_inner(

fn replay_output_cache(
package_id: PackageId,
manifest_path: PathBuf,
target: &Target,
path: PathBuf,
format: MessageFormat,
Expand Down Expand Up @@ -1343,7 +1369,14 @@ fn replay_output_cache(
break;
}
let trimmed = line.trim_end_matches(&['\n', '\r'][..]);
on_stderr_line(state, trimmed, package_id, &target, &mut options)?;
on_stderr_line(
state,
trimmed,
package_id,
&manifest_path,
&target,
&mut options,
)?;
line.clear();
}
Ok(())
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/util/machine_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub trait Message: ser::Serialize {
#[derive(Serialize)]
pub struct FromCompiler<'a> {
pub package_id: PackageId,
pub manifest_path: &'a Path,
pub target: &'a Target,
pub message: Box<RawValue>,
}
Expand All @@ -33,6 +34,7 @@ impl<'a> Message for FromCompiler<'a> {
#[derive(Serialize)]
pub struct Artifact<'a> {
pub package_id: PackageId,
pub manifest_path: PathBuf,
pub target: &'a Target,
pub profile: ArtifactProfile,
pub features: Vec<String>,
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1736,6 +1736,7 @@ fn json_artifact_includes_executable_for_benchmark() {
"filenames": "{...}",
"fresh": false,
"package_id": "foo 0.0.1 ([..])",
"manifest_path": "[..]",
"profile": "{...}",
"reason": "compiler-artifact",
"target": {
Expand Down
7 changes: 7 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3140,6 +3140,7 @@ fn compiler_json_error_format() {
{
"reason":"compiler-artifact",
"package_id":"foo 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["custom-build"],
"crate_types":["bin"],
Expand All @@ -3166,6 +3167,7 @@ fn compiler_json_error_format() {
{
"reason":"compiler-message",
"package_id":"bar 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["lib"],
"crate_types":["lib"],
Expand All @@ -3191,6 +3193,7 @@ fn compiler_json_error_format() {
"executable": null,
"features": [],
"package_id":"bar 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["lib"],
"crate_types":["lib"],
Expand Down Expand Up @@ -3221,6 +3224,7 @@ fn compiler_json_error_format() {
{
"reason":"compiler-message",
"package_id":"foo 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["bin"],
"crate_types":["bin"],
Expand All @@ -3237,6 +3241,7 @@ fn compiler_json_error_format() {
{
"reason":"compiler-artifact",
"package_id":"foo 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["bin"],
"crate_types":["bin"],
Expand Down Expand Up @@ -3307,6 +3312,7 @@ fn message_format_json_forward_stderr() {
{
"reason":"compiler-message",
"package_id":"foo 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["bin"],
"crate_types":["bin"],
Expand All @@ -3323,6 +3329,7 @@ fn message_format_json_forward_stderr() {
{
"reason":"compiler-artifact",
"package_id":"foo 0.5.0 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["bin"],
"crate_types":["bin"],
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ fn doc_message_format() {
"spans": "{...}"
},
"package_id": "foo [..]",
"manifest_path": "[..]",
"reason": "compiler-message",
"target": "{...}"
}
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/metabuild.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ fn metabuild_json_artifact() {
"filenames": "{...}",
"fresh": false,
"package_id": "foo [..]",
"manifest_path": "[..]",
"profile": "{...}",
"reason": "compiler-artifact",
"target": {
Expand Down Expand Up @@ -743,6 +744,7 @@ fn metabuild_failed_build_json() {
"spans": "{...}"
},
"package_id": "foo [..]",
"manifest_path": "[..]",
"reason": "compiler-message",
"target": {
"crate_types": [
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3536,6 +3536,7 @@ fn json_artifact_includes_test_flag() {
"executable": "[..]/foo-[..]",
"features": [],
"package_id":"foo 0.0.1 ([..])",
"manifest_path": "[..]",
"target":{
"kind":["lib"],
"crate_types":["lib"],
Expand Down Expand Up @@ -3572,6 +3573,7 @@ fn json_artifact_includes_executable_for_library_tests() {
"filenames": "{...}",
"fresh": false,
"package_id": "foo 0.0.1 ([..])",
"manifest_path": "[..]",
"profile": "{...}",
"reason": "compiler-artifact",
"target": {
Expand Down Expand Up @@ -3610,6 +3612,7 @@ fn json_artifact_includes_executable_for_integration_tests() {
"filenames": "{...}",
"fresh": false,
"package_id": "foo 0.0.1 ([..])",
"manifest_path": "[..]",
"profile": "{...}",
"reason": "compiler-artifact",
"target": {
Expand Down

0 comments on commit 548300b

Please sign in to comment.