git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [IRC/patches] Failed octopus merge does not clean up
@ 2008-09-15 22:48 Thomas Rast
  2008-09-15 22:49 ` [PATCH] Add test that checks octopus merge cleanup Thomas Rast
  2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Thomas Rast @ 2008-09-15 22:48 UTC (permalink / raw)
  To: git; +Cc: Miklos Vajna, Jakub Narebski

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

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

-- 
Thomas Rast
trast@student.ethz.ch



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2008-09-19 20:29 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-15 22:48 [IRC/patches] Failed octopus merge does not clean up Thomas Rast
2008-09-15 22:49 ` [PATCH] Add test that checks octopus merge cleanup Thomas Rast
2008-09-15 22:49   ` [PATCH] Documentation: warn against merging in a dirty tree Thomas Rast
2008-09-15 23:42     ` Junio C Hamano
2008-09-15 23:53       ` Avery Pennarun
2008-09-16  0:06         ` Junio C Hamano
2008-09-18 15:15         ` Linus Torvalds
2008-09-18 18:18           ` Avery Pennarun
2008-09-19 20:28           ` Junio C Hamano
2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
2008-09-15 23:47   ` Thomas Rast
2008-09-16  0:20   ` Junio C Hamano
2008-09-16 22:53     ` Jakub Narebski
2008-09-17  6:45       ` Andreas Ericsson
2008-09-17  8:11         ` Jakub Narebski
2008-09-17 15:59           ` Miklos Vajna
2008-09-17 16:40             ` Andreas Ericsson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).