refactor(resolver): move yank policy to resolver layer by weihanglo · Pull Request #17083 · rust-lang/cargo

weihanglo · 2026-06-07T14:55:24Z

What does this PR try to resolve?

This PR moves move yank policy to resolver layer from source layer.

We mostly preserve the old Source::query behavior that filter out yanked by default.

The yanking policy logic doesn't go into VersionPreferences or sort_summaries yet because

We want to minimize the refactor change. Changing the order of query and filter may change existing diagnostics
[replace] resolution should not see yanked candidates either
A future VersionPreferences predicate may still need to be invoked at this same point.

cc #17012, #17014 (comment)

How to test and review this PR?

Three tests are added to pin down more existing tests, along with #17082

Here is the list of all call site I audit:

Call site	Yanked versions handling
`core/resolver/dep_cache.rs` `query`	Filter
`core/resolver/dep_cache.rs` `[replace]` override	Filter
`core/registry.rs` `patch` collection	Filter
`core/registry.rs` `query_overrides`	Filter
`core/registry.rs` `summary_for_patch` unlocked-retry	Filter
`core/registry.rs` `summary_for_patch` name-only diagnostic	Filter
`core/resolver/errors.rs` `alt_versions`	Filter
`core/resolver/errors.rs` `rejected_versions`	Keep
`core/resolver/errors.rs` `alt_names`	Keep
`ops/common_for_install_and_uninstall.rs` `select_dep_pkg` (x2)	Filter
`ops/cargo_update.rs` `upgrade_dependency`	Filter
`ops/cargo_update.rs` `report_latest` (x4)	Filter
`ops/cargo_add/mod.rs` `get_latest_dependency`	Filter
`ops/cargo_add/mod.rs` `select_package`	Filter
`ops/cargo_add/mod.rs` `populate_available_features`	Filter
`ops/registry/info/mod.rs` `query_summaries`	Filter
`ops/registry/publish.rs` `poll_one_package`	Filter
`ops/registry/publish.rs` `verify_unpublished`	Filter
`core/compiler/future_incompat.rs` `get_updates`	Filter
`crates/xtask-bump-check/src/xtask.rs` `check`	Filter

Error message is not helpful here because `source::query` has filtered out yanked versions. Later install didn't know how to recover from that. Only exact requirement has informative error message. This will be improved when moving yanked version handling to resolver

rustbot · 2026-06-07T14:55:29Z

r? @epage

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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @ehuss, @epage, @weihanglo
@ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
Random selection from ehuss, epage

weihanglo · 2026-06-07T15:03:27Z

-                false
-            };
-            if is_yanked {
+            // Let's see if there is any yanked version so can give a more concrete error.


This showcases where this refactor could be useful.

View changes since the review

weihanglo · 2026-06-07T15:04:08Z

    let summaries = crate::util::block_on(source.query_vec(&query, QueryKind::Exact))?;
-    let available = summaries.iter().any(|s| !s.as_summary().is_yanked());
-    Ok(available)
+    Ok(!summaries.is_empty())


This showcases where this refactor could be useful.

View changes since the review

epage · 2026-06-08T17:44:24Z

    /// Available for consideration
    Candidate(Summary),
-    /// Yanked within its registry
-    Yanked(Summary),


I'm hesitant about dropping this. Having it as an enum ensures explicit acknowledgement rather than hoping callers know to check this.

View changes since the review

We can keep this, though I wonder how we envision on IndexSummary evolution. Do we want to have variant like IndexSummary::TooNew for min-publish-age? It was done in #17012. Or we treat yanked a bit special but not others.

Actually having IndexSummary::TooNew may help as we can flag those at registry level, and apply the policy at resolver level. What do you think?

explicit acknowledgement rather than hoping callers know to check this.

Caller still need to manually check because we have IndexSummary::{as_summary, into_summary} providing a error-prone API bypassing it. And they are all over the places we call Summary::is_yanked in this PR.

Is there anything we can do to help in this situation?

What is your read on how worth it this has been in the end?

I experimented a branch removing as_summary and into_summary. It was noisy. Some call sites only need something like s.as_summary().version() and maybe for that we should expose IndexSummary::version. Or maybe Invalid/Unsupported/Offline should be folded into one Unavailable and have their sub-enum.

Actually having IndexSummary::TooNew may help as we can flag those at registry level, and apply the policy at resolver level. What do you think?

This may look weird, as IndexSummary::parse currently doesn't have any registry specific logic. If we construct IndexSummary::TooNew there we need to thread registry config into. If we don't, we need to remap IndexSummary during query just like how IndexSummary::Offline is doing now.

On the other hand, the current change (removing IndexSummary:Yanked) is also weird because it pushes the yanked field down to Summary which is shared with git/path packages as well, and they don't have the concept of yanking.

I don't have a good answer to it though, maybe we can start with not removing IndexSummary::Yanked and see what we can do next?

Done the refactor. I plan to remove into_summary and as_summary in follow-up.

epage · 2026-06-10T20:52:58Z

+                // Prefer yanked only when
+                //
+                // * it is recorded in lock file or a `[patch]` entry
+                // * it is specified in `cargo update --precise`
+                IndexSummary::Yanked(summary) => {
+                    let pkg_id = summary.package_id();
+                    let allow_precise = pkg_id
+                        .source_id()
+                        .precise_registry_version(pkg_id.name().as_str())
+                        .is_some_and(|(_, to)| to == pkg_id.version());
+                    if allow_precise || self.version_prefs.should_prefer(&pkg_id) {


Isn't this beyond what the current code does for this to be just a refactor?

View changes since the review

Oh, its because this logic got moved up into this layer

If so, why isn't this needed at other call sites?

Oh, is it because this code is only active within this branch?

Yes, I think you answered yourself :)

epage

Thanks!

View changes since this review

### What does this PR try to resolve? `cargo_package::check_yanked` is the only external caller. Replace it with a direct `Source::query` instead since we want `Source` to be yanked-naive. This is an follow-up of rust-lang#17083 ### How to test and review this PR?

…17092) ### What does this PR try to resolve? `Summary::{as_summary,into_summary}` are easy to misuse. * Removed `into_summary` as it has only one caller. * Changed the visibility of `as_summary` and renamed it to `as_summary_unchecked`. This is a follow-up of rust-lang#17083 ### How to test and review this PR?

Update cargo submodule 11 commits in 0b1123a48825309b697312b44fdb64b3df00c958..fe63976b245b8a649c3f2949bf89fdc307bfbae4 2026-06-01 21:20:28 +0000 to 2026-06-11 09:17:57 +0000 - refactor: reduce use `Summary::{as_summary,into_summary}` (rust-lang/cargo#17092) - docs(diag): Provide jumping off points for writing diagnostics and passes (rust-lang/cargo#17090) - refactor(source): drop `Source::is_yanked` (rust-lang/cargo#17091) - refactor(resolver): move yank policy to resolver layer (rust-lang/cargo#17083) - fix(publish): avoid false deadlock when to_confirm is non-empty (rust-lang/cargo#17071) - doc(guide): use fresh actions/checkout version in GH actions examples (rust-lang/cargo#17087) - fix: strip CR from `cargo:token-from-stdout` (rust-lang/cargo#17081) - test: show some odd `--precise` cases (rust-lang/cargo#17082) - chore(deps): update rust crate gix to 0.84.0 (rust-lang/cargo#17063) - refactor: drop `yanked_whitelist` from source loading (rust-lang/cargo#17014) - chore(deps): update msrv to v1.96 (rust-lang/cargo#17041)

rustbot assigned epage Jun 7, 2026

weihanglo added 2 commits June 7, 2026 11:02

test(publish): duplicate-version check skips yanked versions

7bb68a2

test(replace): [replace] override path against yanked

89a5dfc

weihanglo force-pushed the yanked-refactor branch from e171161 to f4a63f0 Compare June 7, 2026 15:02

weihanglo commented Jun 7, 2026

View reviewed changes

epage reviewed Jun 8, 2026

View reviewed changes

refactor(resolver): extract should_prefer function

9bc7940

weihanglo force-pushed the yanked-refactor branch from f4a63f0 to 0a51a03 Compare June 10, 2026 03:46

weihanglo added 3 commits June 9, 2026 23:52

fix(install): better error when pkg is yanked

2adb4a8

fix(publish): bail when target version exists but yanked

9041c48

weihanglo force-pushed the yanked-refactor branch from 0a51a03 to 9041c48 Compare June 10, 2026 03:52

weihanglo requested a review from epage June 10, 2026 20:35

epage reviewed Jun 10, 2026

View reviewed changes

epage approved these changes Jun 10, 2026

View reviewed changes

epage added this pull request to the merge queue Jun 10, 2026

Merged via the queue into rust-lang:master with commit 3fd8987 Jun 10, 2026
29 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 10, 2026

weihanglo deleted the yanked-refactor branch June 10, 2026 22:47

weihanglo mentioned this pull request Jun 10, 2026

refactor(source): drop Source::is_yanked #17091

Merged

weihanglo mentioned this pull request Jun 11, 2026

refactor: reduce use Summary::{as_summary,into_summary} #17092

Merged

weihanglo mentioned this pull request Jun 11, 2026

Update cargo submodule rust-lang/rust#157760

Merged

rustbot added this to the 1.98.0 milestone Jun 11, 2026

Conversation

weihanglo commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

rustbot commented Jun 7, 2026

Uh oh!

weihanglo Jun 7, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Jun 7, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epage Jun 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epage Jun 10, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epage left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

weihanglo commented Jun 7, 2026 •

edited

Loading

weihanglo Jun 7, 2026 •

edited by rustbot

Loading

weihanglo Jun 7, 2026 •

edited by rustbot

Loading

epage Jun 8, 2026 •

edited by rustbot

Loading

epage Jun 10, 2026 •

edited by rustbot

Loading

epage left a comment •

edited by rustbot

Loading