search

crate-ci / cargo-release

Cargo subcommand `release`: everything about releasing a rust crate.
Apache License 2.0
1.31k stars 109 forks source link

Release with version bump not robust against network errors #788

Closed kornelski closed 3 months ago

kornelski commented 3 months ago

I've expected that running cargo release minor --execute again after a publishing error to just resume publishing of the failed version, rather than to bump the version again.

  1. I've ran cargo release minor --execute. It created "chore: Release deunicode version 1.5.0" commit, and ran cargo publish
  2. Publish timed out due to a flaky network connection, and 1.5.0 has not been published. cargo release did not create the 1.5.0 tag, and did not push the bump commit.
  3. I've ran cargo release minor --execute again. Instead of reusing and tagging the 1.5.0 commit as I expected, it bumped version to 1.6.0, and published and tagged 1.6.0. This seems anomalous, since there was no 1.5.0 tag, and no 1.5.0 crates.io release.
epage commented 3 months ago

In general, recovery is difficult because there are a lot of different factors (publish, push, custom hooks, etc). Instead of trying to handle recovery behind the scenes, we try to give users the details of whats going on (e.g. the confirmation prompt showing the versions) and the individual steps within a release (via subcommands) for them to recover as they need.

In particular, I don't think we should be trying to out-guess the user on cargo release minor --execute. We can't tell what was a version bump from a failed release vs the user deciding on a different version to release with.

kornelski commented 3 months ago

I got desentisized to cargo-release questions, and I'm pressing enter + Y + enter with muscle memory, without thinking. It's because there's a delay before the yes/no question shows up, so I press Y immediately to skip waiting. Plus in all cases before this one the question was obvious and unnecessary.

Maybe the bump should be reverted on error then? Currently the release+bump isn't atomic, and I think atomicity would make sense for bump releases.

Leaving half-unfinished state for user to deal with seems inelegant to me. I expected more from a dedicated tool.

You should be able to recognize the bump commit made by cargo-release, and notice the discrepancy between local state, pushed repo state, and crates.io state.

epage commented 3 months ago

Maybe the bump should be reverted on error then? Currently the release+bump isn't atomic, and I think atomicity would make sense for bump releases.

We've had plans in the past for recovery, including imitating atomicity, and ran into problems with it and instead went the route we went. That is unlikely to change due to the complexity of all of the interacting pieces and the ability to detect situations, and correctly report where we are at with the user.

Since I'm not seeing anything actionable from this that is within scope of the project, I'm going to go ahead and close this.