Re: [PATCH v3 3/3] git-merge-one-file: revise merge error reporting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 13/03/2013 19:57, Junio C Hamano wrote:
Kevin Bracey <kevin@xxxxxxxxx> writes:

-		echo "Added $4 in both, but differently."
+		echo "ERROR: Added $4 in both, but differently."
+		ret=1
The problem you identified may be worth fixing, but I do not think
this change is correct.

This message is at the same severity level as the message on the
other arm of this case that says "Auto-merging $4".  In that other
case arm, we are attempting a true three-way merge, and in this case
arm, we are attempting a similar three-way merge using your "virtual
base".

Neither has found any error in this case arm yet.  The messages are
both "informational", not an error.  I do not think you would want
to set ret=1 until you see content conflict.

I disagree here. At the minute, it does set ret to 1 (but further down the code - bringing it up here next to the "ERROR" print clarifies that), and will report the merge as failed, conflict in the 3-way merge or not. Which I think is correct.

We have to stop for user inspection here. We do have a fake base; we can't trust the 3-way merge with it.

The virtual 3-way merge will take ABCDE and ABDE and produce ABCDE without conflict. That's flat wrong if the real base they failed to tell Git about was ABCDE.

Despite being useful, I'm still slightly uncomfortable that it can produce something without any conflict markers. The user really needs to look at properly.

(And one interesting related glitch, or at least thing that puzzled me when it happened. This is from memory, so may be slightly mistaken, but what seemed to happen was that if you have rerere enabled, then mergetool tends to say "nothing to merge", because it relies on "rerere remaining", which relies on conflict markers. I think you could still force a mergetool up by specifying the specific file though.)

Maybe the virtual base itself should be different. Maybe it should put a ??????? marker in place of every unique line. So you get:

Left   ABCEFGH
Right XABCDEFJH  -> Merge result <|X>ABC<|D>EF<G|J>H
VBase ?ABC?EF??H

That actually feels like it may be the correct answer here. And it's effectively what P4Merge does in its "2-way" mode I failed to invoke. (At least for the result view).

Kevin


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]