[PATCH] merge-recursive: do not look at the index during recursive merge

[Junio C Hamano <gitster@pobox.com>]

When merging another branch into ours, if their tree is the same as
the common ancestor's, we can declare that our tree represents the
result of three-way merge.  In such a case, the recursive merge
backend incorrectly used to create a commit out of our index, even
when the index has changes.

A recent fix attempted to prevent this by adding a comparison
between "our" tree and the index, but forgot that this check must be
restricted only to the outermost merge.  Inner merges performed by
the recursive backend across merge bases are by definition made from
scratch without having any local changes added to the index.  The
call to index_has_changes() during an inner merge is working on the
index that has no relation to the merge being performed, preventing
legitimate merges from getting carried out.

Fix it by limiting the check to the outermost merge.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

    Junio C Hamano <gitster@pobox.com> writes:

    > Elijah Newren <newren@gmail.com> writes:
    >
    >> diff --git a/merge-recursive.c b/merge-recursive.c
    >> index 2ecf495cc2..780f81a8bd 100644
    >> --- a/merge-recursive.c
    >> +++ b/merge-recursive.c
    >> @@ -1952,6 +1952,13 @@ int merge_trees(struct merge_options *o,
    >>  	}
    >>  
    >>  	if (oid_eq(&common->object.oid, &merge->object.oid)) {
    >> +		struct strbuf sb = STRBUF_INIT;
    >> +
    >> +		if (index_has_changes(&sb)) {
    >> +			err(o, _("Dirty index: cannot merge (dirty: %s)"),
    >> +			    sb.buf);
    >> +			return 0;
    >> +		}
    >>  		output(o, 0, _("Already up to date!"));
    >>  		*result = head;
    >>  		return 1;
    >
    > I haven't come up with an addition to the test suite, but I suspect
    > this change is conceptually wrong.  What if a call to this function
    > is made during a recursive, inner merge?

    Now I have.

 merge-recursive.c          |  2 +-
 t/t3030-merge-recursive.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 780f81a8bd..0fc580d8ca 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1954,7 +1954,7 @@ int merge_trees(struct merge_options *o,
 	if (oid_eq(&common->object.oid, &merge->object.oid)) {
 		struct strbuf sb = STRBUF_INIT;
 
-		if (index_has_changes(&sb)) {
+		if (!o->call_depth && index_has_changes(&sb)) {
 			err(o, _("Dirty index: cannot merge (dirty: %s)"),
 			    sb.buf);
 			return 0;
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 9a893b5fe7..12e3b1392d 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -678,4 +678,54 @@ test_expect_success 'merge-recursive remembers the names of all base trees' '
 	test_cmp expect actual
 '
 
+test_expect_success 'merge-recursive internal merge resolves to the sameness' '
+	git reset --hard HEAD &&
+
+	# We are going to create a history leading to two criss-cross
+	# branches A and B.  The common ancestor at the bottom, O0,
+	# has two childs O1 and O2, both of which will be merge base
+	# between A and B, like so:
+	#
+	#       O1---A
+	#      /  \ /
+	#    O0    .
+	#      \  / \
+	#       O2---B
+	#
+	# The recently added "check to see if the index is different from
+	# the tree into which something else is getting merged into and
+	# reject" check must NOT kick in when an inner merge between O1
+	# and O2 is made.  Both O1 and O2 happen to have the same tree
+	# as O0 in this test to trigger the bug---whether the inner merge
+	# is made by merging O2 into O1 or O1 into O2, their common ancestor
+	# O0 and the branch being merged have the same tree, and the code
+	# before fix will incorrectly try to look at the index.
+
+	echo "zero" >file &&
+	git add file &&
+	test_tick &&
+	git commit -m "O0" &&
+	O0=$(git rev-parse HEAD) &&
+
+	test_tick &&
+	git commit --allow-empty -m "O2" &&
+	O1=$(git rev-parse HEAD) &&
+
+	git reset --hard $O0 &&
+	test_tick &&
+	git commit --allow-empty -m "O2" &&
+	O2=$(git rev-parse HEAD) &&
+
+	test_tick &&
+	git merge -s ours $O1 &&
+	B=$(git rev-parse HEAD) &&
+
+	git reset --hard $O1 &&
+	test_tick &&
+	git merge -s ours $O2 &&
+	A=$(git rev-parse HEAD) &&
+
+	git merge $B
+'
+
 test_done
-- 
2.16.0-rc1-164-gb9fca19b00