Skip to content

fix(package): Don't verify registry for --list #16341

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.

Sign up for GitHub

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

Jump to bottom
Open
epage wants to merge 7 commits into
base: master
Choose a base branch
from
Open

fix(package): Don't verify registry for --list #16341

epage wants to merge 7 commits into from
+25 −16

Conversation

@epage
Copy link
Contributor

@epage epage commented Dec 4, 2025

What does this PR try to resolve?

Since --list is doing nothing with registries, imo, it shouldn't error if
CARGO_REGISTRY_DEFAULT is not in package.publish.
This also affects --registry and --index but that should be fine.

Fixes crate-ci/cargo-release#921

How to test and review this PR?

@epage epage added the T-cargo Team: Cargo label Dec 4, 2025
@rustbot rustbot added Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2025

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

epage
epage commented Dec 4, 2025
View reviewed changes
tests/testsuite/package.rs Outdated
Comment on lines 6797 to 6802
p.cargo("package --registry alternative --list")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] `foo` cannot be packaged.
The registry `alternative` is not listed in the `package.publish` value in Cargo.toml.
.with_stdout_data(str![[r#"
Cargo.lock
Cargo.toml
Cargo.toml.orig
src/main.rs
Copy link
Contributor Author

@epage epage Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this turns an error case into a success case, I'm assuming we should have an FCP for this

Copy link
Member

@weihanglo weihanglo Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't think it is needed, but no harm to do it. We are going to miss the 1.93 window anyway.

@rustbot

This comment has been minimized.

Sign in to view
weihanglo
weihanglo approved these changes Dec 5, 2025
View reviewed changes
tests/testsuite/package.rs Outdated
Comment on lines 6797 to 6802
p.cargo("package --registry alternative --list")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] `foo` cannot be packaged.
The registry `alternative` is not listed in the `package.publish` value in Cargo.toml.
.with_stdout_data(str![[r#"
Cargo.lock
Cargo.toml
Cargo.toml.orig
src/main.rs
Copy link
Member

@weihanglo weihanglo Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't think it is needed, but no harm to do it. We are going to miss the 1.93 window anyway.

@weihanglo
Copy link
Member

weihanglo commented Dec 5, 2025 •
edited
Loading

@rfcbot fcp merge T-cargo

Before this change

  • cargo package --registry REG --list would fail if REG wasn’t in the package.publish list.
  • If config registry.default is set, cargo package --list can fail even without --registry CLI option specified.

After this change, it now succeeds. This closes a door to the possibility that (--registry/registry.default) + --list with package.publish specified may be useful in the future.

Note that it already succeeds with package.publish=false.

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Dec 5, 2025 •
edited by epage
Loading

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Dec 5, 2025
@epage
Copy link
Contributor Author

epage commented Dec 5, 2025

@rfcbot fcp merge T-cargo

Before this change

cargo package --registry REG --list would fail if REG wasn’t in the package.publish list.

After this change, it now succeeds. This closes a door to the possibility that --registry + --list with package.publish specified may be useful in the future.

Note that it isn't just with --registry that this failed but with registry.default being set which is the more interesting case because it means cargo package --list can fail.

@weihanglo
Copy link
Member

weihanglo commented Dec 5, 2025

Thanks. Included in the FCP text.

@rustbot

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@ehuss ehuss

Labels

Command-package disposition-merge FCP with intent to merge proposed-final-comment-period An FCP proposal has started, but not yet signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cargo Team: Cargo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cargo registry.default setting breaks registry checks

5 participants

@epage @rustbot @weihanglo @rust-rfcbot @ehuss