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

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

 



Kevin Bracey <kevin@xxxxxxxxx> writes:

> Commit 718135e improved the merge error reporting for the resolve
> strategy's merge conflict and permission conflict cases, but led to a
> malformed "ERROR:  in myfile.c" message in the case of a file added
> differently.
>
> This commit reverts that change, and uses an alternative approach without
> this flaw.
>
> Signed-off-by: Kevin Bracey <kevin@xxxxxxxxx>

We used to treat "Both added differently" as a separate "info"
message, just like the "Auto-merging" message, and let "content
conflict" that is an "error" to happen naturally by doing such a
merge, possibly followed by permission conflict which is another
kind of "error".  We coalesced these two into a single message.

And this patch breaks them into separate messages.  I am not sure if
that aspect of the change is desirable.

The source of "malformed" message seems suspicious.  Isn't the root
cause of $msg being empty that merge-file can (sometimes) cleanly
merge two files using the phoney base in the "both added
differently" codepath?

If you resolve that issue by forcing a "conflicted" failure when we
handle "add/add" conflict, I think the behaviour of the remainder of
the code is better in the original than the updated one.

Perhaps something like this (I am applying these on 'maint')?

 git-merge-one-file.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 25d7714..aa06282 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -107,6 +107,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 		;;
 	esac
 
+	add_add_conflict=
 	src2=`git-unpack-file $3`
 	case "$1" in
 	'')
@@ -121,6 +122,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 		# If we do not have enough common material, it is not
 		# worth trying two-file merge using common subsections.
 		expr $sz0 \< $sz1 \* 2 >/dev/null || : >$orig
+		add_add_conflict=yes
 		;;
 	*)
 		echo "Auto-merging $4"
@@ -128,15 +130,13 @@ case "${1:-.}${2:-.}${3:-.}" in
 		;;
 	esac
 
-	# Be careful for funny filename such as "-L" in "$4", which
-	# would confuse "merge" greatly.
 	src1=`git-unpack-file $2`
-	git merge-file "$src1" "$orig" "$src2"
-	ret=$?
-	msg=
-	if test $ret != 0
+
+	ret=0 msg=
+	if git merge-file "$src1" "$orig" "$src2" || test -n "$add_add_conflict"
 	then
 		msg='content conflict'
+		ret=1
 	fi
 
 	# Create the working tree file, using "our tree" version from the
--
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]