[IRC/patches] Failed octopus merge does not clean up

Hi *

James "jammyd" Mulcahy pointed out on IRC that the octopus merge strategy doesn't properly clean up behind itself. To wit:

  git init
  echo initial > foo
  git add foo
  git commit -m initial
  echo a > foo
  git commit -m a foo
  git checkout -b b HEAD^
  echo b > foo
  git commit -m b foo
  git checkout -b c HEAD^
  echo c > foo
  git commit -m c foo
  git checkout master
  git merge b c

The merge says

  Trying simple merge with 5b3e4bb1c2d88d6967fb575729fbfc86df5eaec9
  Simple merge did not work, trying automatic merge.
  Auto-merging foo
  ERROR: Merge conflict in foo
  fatal: merge program failed
  Automated merge did not work.
  Should not be doing an Octopus.
  Merge with strategy octopus failed.

So far so good.  However, 'git status' claims

  #       unmerged:   foo

and indeed the contents of 'foo' are the conflicted merge between 'master' and 'b', yet there is no .git/MERGE_HEAD. This behaviour is identical for 1.5.6 and 1.6.0.2, so it is not caused by the merge rewrite as a builtin. Shouldn't it either really clean up, or really leave the repo in a conflicted merge state? (I'm following up with a patch that turns the above into a test. Octopus doesn't really have many tests, does it?)

On the code path to the "Merge with strategy %s failed" message, builtin-merge.c:1134 runs restore_state() which runs reset_hard(). But (as Miklos pointed out) that cannot actually do 'git reset --hard' because it is possible (though not recommended, see below) to start a merge with a dirty index.

Jakub mentioned that there are only three index stages for a three-way merge, so a conflicted n-way (simultaneous) merge is not really possible.

The merge manpage should warn about merging with uncommitted changes. It recommends 'git rebase --hard' to abort during conflicts, but does not mention that this throws away said changes. I'm following up with a patch for this.

- Thomas