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

[PATCHv2 2/2] merge-one-file: fix broken merges with alternate work trees

From
Jeff King <peff@peff.net>
Date
Apr 29, 2011, 22:24 UTC
Message-ID
<20110429222432.GB3347@sigill.intra.peff.net>
In-Reply-To
<20110429185228.GA27268@sigill.intra.peff.net>

The merge-one-file tool predates the invention of GIT_WORK_TREE. By the time GIT_WORK_TREE was invented, most people were using the merge-recursive strategy, which handles resolving internally. Therefore these features have had very little testing together.

For the most part, merge-one-file just works with GIT_WORK_TREE; most of its heavy lifting is done by plumbing commands which do respect GIT_WORK_TREE properly. The one exception is a shell redirection which touches the worktree directly, writing results to the wrong place in the presence of a GIT_WORK_TREE variable.

This means that merges won't even fail; they will silently produce incorrect results, throwing out the entire "theirs" side of files which need content-level merging!

This patch makes merge-one-file chdir to the toplevel of the working tree (and exit if we don't have one). This most closely matches the assumption made by the original script (before separate work trees were invented), and matches what happens when the script is called as part of a merge strategy.

While we're at it, we'll also error-check the call to cat. Merging a file in a subdirectory could in fact fail, as the redirection relies on the "checkout-index" call just prior to create leading directories. But we never noticed, since we ignored the error return from running cat.

Signed-off-by: Jeff King <peff@peff.net>
---
This one takes a totally different strategy than v1, but I think it
makes more sense (and it fixes the core.worktree bug).
 git-merge-one-file.sh  |    7 ++++++-
 t/t6060-merge-index.sh |    4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index b86402a..7aeb969 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -22,6 +22,11 @@ LONG_USAGE="Usage: git merge-one-file $USAGE
 
 Blob ids and modes should be empty for missing files."
 
+SUBDIRECTORY_OK=Yes
+. git-sh-setup
+cd_to_toplevel
+require_work_tree
+
 if ! test "$#" -eq 7
 then
 	echo "$LONG_USAGE"
@@ -132,7 +137,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 
 	# Create the working tree file, using "our tree" version from the
 	# index, and then store the result of the merge.
-	git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4"
+	git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4" || exit 1
 	rm -f -- "$orig" "$src1" "$src2"
 
 	if [ "$6" != "$7" ]; then
diff --git a/t/t6060-merge-index.sh b/t/t6060-merge-index.sh
index 895f079..debadbd 100755
--- a/t/t6060-merge-index.sh
+++ b/t/t6060-merge-index.sh
@@ -68,7 +68,7 @@ test_expect_success 'merge-one-file fails without a work tree' '
 	)
 '
 
-test_expect_failure 'merge-one-file respects GIT_WORK_TREE' '
+test_expect_success 'merge-one-file respects GIT_WORK_TREE' '
 	(cd bare.git &&
 	 mkdir work &&
 	 GIT_WORK_TREE=$PWD/work &&
@@ -82,7 +82,7 @@ test_expect_failure 'merge-one-file respects GIT_WORK_TREE' '
 	test_cmp expect-merged bare.git/work/file-index
 '
 
-test_expect_failure 'merge-one-file respects core.worktree' '
+test_expect_success 'merge-one-file respects core.worktree' '
 	mkdir subdir &&
 	git clone . subdir/child &&
 	(cd subdir &&
-- 
1.7.4.3.28.g10631
Previous: Jeff KingNext: Junio C Hamano
Message 3 of 5 in “Re: [PATCH 2/2] merge-one-file: fix broken merges with GIT_WORK_TREE”
  1. Jeff KingApr 29, 2011
  2. 1/2 add tests for merge-index / merge-one-fileJeff King, Apr 29, 2011
  3. 2/2 merge-one-file: fix broken merges with alternate work treesJeff King, Apr 29, 2011
  4. Junio C HamanoApr 29, 2011
  5. Jeff KingApr 29, 2011

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.