Skip to content
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.

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

Cannot dry-run cargo publish without bumping versions #14721

Closed
epage opened this issue Oct 23, 2024 · 5 comments · Fixed by #14742
Closed

Cannot dry-run cargo publish without bumping versions #14721

epage opened this issue Oct 23, 2024 · 5 comments · Fixed by #14742
Labels
C-bug Category: bug Command-publish regression-from-stable-to-beta Regression in beta that previously worked in stable. S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@epage
Copy link
Contributor

epage commented Oct 23, 2024

Problem

When a tool like cargo release runs in a dry-run mode, it shouldn't be bumping version numbers. However, as of #14448, cargo publish --dry-run will fail unless the version numbers are bumped

Steps

$ git clone [email protected]:clap-rs/clap.git
$ cd clap
$ cargo +beta release major -vvv

Possible Solution(s)

Revert #14448 which makes us lose out on early error reporting. However, if we view publish as plumbing with tools like cargo release handling it, make its not too bad

Skip #14448's check in dry-run mode or downgrade it to a warning. Deviating behavior between dry-run or not can be confusing and break other workflows.

Tell people to use cargo package. There are slight behavior differences though.

Notes

cargo 1.83.0-beta.2 (15fbd2f 2024-10-08)

Version

No response

@epage epage added C-bug Category: bug Command-publish regression-from-stable-to-beta Regression in beta that previously worked in stable. S-triage Status: This issue is waiting on initial triage. labels Oct 23, 2024
@weihanglo
Copy link
Member

weihanglo commented Oct 23, 2024

See also #14550 (comment)

@weihanglo
Copy link
Member

Skip #14448's check in dry-run mode or downgrade it to a warning. Deviating behavior between dry-run or not can be confusing and break other workflows.

What kind of workflows it may break?

The benefit of #14448 is it fails fast, so users don't need to wait for the verification step. Unless I misunderstood, it doesn't really change any final result of non dry-run publish.

@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Oct 25, 2024
@epage
Copy link
Contributor Author

epage commented Oct 25, 2024

I've been thinking more about that comment I made. A lot of it comes down to framing. If you view this as general validation, it should always be reported. If you view this as just reporting the upload error earlier, than warning or ignoring is perfectly fine.

I do not remember what I was considering when I said it could break other workflows.

@weihanglo
Copy link
Member

If it is not too hard to downgrade to a warning, I am slightly inclined to that.

@ehuss
Copy link
Contributor

ehuss commented Oct 29, 2024

The team discussed this today and decided to downgrade it to a warning for dry-run only, which is up at #14742.

bors added a commit that referenced this issue Oct 29, 2024
fix(publish): Downgrade version-exists error to warning on dry-run

### What does this PR try to resolve?

Especially for beta, this was the most conservative, minimal change.

Fixes #14721

### How should we test and review this PR?

### Additional information

This will get cherry-picked to beta
@bors bors closed this as completed in 947e1ff Oct 29, 2024
epage added a commit to epage/cargo that referenced this issue Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-publish regression-from-stable-to-beta Regression in beta that previously worked in stable. S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants