feat(update): Report when incompatible-rust-version packages are selected by epage · Pull Request #14401 · rust-lang/cargo

epage · 2024-08-14T17:27:17Z

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.

rustbot · 2024-08-14T17:27:22Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

src/cargo/ops/cargo_update.rs

…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.

src/cargo/ops/cargo_update.rs

weihanglo · 2024-08-16T20:12:27Z

@bors r+

bors · 2024-08-16T20:12:30Z

📌 Commit 428e173 has been approved by weihanglo

It is now in the queue for this repository.

weihanglo · 2024-08-16T20:23:48Z

src/cargo/ops/cargo_update.rs

+    if let Some(ver) = ws.rust_version() {
+        Some(ver.clone().into_partial())
+    } else {
+        let rustc = ws.gctx().load_global_rustc(Some(ws)).ok()?;


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.

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 · 2024-08-16T20:28:31Z

⌛ Testing commit 428e173 with merge fae72b0...

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.

bors · 2024-08-16T22:28:32Z

💥 Test timed out

weihanglo · 2024-08-16T22:44:23Z

@bors retry

bors · 2024-08-16T22:47:59Z

⌛ Testing commit 428e173 with merge 51482dc...

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.

bors · 2024-08-16T22:49:00Z

⌛ Testing commit 428e173 with merge ba8b394...

bors · 2024-08-16T23:19:29Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing ba8b394 to master...

bors · 2024-08-16T23:19:30Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing ba8b394 to master...

bors · 2024-08-16T23:20:28Z

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

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

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

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

refactor(update): Consolidate latest message rendering

0924f83

rustbot assigned ehuss Aug 14, 2024

rustbot added Command-update S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2024

epage force-pushed the required-rust branch 2 times, most recently from 209c3c9 to 339fefe Compare August 15, 2024 20:46

weihanglo reviewed Aug 16, 2024

View reviewed changes

src/cargo/ops/cargo_update.rs Outdated Show resolved Hide resolved

src/cargo/ops/cargo_update.rs Show resolved Hide resolved

src/cargo/ops/cargo_update.rs Show resolved Hide resolved

epage added 4 commits August 16, 2024 15:01

refactor(update): Consolidate latest calculation

ad4f647

refactor(update): Extract rust-version lookup

497c8b6

fix(update): Only show 'latest' messages for registry sources

a4c4c2d

epage force-pushed the required-rust branch from 339fefe to 428e173 Compare August 16, 2024 20:07

weihanglo approved these changes Aug 16, 2024

View reviewed changes

src/cargo/ops/cargo_update.rs Show resolved Hide resolved

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 Aug 16, 2024

weihanglo reviewed Aug 16, 2024

View reviewed changes

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

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 Aug 16, 2024

bors merged commit ba8b394 into rust-lang:master Aug 16, 2024

epage deleted the required-rust branch August 17, 2024 00:02

weihanglo mentioned this pull request Aug 17, 2024

Update cargo rust-lang/rust#129178

Merged

rustbot added this to the 1.82.0 milestone Aug 17, 2024

epage mentioned this pull request Aug 21, 2024

refactor(update): Prepare for smarter update messages #14440

Merged

This was referenced Aug 26, 2024

MSRV resolver has surprising behaviour in workspaces with mixed MSRV #14414

Closed

fix(resolve): Report incompatible packages with precise Rust version #14457

Merged

Call for Testing: MSRV-aware resolver #13873

Closed

Conversation

epage commented Aug 14, 2024 • edited by weihanglo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How should we test and review this PR?

Additional information

Uh oh!

rustbot commented Aug 14, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

weihanglo commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

weihanglo Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

epage Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

bors commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

weihanglo commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

epage commented Aug 14, 2024 •

edited by weihanglo

Loading