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

[PATCH 6/6] builtin/merge.c: Fix a bug with trivial merges

From
Elijah Newren <newren@gmail.com>
Date
Apr 10, 2016, 06:13 UTC
Message-ID
<1460268820-8308-7-git-send-email-newren@gmail.com>
In-Reply-To
<1460268820-8308-1-git-send-email-newren@gmail.com>

If read_tree_trivial succeeds and produces a tree that is already in the object store, then the index is not written to disk, leaving it out-of-sync with both HEAD and the working tree.

In order to write the index back out to disk after a merge,
write_index_locked() needs to be called.  For most merge strategies, this
is done from try_merge_strategy().  For fast forward updates, this is
done from checkout_fast_forward().  When trivial merges work, the call to
write_index_locked() is buried a little deeper:
  merge_trivial()
  -> write_tree_trivial()
     -> write_cache_as_tree()
        -> write_index_as_tree()
           -> write_locked_index()
However, it is only called when !cache_tree_fully_valid(), which is how
this bug is triggered.  But that also shows why this bug doesn't affect
any other merge strategies or cases.

Add a direct call to write_index_locked() from merge_trivial() to fix this issue. Since the indirect call to write_locked_index() was conditional on cache_tree_fully_valid(), it won't be written twice.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/merge.c          | 8 ++++++++
 t/t7605-merge-resolve.sh | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 101ffef..8615343 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -819,6 +819,14 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
 	unsigned char result_tree[20], result_commit[20];
 	struct commit_list *parents, **pptr = &parents;
+	static struct lock_file lock;
+
+	hold_locked_index(&lock, 1);
+	refresh_cache(REFRESH_QUIET);
+	if (active_cache_changed &&
+	    write_locked_index(&the_index, &lock, COMMIT_LOCK))
+		return error(_("Unable to write index."));
+	rollback_lock_file(&lock);
 
 	write_tree_trivial(result_tree);
 	printf(_("Wonderful.\n"));
diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh
index 2f80037..5d56c38 100755
--- a/t/t7605-merge-resolve.sh
+++ b/t/t7605-merge-resolve.sh
@@ -43,7 +43,7 @@ merge_c1_to_c2_cmds='
 
 test_expect_success 'merge c1 to c2'        "$merge_c1_to_c2_cmds"
 
-test_expect_failure 'merge c1 to c2, again' "$merge_c1_to_c2_cmds"
+test_expect_success 'merge c1 to c2, again' "$merge_c1_to_c2_cmds"
 
 test_expect_success 'merge c2 to c3 (fails)' '
 	git reset --hard c2 &&
-- 
2.8.0.21.g229f62a
Previous: Elijah NewrenNext: Junio C Hamano
Message 8 of 10 in “Miscellaneous merge fixes”
  1. 0/6 Miscellaneous merge fixesElijah Newren, Apr 10, 2016
  2. 1/6 Remove duplicate codeElijah Newren, Apr 10, 2016
  3. 2/6 Avoid checking working copy when creating a virtual merge baseElijah Newren, Apr 10, 2016
  4. Junio C HamanoApr 13, 2016
  5. 3/6 Add merge testcases for when index doesn't match HEADElijah Newren, Apr 10, 2016
  6. 4/6 merge-octopus: Abort if index does not match HEADElijah Newren, Apr 10, 2016
  7. 5/6 Add a testcase demonstrating a bug with trivial mergesElijah Newren, Apr 10, 2016
  8. 6/6 builtin/merge.c: Fix a bug with trivial mergesElijah Newren, Apr 10, 2016
  9. Junio C HamanoApr 13, 2016
  10. Elijah NewrenApr 15, 2016

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.