From ef7d3621d76fe210827698cba79c737906048191 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 9 Apr 2016 23:13:39 -0700 Subject: t7605: add a testcase demonstrating a bug with trivial merges Repeating a trivial merge more than once will leave the index out of sync, despite being clean before the merge and operating on the exact same heads as the first run. The recorded merge has the correct tree and the working tree is brought up to date, it is just the index that is left as it was before the merge. Every attempt to repeat the merge beyond the first will leave the index in the same weird out-of-sync state. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh index 0cb9d11..2f80037 100755 --- a/t/t7605-merge-resolve.sh +++ b/t/t7605-merge-resolve.sh @@ -27,7 +27,7 @@ test_expect_success 'setup' ' git tag c3 ' -test_expect_success 'merge c1 to c2' ' +merge_c1_to_c2_cmds=' git reset --hard c1 && git merge -s resolve c2 && test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" && @@ -41,6 +41,10 @@ test_expect_success 'merge c1 to c2' ' test 3 = $(git ls-files | wc -l) ' +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 c2 to c3 (fails)' ' git reset --hard c2 && test_must_fail git merge -s resolve c3 -- cgit v0.10.2-6-g49f6 From 40d71940b6a0dadb7873cce614992f1d71fcdfbd Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 9 Apr 2016 23:13:40 -0700 Subject: builtin/merge.c: fix a bug with trivial merges 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 Signed-off-by: Junio C Hamano diff --git a/builtin/merge.c b/builtin/merge.c index 41467e4..eb94151 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -822,6 +822,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 && -- cgit v0.10.2-6-g49f6