Re: two-way merge corner case bug

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2013, 23:49 UTC
Message-ID: <7va9qmel4s.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130301230845.GA7317@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

>> I actually was wondering if we can remove that sole uses of two-way
>> merge with --reset -u from "git am" and replace them with something
>> else.  But we do want to keep local change that existed before "am"
>> started, so we cannot avoid use of two-way merge, I guess...
>
> Yeah, I think that is a case we definitely want to keep, as it means any
> intermediate work done by the user in applying the patch is not lost.

I am not sure what you mean by "intermediate" here. When the user attempts to resolve conflict in a path manually and gives up, we do want to revert changes to such a path to that of HEAD.

Clarifying the semantics of "--reset" needs to be done carefully.

I think any difference "git diff --cached" shows are fair game to revert to HEAD. In the earlier example, path "Z" that was created by recursive merge in an attempt to rename "A" should be removed, and path "A" should be recreated to that of HEAD, as we know at the point of "am --skip/--abort" that these two paths were involved in the recursive merge invoked by the patch application process (that is the only possible reason why these are different in the index from HEAD). Also any conflicting entries can only come from three-way merge and they should be reverted to that of HEAD.

On the other hand, the paths that appear in "git diff" (except for those that appear in "git diff --cached", which we will revert to HEAD following the logic of the previous paragraph) must be kept. They are changes that were already present in the working tree before the user decided to accept a trivial patch via "am" that does not overlap with what the user was doing. We allow a dirty working tree but disallow a dirty index when "git am" starts, exactly because we want to ensure this property.

By doing both of the above, we should be able to satisfy the user who uses "am --abort/--skip", in order to restore paths that were involved in the failed attempt to three-way merge to revert back to that of HEAD, while keeping unrelated changes that were present before "am" started.