cargo-release icon indicating copy to clipboard operation
cargo-release copied to clipboard

Create merge commit into the upstream branch in case it has moved forward

Open jstoneham opened this issue 5 years ago • 8 comments

If anyone merges to master during our release - and we have a pre-commit hook that takes a few minutes - the push fails. Can I suggest that cargo release pull and merge with the remote before pushing?

jstoneham avatar Sep 18 '19 21:09 jstoneham

Sorry for never getting to you on this.

I feel I'm mixed on whether it is more appropriate to pull or to error if local is behind remote. For example, if you write release notes before calling cargo-release, an error before cargo-release does anything will help you realize you might have written the release notes based on stale information.

In your particular case, it sounds like you have a race condition where someone could change master between the start and end of the release process. My concern is that it might be unclear which release some commits are associated with though I'm not aware of the right way to solve it.

epage avatar Nov 06 '19 03:11 epage

It can be confusing, but actually Git makes it work out rather well. In our case our pre-commit hook builds the software both to double-check that it works and to update Cargo.lock. This means if someone merges something in while that process is occurring, the push fails. Now we've cut and tagged a release but pushing it straight to master won't work. So we end up having to do a merge and push anyway.

Git will unify the history and track the separate streams, and the result would look like this: image

So if you look at HEAD, you see it's got two parents merging the two workstreams, but the release tag has only the ancestors you built and released from.

The way this would still fall down is if Cargo.toml got a merge conflict, but that is probably a rarer thing.

jstoneham avatar Nov 06 '19 04:11 jstoneham

So I guess maybe I wasn't clear, that we ought to tag before the pull. The pull is just to merge whatever's happened remotely with the result of what we already did locally, so the release tag contains exactly what you would have with the current release.

jstoneham avatar Nov 06 '19 04:11 jstoneham

So a couple of thoughts

  • Yes, git keeps both branches of the commit graph but the history can seem confusing to users nonetheless. You have a merge commit about releasing "v3.14" but before that merge commit is a commit that doesn't exist in "v3.14".
  • "n our case our pre-commit hook ... update[s] Cargo.lock" except we also update the lock file. Whats the chance that we'll have a merge conflict between the new development and the release?

epage avatar Nov 12 '19 15:11 epage

I'm not sure about the v3.14 commit you're referring to - maybe you picked an example 'pi' version number, since my diagram has v0.0.1 and v0.0.2. I'd suggest that yes, this is confusing, but likely only to new Git users, who are (I believe) less likely to end up in a situation where this would matter anyway. And, no more confusing than what actually occurred in real life, because the diagram actually reflects what occurred.

Yep, there's a chance for a merge conflict. We'd have to get the user to handle that manually.

I think the alternative, though, is failing on a push and forcing the user to pick up the pieces, where (to me) it seems clear what ought to happen. At minimum, it seems we ought to provide some guidance about how to recover.

jstoneham avatar Nov 12 '19 15:11 jstoneham

#264 is a little more generic (help users from doing this) and with #265, I'm starting off with a looser solution (just warn).

Do you expect this to be good enough that we merge these Issues or would you still like to leave the conversation open for more specific solutions?

epage avatar Jun 16 '21 01:06 epage

Oof, it's been a long time since I've thought about this and I'm not writing Rust day-to-day (though I may be again soon...).

I remember the problem here was the failure. You'll always have a race condition, though, where someone could push at the EXACT wrong moment...

I'm glad to hear that some progress is being made here. I think it would be interesting to look at what something like Maven does in this instance. The last time I used Maven was before I was using Git....

jstoneham avatar Jun 16 '21 02:06 jstoneham

Thats right, this is about reducing the gap for race conditions on heavy branches with many people with merge permissions.

epage avatar Jun 16 '21 13:06 epage