git stash while pending merge should not be allowed

git stash while pending merge should not be allowed

[Scott McPeak]

A colleague of mine who is well-intentioned and fairly knowledgeable
about git recently caused havoc with our repository while merging
changes on the main line branch into a feature branch.  The reason is
that, along the way, he tried to use "git stash" while the merge was
pending.  A few commands later, this severely polluted the history of
all the files involved.

The command sequence started as follows:

   $ git checkout feature-branch
   $ git merge mainline-branch
   (conflicts; merge pending)
   $ git mergetool
   (resolve all conflicts; takes a while as there are many)
   $ make && make testsuite
   (oops, tests don't pass)

At this point, the user notices that the fix to the tests is in the
next commit on mainline-branch, which wasn't present when it was first
merged.  Not wanting to have a commit in the history whose tests do
not pass, and not wanting to redo all the conflict resolution work
involved in starting over with a different merge parent, he tries to
cherry pick it:

   $ git fetch
   $ git cherry-pick commit-with-fix
   (error: can't cherry pick while merge pending)

Ok, he thinks, git stash has worked in the past as a way to temporarily
move aside some work in progress.  So:

   $ git stash
   $ git cherry-pick commit-with-fix
   $ git stash pop

All seems well, so:

   $ git commit

Unfortunately, this does *not* do what was intended.  You see,
unbeknownst to this developer, git stash does not save the MERGE_HEAD!
So the commit shows up as an enormous single-parent commit on top of
feature-branch, referring to none of the merged commits from
mainline-branch in its ancestry.  Consequently, "git annotate" and
friends regard all the merged changes as having been made by the
developer alone and all at once, distorting the file histories to the
point of being nearly useless.  Furthermore, git still doesn't think
mainline-branch has been merged, so there is more fun to be had
sorting that out when the next merge from mainline happens.

We are now in the middle of an expensive and risky project to repair
the damage, since people pulled and pushed afterward, before the
problem was noticed.

I suggest that this problem could easily have been avoided if "git
stash" refused to run with a pending merge (present MERGE_HEAD file),
since this is crucial repository state that it does not save.  This
seems similar to what "git cherry-pick" does.

-Scott

Re: git stash while pending merge should not be allowed

[Junio C Hamano]

Scott McPeak <smcpeak@coverity.com> writes:

> I suggest that this problem could easily have been avoided if "git
> stash" refused to run with a pending merge (present MERGE_HEAD file),
> since this is crucial repository state that it does not save.  This
> seems similar to what "git cherry-pick" does.

Sounds senslbe.  What do we want to see happen in other states, in
which Git gives control back to the user asking for help before
moving forward (e.g. am, rebase, cherry-pick, revert)?

Re: git stash while pending merge should not be allowed

[Scott McPeak]

On 06/07/13 11:47, Junio C Hamano wrote:
> Scott McPeak <smcpeak@coverity.com> writes:
>
>> I suggest that this problem could easily have been avoided if "git
>> stash" refused to run with a pending merge (present MERGE_HEAD file),
>> since this is crucial repository state that it does not save.  This
>> seems similar to what "git cherry-pick" does.
>
> Sounds senslbe.  What do we want to see happen in other states, in
> which Git gives control back to the user asking for help before
> moving forward (e.g. am, rebase, cherry-pick, revert)?

If you're asking me, I don't know.  My first thought is if there is any 
pending state that "stash" doesn't save, stash should refuse to run. 
But I don't know know very much about some of those commands.

-Scott

Re: git stash while pending merge should not be allowed

[John Keeping]

On Fri, Jun 07, 2013 at 11:47:07AM -0700, Junio C Hamano wrote:
> Scott McPeak <smcpeak@coverity.com> writes:
> 
> > I suggest that this problem could easily have been avoided if "git
> > stash" refused to run with a pending merge (present MERGE_HEAD file),
> > since this is crucial repository state that it does not save.  This
> > seems similar to what "git cherry-pick" does.
> 
> Sounds senslbe.  What do we want to see happen in other states, in
> which Git gives control back to the user asking for help before
> moving forward (e.g. am, rebase, cherry-pick, revert)?

I don't think there's any need to prevent stash running in these cases
and I sometimes find it useful that I can stash during a rebase.

Having said that, I wonder what happens with "cherry-pick -x" if you do
stash changes while it is stopped.  I don't think that is as serious as
the merge case because it's easy to detect in the commit message.