-
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
fix(install): Suggest an alternative version on MSRV failure #12798
Merged
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
This will also report the error without having to download the `.crate` first. If installing multiple packages, this will also report it immediately, rather than waiting for the other packages to be installed first. This also offers us more flexibility in the error we report, like suggesting more appropriate fixes.
The next step would be to also automatically install an MSRV compatible version if compatible with the version req (rust-lang#10903). This improved error message will still be useful if the MSRV compatible version is outside of the version req. I did this as the first step - Helps people now, not needing to wait on `-Zmsrv-policy` to be stabilized - Has fewer questions on how it should be done (or if it should be)
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-cli
Area: Command-line interface, option parsing, etc.
Command-install
Command-uninstall
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Oct 10, 2023
epage
commented
Oct 10, 2023
Comment on lines
+632
to
+637
// Remove any pre-release identifiers for easier comparison | ||
let current_version = &rustc.version; | ||
let untagged_version = semver::Version::new( | ||
current_version.major, | ||
current_version.minor, | ||
current_version.patch, | ||
); |
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.
This was copied from src/cargo/ops/cargo_compile/mod.rs
ehuss
reviewed
Oct 10, 2023
Thanks! @bors r+ |
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
Oct 10, 2023
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2023
Update cargo 5 commits in 794d0a82547f3081044c0aca7b6083733ce51344..6fa6fdc7606cfa664f9bee2fb33ee2ed904f4e88 2023-10-03 23:19:33 +0000 to 2023-10-10 23:06:08 +0000 - test(build): generalize test assertion for non-rustup env (rust-lang/cargo#12804) - chore: Sort dependency tables (rust-lang/cargo#12803) - fix(install): Suggest an alternative version on MSRV failure (rust-lang/cargo#12798) - rustdoc: remove the word "Version" from test cases (rust-lang/cargo#12800) - Add unsupported lowercase `-z` flag suggestion for `-Z` flag (rust-lang/cargo#12788) r? ghost
27 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-cli
Area: Command-line interface, option parsing, etc.
Command-install
Command-uninstall
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
to-announce
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?
Moves users from a bad error message, suggesting
--locked
which won't do anything, to suggesting a version of a package to use instead.The side benefit is errors get reported sooner
.crate
This comes at the cost of an extra
rustc
invocation.How should we test and review this PR?
Per-commit this builds it up, from tests to the final design.
Additional information
This is also written in a way to align fairly well with how we'd likely implement #10903.
This improved error message will still be useful after that issue is resolved when the MSRV compatible version is outside of the version req.