-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Handle $message_type in JSON diagnostics #13016
Conversation
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
Thanks! @bors r+ |
Handle $message_type in JSON diagnostics ### What does this PR try to resolve? Unblocks rust-lang/rust#115691. Without this change, Cargo's testsuite fails in `doc::doc_message_format` and `metabuild::metabuild_failed_build_json`. ### How should we test and review this PR? Tested with and without rust-lang/rust#115691. In Cargo repo: `cargo test --test testsuite` In Rust repo: `x.py test src/tools/cargo` (separately on master and $message_type PR)
// Compilers older than 1.76 do not produce $message_type. | ||
// Treat it as optional for now. | ||
let mut expected_entries_without_message_type; | ||
let expected_entries: &mut dyn Iterator<Item = _> = | ||
if l.contains_key("$message_type") && !r.contains_key("$message_type") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specializing this for the compiler runs counter to a (slow moving) effort I'm working on to use a generic, third party library rather than us using bespoke testing tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can delete this code as soon as Rust 1.76 is stable (February 8) and Cargo's MSRV is raised. From #12775 it looks like MSRV update is immediate upon stabilization. I have filed #13017 to follow up.
If the generic third-party library adoption proceeds faster than that, you would need to figure out how to accommodate this situation in a principled way. This will not be the last time that rustc's JSON is tweaked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo this should have been done principled before merging rather than punting that onto others.
💔 Test failed - checks-actions |
@bors retry issue with the ubuntu azure mirror. |
☀️ Test successful - checks-actions |
Update cargo 9 commits in 9765a449d9b7341c2b49b88da41c2268ea599720..71cd3a926f0cf41eeaf9f2a7f2194b2aff85b0f6 2023-11-17 20:58:23 +0000 to 2023-11-20 15:30:57 +0000 - Handle $message_type in JSON diagnostics (rust-lang/cargo#13016) - refactor(toml): Further clean up inheritance (rust-lang/cargo#13000) - Fix `--check-cfg` invocations with zero features (rust-lang/cargo#13011) - chore: bump `cargo-credential-*` crates as e58b84d broke stuff (rust-lang/cargo#13010) - contrib docs: Update now that credential crates are published. (rust-lang/cargo#13006) - Add more resources to the contrib docs. (rust-lang/cargo#13008) - Respect `rust-lang/rust`'s `omit-git-hash` (rust-lang/cargo#12968) - Fix clippy-wrapper test race condition. (rust-lang/cargo#12999) - fix(resolver): Don't do git fetches when updating workspace members (rust-lang/cargo#12975)
What does this PR try to resolve?
Unblocks rust-lang/rust#115691.
Without this change, Cargo's testsuite fails in
doc::doc_message_format
andmetabuild::metabuild_failed_build_json
.How should we test and review this PR?
Tested with and without rust-lang/rust#115691.
In Cargo repo:
cargo test --test testsuite
In Rust repo:
x.py test src/tools/cargo
(separately on master and $message_type PR)