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(json-msg): use pkgid spec in in JSON messages #13311

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

In #12914 we stabilized pkgid spec as unique package identifier for
cargo metadata. However, we forgot to make the same change to
JSON message format1. 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.

How should we test and review this PR?

If we aren't sure about stabilizing this, we may need to revert #12914 as well.

Additional information

Zulip "miri" stream: https://rust-lang.zulipchat.com/#narrow/stream/269128-miri/topic/Cron.20Job.20Failure.20.28miri.2C.202024-01.29

Footnotes

  1. https://doc.rust-lang.org/nightly/cargo/reference/external-tools.html#compiler-messages

@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2024

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-cli-help Area: built-in command-line help A-console-output Area: Terminal output, colors, progress bar, etc. A-documenting-cargo-itself Area: Cargo's documentation A-timings Area: timings S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 17, 2024
@epage
Copy link
Contributor

epage commented Jan 17, 2024

I can see the value of keeping these consistent and we've already standardized the syntax, so lets go for it

@bors r+

@bors
Copy link
Contributor

bors commented Jan 17, 2024

📌 Commit 273168c has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 17, 2024
@bors
Copy link
Contributor

bors commented Jan 17, 2024

⌛ Testing commit 273168c with merge df0efb2...

bors added a commit that referenced this pull request Jan 17, 2024
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
@weihanglo
Copy link
Member Author

Spurious network issue:

test failed running `/Users/runner/work/cargo/cargo/target/debug/cargo publish --registry crates-io`
error: stderr did not match:
1   1         Updating crates.io index
    2    +warning: spurious network error (3 tries remaining): [35] SSL connect error (Recv failure: Connection reset by peer)
2   3     warning: unused config key `registry.unexpected-field` in `[..]config.toml`
3   4     error: no token found, please run `cargo login`
4   5     or use environment variable CARGO_REGISTRY_TOKEN

It basically succeeded on my fork.

@weihanglo
Copy link
Member Author

@bors r=epage

@bors
Copy link
Contributor

bors commented Jan 17, 2024

📌 Commit ad1a3b3 has been approved by epage

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 17, 2024

⌛ Testing commit ad1a3b3 with merge 1ae6310...

@bors
Copy link
Contributor

bors commented Jan 17, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing 1ae6310 to master...

@bors bors merged commit 1ae6310 into rust-lang:master Jan 17, 2024
13 of 20 checks passed
@weihanglo weihanglo deleted the pkgid-json-message branch January 17, 2024 18:11
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2024
Update cargo

2 commits in 1cff2ee6b92e0ad3f87c44b70b28f788b2528b3c..1ae631085f01c1a72d05df1ec81f3759a8360042
2024-01-16 16:56:57 +0000 to 2024-01-17 17:26:41 +0000
- fix(json-msg): use pkgid spec in in JSON messages (rust-lang/cargo#13311)
- doc(features): Highlight the non-blocking feature gating technique (rust-lang/cargo#13307)

r? oli-obk

Could you check if this fixes miri build?
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2024
Update cargo

2 commits in 1cff2ee6b92e0ad3f87c44b70b28f788b2528b3c..1ae631085f01c1a72d05df1ec81f3759a8360042
2024-01-16 16:56:57 +0000 to 2024-01-17 17:26:41 +0000
- fix(json-msg): use pkgid spec in in JSON messages (rust-lang/cargo#13311)
- doc(features): Highlight the non-blocking feature gating technique (rust-lang/cargo#13307)

r? oli-obk

Could you check if this fixes miri build?
@rustbot rustbot added this to the 1.77.0 milestone Jan 18, 2024
@vlad20012
Copy link
Member

vlad20012 commented Jan 18, 2024

I noticed that the id format is different in cargo metadata and cargo check output in today's nightly:

rustc --version
rustc 1.77.0-nightly (6ae4cfbbb 2024-01-17)

Does this version not yet include this fix? Or it is another bug?

@epage
Copy link
Contributor

epage commented Jan 18, 2024

rust-lang/rust#120071 needs to be in your nightly to have them align.

@sunshowers
Copy link
Contributor

Thanks for fixing this! Just wanted to mention that nextest matches up the package IDs from cargo metadata with the ones from JSON messages, so having consistency between the two is essential.

@weihanglo
Copy link
Member Author

The fix is now available on cargo 1.77.0-nightly (1ae631085 2024-01-17).

Sorry for overlooking the consistency issue in the first place 😞.
Regression tests are added in #13322 and hope it won't happen again.

Nadrieril added a commit to Nadrieril/rust that referenced this pull request Jan 21, 2024
…acrum

fix(rust-analyzer): use new pkgid spec to compare

Starting from rust-lang/cargo#13311, Cargo's compiler artifact message
uses Package ID specification as package's identifier format.

Zulip topic: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/proc-macro-test.20bootstrap.20and.20pkgid.20JSON

cc `@ehuss`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
Rollup merge of rust-lang#120084 - weihanglo:pkgid-spec, r=Mark-Simulacrum

fix(rust-analyzer): use new pkgid spec to compare

Starting from rust-lang/cargo#13311, Cargo's compiler artifact message
uses Package ID specification as package's identifier format.

Zulip topic: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/proc-macro-test.20bootstrap.20and.20pkgid.20JSON

cc `@ehuss`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jan 21, 2024
fix(rust-analyzer): use new pkgid spec to compare

Starting from rust-lang/cargo#13311, Cargo's compiler artifact message
uses Package ID specification as package's identifier format.

Zulip topic: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/proc-macro-test.20bootstrap.20and.20pkgid.20JSON

cc `@ehuss`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
fix(rust-analyzer): use new pkgid spec to compare

Starting from rust-lang/cargo#13311, Cargo's compiler artifact message
uses Package ID specification as package's identifier format.

Zulip topic: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/proc-macro-test.20bootstrap.20and.20pkgid.20JSON

cc `@ehuss`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
fix(rust-analyzer): use new pkgid spec to compare

Starting from rust-lang/cargo#13311, Cargo's compiler artifact message
uses Package ID specification as package's identifier format.

Zulip topic: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/proc-macro-test.20bootstrap.20and.20pkgid.20JSON

cc `@ehuss`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-cli-help Area: built-in command-line help A-console-output Area: Terminal output, colors, progress bar, etc. A-documenting-cargo-itself Area: Cargo's documentation A-timings Area: timings S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants