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(install): Suggest an alternative version on MSRV failure #12798

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Oct 10, 2023

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

  • Before downloading the .crate
  • When installing multiple packages, before building the first

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.

epage added 3 commits October 9, 2023 15:48
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)
@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2023

r? @ehuss

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

@rustbot 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
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,
);
Copy link
Contributor Author

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
Copy link
Contributor

ehuss commented Oct 10, 2023

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 10, 2023

📌 Commit 9b32be7 has been approved by ehuss

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 Oct 10, 2023
@bors
Copy link
Contributor

bors commented Oct 10, 2023

⌛ Testing commit 9b32be7 with merge 8ba1c31...

@bors
Copy link
Contributor

bors commented Oct 10, 2023

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 8ba1c31 to master...

@bors bors merged commit 8ba1c31 into rust-lang:master Oct 10, 2023
19 checks passed
@epage epage deleted the msrv-install branch October 10, 2023 21:06
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
@ehuss ehuss added this to the 1.75.0 milestone Oct 11, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants