fix(resolve): Dont show locking workspace members #14445

epage · 2024-08-22T22:05:50Z

What does this PR try to resolve?

This is for cargo generate-lockfile and when syncing the lockfile with
the manifest.
We still show it for cargo update because of cargo update --workspace.

We hacked around this previously by filtering out the num_pkgs==1 case
for single packages but this didn't help with workspaces.

How should we test and review this PR?

Additional information

This builds on #14440

This is for `cargo generate-lockfile` and when syncing the lockfile with the manifest. We still show it for `cargo update` because of `cargo update --workspace`. We hacked around this previously by filtering out the `num_pkgs==1` case for single packages but this didn't help with workspaces.

rustbot · 2024-08-22T22:05:55Z

r? @weihanglo

rustbot has assigned @weihanglo.
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

weihanglo · 2024-08-22T22:22:40Z

src/cargo/ops/cargo_update.rs

    }

-    fn with_diff(diff: impl Iterator<Item = PackageDiff>) -> Vec<Self> {
+    fn with_diff(diff: impl Iterator<Item = PackageDiff>, ws: &Workspace<'_>) -> Vec<Self> {
+        let member_ids: HashSet<_> = ws.members().map(|p| p.package_id()).collect();


Use Workspace::is_member instead?

Thats annoying, it requires a Package just to do a lookup with PackageId

weihanglo · 2024-08-22T22:26:17Z

src/cargo/ops/cargo_update.rs

@@ -787,20 +800,23 @@ struct PackageChange {
    package_id: PackageId,
    previous_id: Option<PackageId>,
    kind: PackageChangeKind,
+    is_member: Option<bool>,


I can't see the necessity of wrapping with an Option here except for representing “not relevant because it is a removed package”. Is that the reason of using it?

Yes, I wanted to make sure that when I accessed it, this state was clear. At first, I thought I was also going to have to act on it but that didn't end up happening.

weihanglo · 2024-08-22T22:32:26Z

src/cargo/ops/cargo_update.rs

The truth is that Cargo actually also locks versions of workspaces members, though I believe users often just don't care.

Yes. When I first did this, the intent was to inform users of relevant information. It can also be confusing to see these mesaages. I just made a change to some of my workspaces, removing version and cargo check reported that it downgraded those packages.

epage · 2024-08-23T02:14:29Z

This PR had a refactor on it, merging the cargo generate-lockfile and the lockfile sync (e.g. cargo check) reporting but I realized there was a difference that we might want to preserve: cargo generate-lockfile only shows Adds with relevant information to avoid overwhelming the user.

weihanglo · 2024-08-26T22:14:53Z

@bors r+

Thanks!

bors · 2024-08-26T22:14:55Z

📌 Commit d2ec764 has been approved by weihanglo

It is now in the queue for this repository.

bors · 2024-08-26T22:16:03Z

⌛ Testing commit d2ec764 with merge ef854d2...

bors · 2024-08-26T22:48:47Z

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

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

Update cargo 9 commits in 8f40fc59fb0c8df91c97405785197f3c630304ea..c1fa840a85eca53818895901a53fae34247448b2 2024-08-21 22:37:06 +0000 to 2024-08-29 21:03:53 +0000 - fix(resolve): With `latest` message, differentiate actionable updates (rust-lang/cargo#14461) - fix(pkgid): Allow open namespaces in PackageIdSpec's (rust-lang/cargo#14467) - feat(resolve): Report incompatible-with-rustc when MSRV-resolver is disabled (rust-lang/cargo#14459) - fix(resolve): Report incompatible packages with precise Rust version (rust-lang/cargo#14457) - fix(resolve): Dont show locking workspace members (rust-lang/cargo#14445) - Log details of failure if no errors were seen (rust-lang/cargo#14453) - More helpful missing feature error message (rust-lang/cargo#14436) - feat: Add matches_prerelease semantic (rust-lang/cargo#14305) - refactor(update): Prepare for smarter update messages (rust-lang/cargo#14440) r? ghost

rustbot assigned weihanglo Aug 22, 2024

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

weihanglo reviewed Aug 22, 2024

View reviewed changes

epage force-pushed the locked branch 7 times, most recently from 56fca86 to d2ec764 Compare August 24, 2024 01:17

weihanglo approved these changes Aug 26, 2024

View reviewed changes

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 26, 2024

bors merged commit ef854d2 into rust-lang:master Aug 26, 2024
22 checks passed

epage deleted the locked branch August 27, 2024 01:30

This was referenced Aug 27, 2024

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

Merged

cargo update should not highlight non-semver-compatible "(latest: v0.23.5)" for indirect dependencies #13908

Closed

weihanglo mentioned this pull request Aug 31, 2024

Update cargo rust-lang/rust#129798

Merged

rustbot added this to the 1.83.0 milestone Sep 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(resolve): Dont show locking workspace members #14445

fix(resolve): Dont show locking workspace members #14445

epage commented Aug 22, 2024

rustbot commented Aug 22, 2024

weihanglo Aug 22, 2024

epage Aug 23, 2024

weihanglo Aug 22, 2024

epage Aug 23, 2024

weihanglo Aug 22, 2024

epage Aug 23, 2024

epage commented Aug 23, 2024

weihanglo commented Aug 26, 2024

bors commented Aug 26, 2024

bors commented Aug 26, 2024

bors commented Aug 26, 2024

fix(resolve): Dont show locking workspace members #14445

fix(resolve): Dont show locking workspace members #14445

Conversation

epage commented Aug 22, 2024

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Aug 22, 2024

weihanglo Aug 22, 2024

Choose a reason for hiding this comment

epage Aug 23, 2024

Choose a reason for hiding this comment

weihanglo Aug 22, 2024

Choose a reason for hiding this comment

epage Aug 23, 2024

Choose a reason for hiding this comment

weihanglo Aug 22, 2024

Choose a reason for hiding this comment

epage Aug 23, 2024

Choose a reason for hiding this comment

epage commented Aug 23, 2024

weihanglo commented Aug 26, 2024

bors commented Aug 26, 2024

bors commented Aug 26, 2024

bors commented Aug 26, 2024