Stashing during merge loses MERGING state

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

I did a large merge, resolved the conflicts but still had compile errors. In order to check how stuff has worked before the merge, I stashed all changes, and eventually popped the stash. Then the MERGING state was gone and committing created a single-parent commit rather than a merge commit with two parents. I was lucky to see that before pushing, so all is good.

Here is a simple recipe with a publicly available repo:

```sh
git clone https://github.com/magit/magit.git
# Current master is 4735b9254105eb7dd538f979d8b4c6ab96b827b9
cd magit
git merge origin/km/reshelve-rewrite # currently 0073bff21c826a57a4b48076074bdbba092d4b67
# Conflict in magit-extras.el
git add lisp/magit-extras.el
git stash
# The MERGING state is gone
git stash pop
# And it won't come back, so when I commit now, my "merge" has just
# one parent.
```

What did you expect to happen? (Expected behavior)

I expected that stashing during a merge will keep the MERGING state. Or that popping the stash again would also restore the MERGING state.

What happened instead? (Actual behavior)

The MERGING state is lost.

What's different between what you expected and what actually happened?

I've lost the MERGING state, thus committing results in a one-parent commit rather than a 2-parent merge commit.

Anything else you want to add:

In my original situation, I've copied the working tree to /tmp, resetted to origin/master, re-initiated the merge, copied the modified files from the /tmp backup, staged the changes, committed, and pushed.

I wonder if there would have been a better approach to come from the "accidentally having a single-parent commit containing all merged and resolution changes" state to a proper merge commit state. In the end, the tree was correct, just the second parent to the commit on the merged branch was missing.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.

[System Info]
git version:
git version 2.30.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.11.4-arch1-1 #1 SMP PREEMPT Sun, 07 Mar 2021 18:00:49 +0000 x86_64 compiler info: gnuc: 10.2
libc info: glibc: 2.33
$SHELL (typically, interactive shell): /usr/bin/fish

[Enabled Hooks]

Thanks for providing a clear recipe and expectation. However, I think Git is working here as intended. The MERGE_HEAD file (which is how "git status", the prompt, etc figure out that we're in the middle of a merge) is cleaned up when stash runs "git reset --hard" under the hood.

However, I don't think we would want to _not_ clear that file. The conflicted merge placed some changes into the index and working tree representing what happened on the branch you're merging in. Then making the stash (and the reset of the working tree) removes those changes. If we were to leave MERGE_HEAD in place and you ran "git commit", then it would create a merge commit that claims to have incorporated everything from the other branch, but has quietly dropped those changes as part of the merge resolution.

This would make more sense: the stash records that part of the state, and then we make it available again later when the stash is applied. However, that feature doesn't exist yet.

I can't offhand think of a reason it couldn't be implemented. It's possible that it would mess with somebody else's workflow (e.g., they think it's useful to stash some changes independent of the merging state, and then apply it later, perhaps while replaying the same or a similar merge). So it might need to be tied to a command-line option or similar.

-Peff

Jeff King <[email protected]> writes:

Hi Jeff,

Yes, that makes sense.

Too bad.

Everything breakes someones workflow [1], so an option would be fine.

However, I'd suggest to protect users shooting in their foot with a warning and confirmation query for the time being. I consider myself a quite experienced git user but this stash trouble today came totally unexpected. And I've asked on #[email protected] and got no answer which is totally uncommon. So I guess that this stash during merge thing is pretty much a gray area.

Bye,
Tassilo

[1] https://xkcd.com/1172/

Consider also what happens when git stash apply results in a merge conflict because of differences between your current index and the one you had when you originally saved the stash. This results in the usual merge conflict markers that then need to be cleaned up.

Could we sanely deal with this in a world where we also tried to restore .git/MERGE_HEAD when the stash was applied. Something like git stash apply --continue, possibly after resolving the stash conflicts? But what if we stashed the merge conflict that resulted from the stash apply? I guess it would still work, but the stash history would be, um, interesting.

I wonder if a fix could be as simple as recording the MERGE_HEAD as the third parent commit of the stash ref. I think that would provide all the information needed to put things back, except possibly for things like the rerere state, which is also set up during a conflict, and other incidentals like .git/MERGE_MSG. (And it feels like it might break compatibility with older versions that don't expect a third parent.)

I would be a bit concerned about the possibility of silently creating an "evil merge". Suppose you stash this conflict on some branch and then pop it onto a different one. I expect we would then be prepared to store all those changes from a different branch including existing resolved merge conflicts into the new one. That could be surprising and subtle.

But maybe I'm overthinking it. Wouldn't the stash apply result in merge conflicts that would catch out all the troubling parts?

I think being able to stash during a merge conflict could be very useful. I do sometimes need to get back to a working state momentarily and a merge conflict represents a long pole to doing so. Similarly, it could be useful to stash a conflicted git rebase so I could return to it later and pick up where I left off. Now we really would need to store some extra metadata, though, like the todo-list and ORIG_HEAD. And we would definitely need some extra command line switch to tell stash (or rebase) that I want to include all the rebase state and also "pause" the rebase by restoring to my starting point.

Thanks for raising the issue, Tassilo. This has obviously given me more ideas for things I forgot were missing.

I don't think we could easily add the warning when the stash is applied since we have forgotten the merge existed in the first place. So we would have to do it during stash save.

"Warning: You are stashing during a merge conflict and your merge state will not be restored by stash apply."

Seems reasonable.

Phil

I agree with you that allowing a stash while a merge is in progress will introduce many unpleasant corner cases the users wouldn't want to deal with. We certainly do prevent "git stash push" from running when the index is still unmerged (which is a sign that a mergy operation (like pull, rebase, merge, am -3, cherry-pick and revert that stops due to a conflict in the middle) is in progress), but once the end user resolves the conflicts in the index (either manually, or having the rerere.autoupdate feature in effect), such a sign of mergy operation still in progress that "git stash" currently uses will be gone. We should teach "git stash push" to pay attention to other such signs like MERGE_HEAD etc. and stop before creating a stash (and also do the same to "git stash pop/apply").

THanks.

I should have read the rest of the emails before responding. Once again, you manage to say roughly what I was thinking, but do so both more concisely and more eloquently.

Third parent is already reserved by --untracked to store a "parent" commit that has all the untracked files added. So, it'd be a fourth parent. Except when --untracked isn't passed we don't need the third parent, and if it wasn't a merge commit we don't need the fourth...fun times.

I don't think you're overthinking it; I think this is highly problematic. Let's consider the following history (from the git-rebase manpage):

               o---o---o---o---o  master
                    \
                     o---o---o---o---o  next
                                      \
                                       o---o---o  topic

First, let's consider the case where you don't record MERGE_HEAD when stashing. So, you start out on next and run:

   $ git merge topic

but you hit some conflicts and stash it:

   $ git stash push -m "in middle of merging topic"

Note that this would record applying the changes new to "topic" since "next", with whatever conflicts still exist. Then sometime later you check out master, and decide to apply the stash:

   $ git stash apply

What would you get? Just the changes in topic new relative to next, but nothing in next that preceded it. If you record that as a merge commit with topic as the second parent, you've created an evil merge.

Of course, that was saying if you didn't store MERGE_HEAD somewhere. But now let's say you did store MERGE_HEAD (== topic) when you stashed. What then happens when you go to apply the stash? If you really want a merge with MERGE_HEAD, then you need to redo the merge. So you invoke the merge machinery. That gives you conflicts anew. How do you use your stash at this point to resolve the set of conflicts, even if they were the same as what you got in your original merge?

Personally, I think we should make stash check for $GIT_DIR/MERGE_HEAD, and error out if it exists. I don't see an easy way to make this behave reasonably. (Which isn't to say I don't see ways, but it'd probably involve being able to do a diff since the point when the merge stopped and recording it, then attempting to reapply _that_ patch, one that removes conflict markers and such, on top of a redone merge, possibly complete with nested conflict markers. But I'm not sure this is sounding like a reasonable level of difficulty for users to deal with.)

I think it should be an error and abort the stash operation; this is way too much of a footgun.

There is already a third parent, but only when using -u / -a: this third-parent commit holds the untracked files (which are then removed a la git clean).

A better trick would, I think, be able to save a partial merge state in general, including the unmerged statuses of various files, the ongoing action (merge or one of the other things that use it), and so on, in a form that could be restored later. A plain git stash in any partially-merged state should tell you: no, use the fancier merge-state-saver instead.

This is the sort of thing I'm thinking of, for the "superstash" (terrible name for it). Note that whatever this becomes, it should be send-able (via push and/or email) so that you can have multiple people work on resolving a particularly hairy merge.

Chris

You can use git-imerge (https://github.com/mhagger/git-imerge) for that...just not once you've already started a merge (unless you're willing to undo and restart).

As per my other email, I'm pretty worried about attempting to stash a merge, given that stash allows its contents to be "reapplied" on top of a different commit. That works fine for single-parent commits, but merges have multiple parents and it'd be rather tricky to reapply such a stash on a different base without creating an evil merge. Perhaps if "superstash" forced you to only reapply on the same base then it'd be fine...but it's starting to no longer be a "stash" anymore but something else.