Change branch in the middle of a merge

Hi folks,

In the past couple of years I've had a few instances of myself or others starting a merge on a particular branch, resolving a bunch of conflicts, realizing "this isn't ready for this branch yet", and wanting to commit the resulting resolved index state *and merge metadata* to a new local branch.

The "obvious" thing to do, like any other time you find you have some work you want to commit to a new branch instead of the one you were working on, is something like "git checkout -b lets-test-this-thing", followed by "git commit".

If you do that, you will probably *eventually* realize you didn't achieve your objective; the merge metadata disappeared when you created a new branch. The index is as you wanted it, and after you commit the committed tree is as you wanted it (so your testing can proceed), but in the commit dialog you don't get the expected merge message prepopulated, and after you commit you don't see the expected second parent in the commit metadata - it's not a merge.

As as an interested adventurous user, you might then search for something like "change branch in the middle of a merge" in google, and discover torek's great stackoverflow answer https://stackoverflow.com/a/45341429/74296 where he explains one thing you could/should do differently, once you understand this git behavior. Basically, *don't* switch to a new branch at that point. Instead, you could/should commit to the "wrong" branch locally, then create your testing branch from there, then reset your original branch, and thus you get where you wanted to be, with the original branch unchanged and a new branch containing the merge.

If you're even more adventurous you can find other ways to do this, of course, like switching to *another* new branch, committing there, then switching to the intended merge-testing branch, preparing a merge state with "-s ours --no-commit", and restoring the desired tree with "git restore" (to then commit normally in the branch where you wanted to).

If you are trying to provide *simple*, reliable instructions that don't require any depth of understanding of git nor any potentially-risky commands/habits, then you might instead just say "always *start* your (risky) merges on new temporary branches, instead of doing them on the branch where you eventually want them after the testing".

All three of these strategies are quite awful things to have to teach users.

At first I hoped that "git switch" might have fixed this - and in a limited sense it has, in that instead of *silently discarding* the merge state/metadata, it refuses with "fatal: cannot switch branch while merging".

I've briefly searched for discussion of this topic on this list in the last couple of years, but not found any.

I'd like to understand whether this behavior, the fact that you can't easily/straightforwardly/intuitively complete a merge onto a new branch (with the exact same "first parent" commit of course), is intentional for some reason that I am failing to grasp, or just a missing feature.

It makes complete sense, of course, that switching to a *different commit* than the one you were on when you started the merge, should be illegal... but why would switching to the *same* commit be either illegal (git switch behavior) or destructive (git checkout behavior), as opposed to just letting you get on with it, as it does with other in-progress local working tree states?

Assuming that I'm not missing anything, I'd like to propose a path forward something like this:

* Change "git switch" to just allow switching in the middle of a
merge, when the destination commit is the same as the previous commit,
without affecting the merge state/metadata. It is still experimental,
after all, so as long as we agree this is sane and helpful behavior,
the change should be welcome?
* Add a config option to "git checkout" to have it behave the same way
in this regard
* Add a hint to "git checkout", which kicks in when you are destroying
a merge state in this way, helping you see:
** That the merge state just disappeared
** That using "git switch" instead would have prevented this issue
** That there is also a config option to "git checkout" that would
have prevented this
** How to recover the lost merge state in this instance
* (update corresponding documentation)

The "How to recover the lost merge state" bit is non-trivial, the best I can come up with is... nasty, but at least looks like it should work across all the common shells I know of: ``` git switch -c temporary-merge-branch-<INITIAL-SHA> git commit -m "temporary commit of merge result" git checkout <THE-BRANCH-YOU-CHECKED-OUT> git merge -s ours <THE-BRANCH-THAT-HAD-BEEN-MERGED> --no-commit git restore --worktree --staged --source temporary-merge-branch-<INITIAL-SHA> -- . ```

If the behavior change to "git switch" is not acceptable, then a parameter, hint, and config option could be added so you can move forward from the current switch failure with the new behavior.

If the proposed behavior is for some reason utterly impossible to achieve, then I suppose a hint providing the above "here is now you get to where you probably wanted to be" procedure would be an absolute-no-brainer change that I could make.

Please let me know if I'm missing any history, concerns, etc. I haven't looked at the code to determine whether this is a direction I'd be comfortable trying to deliver on myself, but I'd like some understanding of whether it's a reasonable direction to be pursuing at all, before I try.

Thanks, Tao

Tao Klerks <tao@klerks.biz> writes:

I think this is in generally seen as a vast improvement over the "silently discarding" to have the safety valve.

I think this is mostly that nobody spent enough brain cycles to ponder on things like:

 - Is it always safe to switch from a merge in progress to a newly
   created branch that point at the same commit as HEAD?  Why?  Is
   the story the same if the switched-to branch is an existing one
   that happens to point at the same commit as HEAD?

 - Maybe the answer to the above question is not "it is not quite
   safe to switch during a conflicted merge, but by adjusting X and
   Y while switching to a new branch, we could make it safe".
 
 - Even if the answer to the question above is "yes it is always
   safe", if it is not safe for a similar situation that ends up in
   unmerged index (i.e. the index with any path at non-0 stage), the
   way we determine if we are in "a merge in progress" situation
   must be reliable.  Is it?

 - Conversely, perhaps it is also safe to switch to a different
   branch that points at the same commit as HEAD from the conflicted
   state after some (but not necessarily all) of the following
   operations: "am -3", "cherry-pick" (single commit), "revert"
   (single commit), etc.  Can we come up with a reliable way to
   determine if we are in such a situation?

 - These "other mergy operations" that can leave unmerged index may
   share the same "is not safe out of the box, but can be made safe
   by adjusting some stuff".

After that is done, we could update the codepath in "git switch" that says "ah, your index is unmerged, so I will not let you switch" to instead say "your index is unmerged; let's first see if you are switching to the same commit as HEAD and then the way your index is unmerged was caused by an operation that we decided is safe to switch out of. It seems to be one of the situations that could be made safe, so let's do adjustment X and Y and let you switch".

And the reason why "git switch" punts is because nobody bothered to, not because there fundamentally is some reason why it shouldn't work.

On Mon, May 1, 2023 at 5:48 PM Junio C Hamano <gitster@pobox.com> wrote:

Heh, that's fair - my apologies for the "ungrateful" tone.

(I'm not sure I *have* enough brain cycles, or background knowledge, to be sure of these things, but I have some tentative answers)

To the best of my knowledge, the only role that the HEAD *ref* plays in any of merge processing (before or after resolution), if there even is one, is its use in the commit message.

My proposal is that keeping the original prepared merge message, but warning about its use, makes more sense / is more straightforward than anything else we might choose to do here, like re-generating the message.

With respect to the safety of index operations - when conflicts have been resolved already the index is in a "normal" state, same as a non-merge checkout with changes, so there is no reason to believe there would be any "unsafeness" to worry about?

The sense in which it is potentially *not* safe, is that some part of checkout may mess with the index in some way that does not expect to encounter conflicts - given that conflicts explicitly prevent not only a "git switch", but also a "git checkout", at this time (due to a check for unmerged paths in builtin/checkout.c#merge_working_tree()).

As such, the "simple answer" is to address the case where conflicts are already resolved, and defer the case of open conflicts to someone who is better able to check or enable the safety of a switch in this state.

I believe the urgency/criticality of opening up a "switch while you still have unmerged paths" usecase is much lower than the simpler-to-prove "switch before committing an already-resolved index" usecase.

I'm not sure what "a path at 0 stage" means, but I do get the *impression* that the conflict checks that are in place in builtin/checkout.c#merge_working_tree() look reasonable...? That said, I do not intend to modify or circumvent these checks, because I don't think the user flow / use case requires it.

I suspect it is, but I also suspect this is a far less common case, and as such one I'm less inclined to worry about. In merge-based workflows, merges can be *large*, and the case where you have a merge ready, conflicts resolved, and you realize you'd like to test it on another branch, is more common and impactful, than when you have some (far more typically limited in scope) stash or cherry-pick...?

(Or maybe I'm just making excuses for the patch I want to submit :) )

My proposal at least is to carve out an exception, for now, only for merges, and only with a clean (not-conflicted) index.

This is what I am proposing, but no adjustment needed if the scope is a merge only and the index is already unconflicted.

Yep, that makes sense.

(background note: Chris accidentally sent a non-plaintext response, and I failed to respond for some time, but I still believe this is something worth fixing)

On Mon, Feb 27, 2023 at 10:16 PM Chris Torek <chris.torek@gmail.com> wrote:

Thanks for confirming, this helped motivate my at least attempting a patch.

It's not ready yet, but what I have so far seems to work.

I don't understand this bit, but I'm probably missing something.

The approach I've taken here is to, when the original commit and the new commit are the same (you're switching to a same-head branch, or creating a new branch at the current head), change the "remove_branch_state()" call at https://github.com/git/git/blob/2807bd2c10606e590886543afe4e4f208dddd489/builtin/checkout.c#L1012 / implemented at https://github.com/git/git/blob/2807bd2c10606e590886543afe4e4f208dddd489/branch.c#L818, to not do the "remove_merge_branch_state()" bit - that is, avoid all the removals at https://github.com/git/git/blob/2807bd2c10606e590886543afe4e4f208dddd489/branch.c#L808.

This behaves as expected for switching to new and expected branches. To change the "git checkout" behavior this change is basically all that's needed (plus tests etc), but for "git switch" we also need to change some validation which would refuse to do the switch in the middle of a merge.

With these changes things behave in what I think is a very reasonable way, with one potentially-surprising aspect: the merge message proposed when you go to commit still refers to the branch you were on when you prepared the merge, rather than the one you're on now. I personally believe this is quite reasonable (you'd still see this if you followed any other process involving fast-forwarding for example), but I can see how it's not perfect.

The bit I'm having more conceptual difficulty with is considering what
to "say" when a checkout/switch completes with a merge-in-progress.
Historically, this happened "as normal", with no special messaging.
Now, I think there should be a message depending on whether the merge
state was discarded (head commit changed) or not:
 * If the commit remained the same and so the merge state was
retained, state as much and warn about the commit message referring to
the previous branch name.
 * If the commit changed and so the merge state was discarded ("git
checkout" only - not possible with "git switch"), state as much and
suggest that using "git switch" would prevent this happening
accidentally in future.

Does this kind of messaging make sense to you?

After thinking about this a bit more, I agree that having divergent different behavior for checkout and switch would suck. The current existing divergences, as far as I can tell, are around validation/safety, not behavior when actions are completed.

I think the best thing is to propose a patch that incorporates a change in checkout behavior (stop throwing away merge metadata when you do a checkout/switch in the middle of a merge), with new messaging as outlined above, and see how people feel about it. I'm hopeful that a patch will see some more reactions than this thread did in its first round.