feat(update): Report when incompatible-rust-version packages are selected #14401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
Command-update
S-waiting-on-review
epage
force-pushed
the
required-rust
branch
2 times, most recently
from
209c3c9
to
339fefe
Compare
weihanglo
reviewed
Aug 16, 2024
…cted In discussin this in rust-lang#13873, it highlighted that we need to make sure we tell people when we get in this state. I decided to keep "latest" and "required rust version" messages mutually exclusive to avoid too much noise. I gave "required rust version" higher precedence as its the more critical to operation and, if you are using an MSRV-incompatible package, it likely is "latest" already. I was tempted to change colors to make "required rust version" stand out from "latest" but was unsure what direction to go, so I held off. Options included - red for "required rust version", yellow for "latest" - yellow for "required rust version", nothing for "latest" There is also more discussion on how to format "latest" at rust-lang#13908.
weihanglo
approved these changes
Aug 16, 2024
|
@bors r+ |
bors
added
S-waiting-on-bors
weihanglo
reviewed
Aug 16, 2024
| if let Some(ver) = ws.rust_version() { | ||
| Some(ver.clone().into_partial()) | ||
| } else { | ||
| let rustc = ws.gctx().load_global_rustc(Some(ws)).ok()?; |
There was a problem hiding this comment.
May not be worth a fix. I didn't benchmark it.
This will load and parse .rustc_info.json in a loop. May have some potential performance impact.
There was a problem hiding this comment.
Yeah. I had thought of that and had considered passing it around but wasn't sure if it was worth it. We already are writing to the screen which is pretty slow.
bors
added a commit
that referenced
this pull request
Aug 16, 2024
feat(update): Report when incompatible-rust-version packages are selected ### What does this PR try to resolve? In discussin this in #13873, it highlighted that we need to make sure we tell people when incompatible-rust-version packages are selected. I decided to keep "latest" and "required rust version" messages mutually exclusive to avoid too much noise. I gave "required rust version" higher precedence as its the more critical to operation and, if you are using an MSRV-incompatible package, it likely is "latest" already. ### How should we test and review this PR? ### Additional information I was tempted to change colors to make "required rust version" stand out from "latest" but was unsure what direction to go, so I held off. Options included - red for "required rust version", yellow for "latest" - yellow for "required rust version", nothing for "latest" There is also more discussion on how to format "latest" at #13908.
|
💥 Test timed out |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Aug 16, 2024
feat(update): Report when incompatible-rust-version packages are selected ### What does this PR try to resolve? In discussin this in #13873, it highlighted that we need to make sure we tell people when incompatible-rust-version packages are selected. I decided to keep "latest" and "required rust version" messages mutually exclusive to avoid too much noise. I gave "required rust version" higher precedence as its the more critical to operation and, if you are using an MSRV-incompatible package, it likely is "latest" already. ### How should we test and review this PR? ### Additional information I was tempted to change colors to make "required rust version" stand out from "latest" but was unsure what direction to go, so I held off. Options included - red for "required rust version", yellow for "latest" - yellow for "required rust version", nothing for "latest" There is also more discussion on how to format "latest" at #13908.
|
☀️ Test successful - checks-actions |
1 similar comment
|
☀️ Test successful - checks-actions |
|
👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 17, 2024
Update cargo 8 commits in 2f738d617c6ead388f899802dd1a7fd66858a691..ba8b39413c74d08494f94a7542fe79aa636e1661 2024-08-13 10:57:52 +0000 to 2024-08-16 22:48:57 +0000 - feat(update): Report when incompatible-rust-version packages are selected (rust-lang/cargo#14401) - test: Migrate old_cargos to snapbox (rust-lang/cargo#14410) - Correct diagnostic for `TomlDebugInfo` (rust-lang/cargo#14413) - Add `--lockfile-path` flag (rust-lang/cargo#14326) - test: Migrate some json tests to snapbox (rust-lang/cargo#14402) - Implement base paths (RFC 3529) 1/n: path dep and patch support (rust-lang/cargo#14360) - doc: convert comments to rustdoc in workspace (rust-lang/cargo#14397) - Fix MSRV for workspace .package and .dependencies (rust-lang/cargo#14400) r? ghost
bors
added a commit
that referenced
this pull request
Aug 22, 2024
refactor(update): Prepare for smarter update messages ### What does this PR try to resolve? This is to make it easier to - Make #14401 path aware - Work on #13908, depending on what is decided - Improve the heuristics for when we show `Locking` messages There are fixes along the way that were found by making the code more consistent in prep for consolidating it, mostly for #14401. ### How should we test and review this PR? Along the way, some odd code structures are used for the sake of making the diff easier to follow. These get cleaned up towards the end I didn't add tests for the fixes because of the code consolidation that happens that causes us to cover more real-world scenarios with fewer tests. ### Additional information
This was referenced Aug 26, 2024
bors
added a commit
that referenced
this pull request
Aug 27, 2024
fix(resolve): Report incompatible packages with precise Rust version ### What does this PR try to resolve? In #14401, we reported about MSRV issues as if we were the resolver. We can be smarter than that though and report precise MSRV information. This allows us to elevate the message from color from yellow to red. This is also prep work for telling users when a newer, MSRV-compatible version of a package is available. ### How should we test and review this PR? The report function I added is a little odd because a follow up commit will update it to report when a package is incompatible with rustc when the MSRV resolver is disabled and do this on stable. ### Additional information This builds on #14445 to improve #14401
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR try to resolve?
In discussin this in #13873, it highlighted that we need to make sure we
tell people when incompatible-rust-version packages are selected.
I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise. I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.
How should we test and review this PR?
Additional information
I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
There is also more discussion on how to format "latest" at #13908.