All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION
Date: Thu, 10 Jun 2021 08:57:05 -0400	[thread overview]
Message-ID: <YMIMIdnDB1Jq+lzp@coredump.intra.peff.net> (raw)
In-Reply-To: <YMIKwsEFnkqz6PWa@coredump.intra.peff.net>

Prior to commit a944af1d86 (merge: teach -Xours/-Xtheirs to binary
ll-merge driver, 2012-09-08), we always reported a conflict from
ll_binary_merge() by returning "1" (in the xdl_merge and ll_merge code,
this value is the number of conflict hunks). After that commit, we
report zero conflicts if the "variant" flag is set, under the assumption
that it is one of XDL_MERGE_FAVOR_OURS or XDL_MERGE_FAVOR_THEIRS.

But this gets confused by XDL_MERGE_FAVOR_UNION. We do not know how to
do a binary union merge, but erroneously report no conflicts anyway (and
just blindly use the "ours" content as the result).

Let's tighten our check to just the cases that a944af1d86 meant to
cover. This fixes the union case (which existed already back when that
commit was made), as well as future-proofing us against any other
variants that get added later.

Note that you can't trigger this from "git merge-file --union", as that
bails on binary files before even calling into the ll-merge machinery.
The test here uses the "union" merge attribute, which does erroneously
report a successful merge.

Signed-off-by: Jeff King <peff@peff.net>
---
 ll-merge.c            |  4 +++-
 t/t6406-merge-attr.sh | 17 +++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/ll-merge.c b/ll-merge.c
index 9a8a2c365c..145deb12fa 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -91,7 +91,9 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
 	 * With -Xtheirs or -Xours, we have cleanly merged;
 	 * otherwise we got a conflict.
 	 */
-	return (opts->variant ? 0 : 1);
+	return opts->variant == XDL_MERGE_FAVOR_OURS ||
+	       opts->variant == XDL_MERGE_FAVOR_THEIRS ?
+	       0 : 1;
 }
 
 static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
index d5a4ac2d81..c1c458d933 100755
--- a/t/t6406-merge-attr.sh
+++ b/t/t6406-merge-attr.sh
@@ -207,4 +207,21 @@ test_expect_success 'custom merge does not lock index' '
 	git merge main
 '
 
+test_expect_success 'binary files with union attribute' '
+	git checkout -b bin-main &&
+	printf "base\0" >bin.txt &&
+	echo "bin.txt merge=union" >.gitattributes &&
+	git add bin.txt .gitattributes &&
+	git commit -m base &&
+
+	printf "one\0" >bin.txt &&
+	git commit -am one &&
+
+	git checkout -b bin-side HEAD^ &&
+	printf "two\0" >bin.txt &&
+	git commit -am two &&
+
+	test_must_fail git merge bin-main
+'
+
 test_done
-- 
2.32.0.529.g079a794268


  reply	other threads:[~2021-06-10 12:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 12:51 [PATCH 0/2] fix union merge with binary files Jeff King
2021-06-10 12:57 ` Jeff King [this message]
2021-06-11  3:36   ` [PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION Junio C Hamano
2021-06-10 12:58 ` [PATCH 2/2] ll_union_merge(): pass name labels to ll_xdl_merge() Jeff King
2021-06-10 15:19   ` Elijah Newren
2021-06-10 16:14     ` [PATCH 3/2] ll_union_merge(): rename path_unused parameter Jeff King
2021-06-10 15:19 ` [PATCH 0/2] fix union merge with binary files Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YMIMIdnDB1Jq+lzp@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.