Re: two-way merge corner case bug
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 26, 2013, 21:41 UTC
- Message-ID
- <7vwqtulplp.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130226201820.GD13830@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 12 quoted lines
> On Tue, Feb 26, 2013 at 12:06:42PM -0800, Junio C Hamano wrote: > >> It seems that we have a corner case bug in two-way merge to reset >> away the conflicts with "read-tree -u --reset HEAD ORIG_HEAD". > > Isn't this a repeat of: > > http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316 > > whose fix never got finalized? It's on my todo list, but it seems that > items keep sinking further down on that list, rather than getting > completed. :-/
Yeah, I think the patch in your message is a good starting point to solve a half of the issue (i.e. unmerged current one should be replaced with the version from the "going to" tree). Attached is with a slight update.
The other half that is not solved by this patch is that we will lose Z because merge-recursive removed it when it renamed it and left three stages for A in the index, and to "read-tree --reset -u HEAD ORIG_HEAD", it looks as if you removed Z yourself while on HEAD and want to carry your local change forward to ORIG_HEAD.
I think this too needs to be fixed. We are saying "-u --reset" to mean "I want to go there no matter what", not "-u -m" that means "I am carrying my changes forward while checking out another branch".
unpack-trees.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c index 09e53df..3d17108 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1748,14 +1748,23 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o) newtree = NULL; if (current) { - if ((!oldtree && !newtree) || /* 4 and 5 */ - (!oldtree && newtree && - same(current, newtree)) || /* 6 and 7 */ - (oldtree && newtree && - same(oldtree, newtree)) || /* 14 and 15 */ - (oldtree && newtree && - !same(oldtree, newtree) && /* 18 and 19 */ - same(current, newtree))) { + if (current->ce_flags & CE_CONFLICTED) { + if (same(oldtree, newtree) || o->reset) { + if (!newtree) + return deleted_entry(current, current, o); + else + return merged_entry(newtree, current, o); + } + return o->gently ? -1 : reject_merge(current, o); + } + else if ((!oldtree && !newtree) || /* 4 and 5 */ + (!oldtree && newtree && + same(current, newtree)) || /* 6 and 7 */ + (oldtree && newtree && + same(oldtree, newtree)) || /* 14 and 15 */ + (oldtree && newtree && + !same(oldtree, newtree) && /* 18 and 19 */ + same(current, newtree))) { return keep_entry(current, o); } else if (oldtree && !newtree && same(current, oldtree)) {