Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:30
Subsystem: the rest · Maintainer: Linus Torvalds

Possibly related (same subject, not in this thread)

Junio C Hamano [off-list ref] writes:
Kevin Bracey [off-list ref] writes:
quoted
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 <redacted>
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')?
Actually, this one is even better, I think.  Again on top of your
two patches applied on 'maint'.

Alternatively, we can remove the whole "if $1 is empty, error the
merge out" logic, which would be more in line with the spirit of
f7d24bbefb06 (merge with /dev/null as base, instead of punting
O==empty case, 2005-11-07), but that will be a change in behaviour
(a "both side added, slightly differently" case that can cleanly
merge will no longer fail), so I am not sure if it is worth it.

-- >8 --
Subject: [PATCH] merge-one-file: force content conflict for "both side added" case

Historically, we tried to be lenient to "both side added, slightly
differently" case and as long as the files can be merged using a
made-up common ancestor cleanly, since f7d24bbefb06 (merge with
/dev/null as base, instead of punting O==empty case, 2005-11-07).
This was later further refined to use a better made-up common file
with fd66dbf5297a (merge-one-file: use empty- or common-base
condintionally in two-stage merge., 2005-11-10), but the spirit has
been the same.

But the original fix in f7d24bbefb06 to avoid punging on "both sides
added" case had a code to unconditionally error out the merge.  When
this triggers, even though the content-level merge can be done
cleanly, we end up not saying "content conflict" in the message, but
still issue the error message, showing "ERROR:  in <pathname>".

Signed-off-by: Junio C Hamano <redacted>
---
 git-merge-one-file.sh | 1 +
 1 file changed, 1 insertion(+)
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 25d7714..62016f4 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -155,6 +155,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 	fi
 	if test -z "$1"
 	then
+		msg='content conflict'
 		ret=1
 	fi
 
-- 
1.8.2-297-g51e0fcd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help