From f9fd4ff4ee88baf9dc1d5781075d5957ef9a93e2 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 17 Jan 2024 09:44:41 -0500 Subject: [PATCH] fix(json-msg): use pkgid spec in in JSON messages In 12914 we stabilized pkgid spec as unique package identifier for `cargo metadata`. However, we forgot to make the same change to JSON message format[^1]. This PR does so. Note that the `package_id` field in JSON message is not clearly stated as "opaque", so it might be considered as a breaking change to some extent. [^1]: https://doc.rust-lang.org/nightly/cargo/reference/external-tools.html#compiler-messages --- src/cargo/core/compiler/custom_build.rs | 2 +- src/cargo/core/compiler/mod.rs | 4 ++-- src/cargo/core/compiler/timings.rs | 2 +- src/cargo/util/machine_message.rs | 12 +++++++----- tests/testsuite/bench.rs | 2 +- tests/testsuite/binary_name.rs | 2 +- tests/testsuite/build.rs | 16 ++++++++-------- tests/testsuite/doc.rs | 8 ++++---- tests/testsuite/install.rs | 4 ++-- tests/testsuite/metabuild.rs | 6 +++--- tests/testsuite/test.rs | 6 +++--- 11 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 0d69d699c..99311a5ff 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -229,7 +229,7 @@ fn emit_build_output( .collect::>(); let msg = machine_message::BuildScript { - package_id, + package_id: package_id.to_spec(), linked_libs: &output.library_links, linked_paths: &library_paths, cfgs: &output.cfgs, diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index eefd08e09..0ca065043 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -578,7 +578,7 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu }; let msg = machine_message::Artifact { - package_id, + package_id: package_id.to_spec(), manifest_path, target: &target, profile: art_profile, @@ -1766,7 +1766,7 @@ fn on_stderr_line_inner( } let msg = machine_message::FromCompiler { - package_id, + package_id: package_id.to_spec(), manifest_path, target, message: compiler_message, diff --git a/src/cargo/core/compiler/timings.rs b/src/cargo/core/compiler/timings.rs index 98c36cfdc..edd89a463 100644 --- a/src/cargo/core/compiler/timings.rs +++ b/src/cargo/core/compiler/timings.rs @@ -222,7 +222,7 @@ impl<'cfg> Timings<'cfg> { .extend(unlocked.iter().cloned().cloned()); if self.report_json { let msg = machine_message::TimingInfo { - package_id: unit_time.unit.pkg.package_id(), + package_id: unit_time.unit.pkg.package_id().to_spec(), target: &unit_time.unit.target, mode: unit_time.unit.mode, duration: unit_time.duration, diff --git a/src/cargo/util/machine_message.rs b/src/cargo/util/machine_message.rs index f1602ae57..f62461847 100644 --- a/src/cargo/util/machine_message.rs +++ b/src/cargo/util/machine_message.rs @@ -1,10 +1,12 @@ use std::path::{Path, PathBuf}; +use cargo_util_schemas::core::PackageIdSpec; use serde::ser; use serde::Serialize; use serde_json::{self, json, value::RawValue}; -use crate::core::{compiler::CompileMode, PackageId, Target}; +use crate::core::compiler::CompileMode; +use crate::core::Target; pub trait Message: ser::Serialize { fn reason(&self) -> &str; @@ -19,7 +21,7 @@ pub trait Message: ser::Serialize { #[derive(Serialize)] pub struct FromCompiler<'a> { - pub package_id: PackageId, + pub package_id: PackageIdSpec, pub manifest_path: &'a Path, pub target: &'a Target, pub message: Box, @@ -33,7 +35,7 @@ impl<'a> Message for FromCompiler<'a> { #[derive(Serialize)] pub struct Artifact<'a> { - pub package_id: PackageId, + pub package_id: PackageIdSpec, pub manifest_path: PathBuf, pub target: &'a Target, pub profile: ArtifactProfile, @@ -71,7 +73,7 @@ pub enum ArtifactDebuginfo { #[derive(Serialize)] pub struct BuildScript<'a> { - pub package_id: PackageId, + pub package_id: PackageIdSpec, pub linked_libs: &'a [String], pub linked_paths: &'a [String], pub cfgs: &'a [String], @@ -87,7 +89,7 @@ impl<'a> Message for BuildScript<'a> { #[derive(Serialize)] pub struct TimingInfo<'a> { - pub package_id: PackageId, + pub package_id: PackageIdSpec, pub target: &'a Target, pub mode: CompileMode, pub duration: f64, diff --git a/tests/testsuite/bench.rs b/tests/testsuite/bench.rs index 01017e857..52e80bdc7 100644 --- a/tests/testsuite/bench.rs +++ b/tests/testsuite/bench.rs @@ -1649,7 +1649,7 @@ fn json_artifact_includes_executable_for_benchmark() { "features": [], "filenames": "{...}", "fresh": false, - "package_id": "foo 0.0.1 ([..])", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "profile": "{...}", "reason": "compiler-artifact", diff --git a/tests/testsuite/binary_name.rs b/tests/testsuite/binary_name.rs index 7735d6054..314ff7fdc 100644 --- a/tests/testsuite/binary_name.rs +++ b/tests/testsuite/binary_name.rs @@ -280,7 +280,7 @@ fn check_msg_format_json() { let output = r#" { "reason": "compiler-artifact", - "package_id": "foo 0.0.1 [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[CWD]/Cargo.toml", "target": "{...}", "profile": "{...}", diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 37e259ae1..c7f692499 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -4160,7 +4160,7 @@ fn compiler_json_error_format() { r#" { "reason":"compiler-artifact", - "package_id":"foo 0.5.0 ([..])", + "package_id":"path+file:///[..]/foo#0.5.0", "manifest_path": "[..]", "target":{ "kind":["custom-build"], @@ -4187,7 +4187,7 @@ fn compiler_json_error_format() { { "reason":"compiler-message", - "package_id":"bar 0.5.0 ([..])", + "package_id":"path+file:///[..]/bar#0.5.0", "manifest_path": "[..]", "target":{ "kind":["lib"], @@ -4213,7 +4213,7 @@ fn compiler_json_error_format() { }, "executable": null, "features": [], - "package_id":"bar 0.5.0 ([..])", + "package_id":"path+file:///[..]/bar#0.5.0", "manifest_path": "[..]", "target":{ "kind":["lib"], @@ -4234,7 +4234,7 @@ fn compiler_json_error_format() { { "reason":"build-script-executed", - "package_id":"foo 0.5.0 ([..])", + "package_id":"path+file:///[..]/foo#0.5.0", "linked_libs":[], "linked_paths":[], "env":[], @@ -4244,7 +4244,7 @@ fn compiler_json_error_format() { { "reason":"compiler-message", - "package_id":"foo 0.5.0 ([..])", + "package_id":"path+file:///[..]/foo#0.5.0", "manifest_path": "[..]", "target":{ "kind":["bin"], @@ -4261,7 +4261,7 @@ fn compiler_json_error_format() { { "reason":"compiler-artifact", - "package_id":"foo 0.5.0 ([..])", + "package_id":"path+file:///[..]/foo#0.5.0", "manifest_path": "[..]", "target":{ "kind":["bin"], @@ -4332,7 +4332,7 @@ fn message_format_json_forward_stderr() { r#" { "reason":"compiler-message", - "package_id":"foo 0.5.0 ([..])", + "package_id":"path+file:///[..]/foo#0.5.0", "manifest_path": "[..]", "target":{ "kind":["bin"], @@ -4349,7 +4349,7 @@ fn message_format_json_forward_stderr() { { "reason":"compiler-artifact", - "package_id":"foo 0.5.0 ([..])", + "package_id":"path+file:///[..]/foo#0.5.0", "manifest_path": "[..]", "target":{ "kind":["bin"], diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index c80f23d91..0358c3790 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1706,7 +1706,7 @@ fn doc_message_format() { "rendered": "{...}", "spans": "{...}" }, - "package_id": "foo [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "reason": "compiler-message", "target": "{...}" @@ -1729,7 +1729,7 @@ fn doc_json_artifacts() { r#" { "reason": "compiler-artifact", - "package_id": "foo 0.0.1 [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[ROOT]/foo/Cargo.toml", "target": { @@ -1751,7 +1751,7 @@ fn doc_json_artifacts() { { "reason": "compiler-artifact", - "package_id": "foo 0.0.1 [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[ROOT]/foo/Cargo.toml", "target": { @@ -1773,7 +1773,7 @@ fn doc_json_artifacts() { { "reason": "compiler-artifact", - "package_id": "foo 0.0.1 [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[ROOT]/foo/Cargo.toml", "target": { diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index c0dd7ef6c..a8aed02d6 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -128,7 +128,7 @@ fn simple_with_message_format() { r#" { "reason": "compiler-artifact", - "package_id": "foo 0.0.1 ([..])", + "package_id": "registry+https://[..]#foo@0.0.1", "manifest_path": "[..]", "target": { "kind": [ @@ -153,7 +153,7 @@ fn simple_with_message_format() { { "reason": "compiler-artifact", - "package_id": "foo 0.0.1 ([..])", + "package_id": "registry+https://[..]#foo@0.0.1", "manifest_path": "[..]", "target": { "kind": [ diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index 1c0196c98..ed603510f 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -694,7 +694,7 @@ fn metabuild_json_artifact() { "features": [], "filenames": "{...}", "fresh": false, - "package_id": "foo [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "profile": "{...}", "reason": "compiler-artifact", @@ -719,7 +719,7 @@ fn metabuild_json_artifact() { "env": [], "linked_libs": [], "linked_paths": [], - "package_id": "foo [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "out_dir": "[..]", "reason": "build-script-executed" } @@ -748,7 +748,7 @@ fn metabuild_failed_build_json() { "rendered": "{...}", "spans": "{...}" }, - "package_id": "foo [..]", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "reason": "compiler-message", "target": { diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 8a1d42326..6128a6e50 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -3925,7 +3925,7 @@ fn json_artifact_includes_test_flag() { }, "executable": "[..]/foo-[..]", "features": [], - "package_id":"foo 0.0.1 ([..])", + "package_id":"path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "target":{ "kind":["lib"], @@ -3962,7 +3962,7 @@ fn json_artifact_includes_executable_for_library_tests() { "features": [], "filenames": "{...}", "fresh": false, - "package_id": "foo 0.0.1 ([..])", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "profile": "{...}", "reason": "compiler-artifact", @@ -4001,7 +4001,7 @@ fn json_artifact_includes_executable_for_integration_tests() { "features": [], "filenames": "{...}", "fresh": false, - "package_id": "foo 0.0.1 ([..])", + "package_id": "path+file:///[..]/foo#0.0.1", "manifest_path": "[..]", "profile": "{...}", "reason": "compiler-artifact",