git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION

From
Jeff King <peff@peff.net>
Date
Jun 10, 2021, 12:57 UTC
Message-ID
<YMIMIdnDB1Jq+lzp@coredump.intra.peff.net>
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
Previous: Jeff KingNext: Junio C Hamano
Message 2 of 7 in “fix union merge with binary files”
  1. 0/2 fix union merge with binary filesJeff King, Jun 10, 2021
  2. 1/2 ll_binary_merge(): handle XDL_MERGE_FAVOR_UNIONJeff King, Jun 10, 2021
  3. Junio C HamanoJun 11, 2021
  4. 2/2 ll_union_merge(): pass name labels to ll_xdl_merge()Jeff King, Jun 10, 2021
  5. Elijah NewrenJun 10, 2021
  6. 3/2 ll_union_merge(): rename path_unused parameterJeff King, Jun 10, 2021
  7. Elijah NewrenJun 10, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.