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

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Thomas Rast
Date: Monday, September 15, 2008 - 3:48 pm

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.

=2D Thomas

=2D-=20
Thomas Rast
trast@student.ethz.ch
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[IRC/patches] Failed octopus merge does not clean up, Thomas Rast, (Mon Sep 15, 3:48 pm)
[PATCH] Add test that checks octopus merge cleanup, Thomas Rast, (Mon Sep 15, 3:49 pm)
Re: [IRC/patches] Failed octopus merge does not clean up, Junio C Hamano, (Mon Sep 15, 4:36 pm)
Re: [IRC/patches] Failed octopus merge does not clean up, Junio C Hamano, (Mon Sep 15, 5:20 pm)
Re: [IRC/patches] Failed octopus merge does not clean up, Jakub Narebski, (Tue Sep 16, 3:53 pm)
Re: [IRC/patches] Failed octopus merge does not clean up, Andreas Ericsson, (Tue Sep 16, 11:45 pm)
Re: [IRC/patches] Failed octopus merge does not clean up, Jakub Narebski, (Wed Sep 17, 1:11 am)
Re: [IRC/patches] Failed octopus merge does not clean up, Andreas Ericsson, (Wed Sep 17, 9:40 am)