Skip to content

refactor: drop yanked_whitelist from source loading#17014

Merged
epage merged 1 commit into
rust-lang:masterfrom
weihanglo:allowlist
Jun 5, 2026
Merged

refactor: drop yanked_whitelist from source loading#17014
epage merged 1 commit into
rust-lang:masterfrom
weihanglo:allowlist

Conversation

@weihanglo

Copy link
Copy Markdown
Member

What does this PR try to resolve?

Split off from #17012 for an easier review experience I guess.

Most call sites constructed with an empty allowlist,
and PackageRegistry::load was the only place that passed a real list
so we do a post construction after PackageRegistry in load.
The parameter was only load-bearing for PackageRegistry.

How to test and review this PR?

This has no user-facing behavior change.

Most call sites constructed with an empty allowlist,
and `PackageRegistry::load` was the only place that passed a real list
so we do a post construction after PackageRegistry construction in `load`.
The parameter was only load-bearing for `PackageRegistry`.
@rustbot rustbot added A-future-incompat Area: future incompatible reporting A-interacts-with-crates.io Area: interaction with registries A-registries Area: registries Command-install Command-publish Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2026
@rustbot

rustbot commented May 19, 2026

Copy link
Copy Markdown
Collaborator

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

@epage

epage commented May 19, 2026

Copy link
Copy Markdown
Contributor

On its own, this is likely fine and I would be fine reviewing and merging if you want but I wonder if we should go in a different direction for the motivating case behind this.

I worry about the complexity that has grown around yanked as we have added features around it, like --precise and that grows stronger in considering doing something similar for other features like min-publish-age.

One half-developed idea is that Source has no knowledge about yanked but passes up the enum of different types of summaries. VersionPreferences would then decide whether to filter out yanked items or not. One possible way of doing this is for anything in VersionPreferences that is preferred is implicitly blessed for yanked, min publish age, etc.

@weihanglo

Copy link
Copy Markdown
Member Author

On its own, this is likely fine and I would be fine reviewing and merging if you want but I wonder if we should go in a different direction for the motivating case behind this.

While the original motivation of this PR wasn't about the larger refactor, removing whitelist from Source would always happen if we are going to move yanking behavior up to resolver. This refactor somehow minimizes future diff from touching too many files. Still worth merging I guess.

@epage epage added this pull request to the merge queue Jun 5, 2026
Merged via the queue into rust-lang:master with commit 71b70c0 Jun 5, 2026
46 of 58 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2026
@weihanglo weihanglo deleted the allowlist branch June 6, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-future-incompat Area: future incompatible reporting A-interacts-with-crates.io Area: interaction with registries A-registries Area: registries Command-install Command-publish Command-vendor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants