merge: fix numerus bugs around "trivial merge" area

The "trivial merge" codepath wants to optimize itself by making an
internal call to the read-tree machinery, but it does not read the index
before doing so, and the codepath is never exercised.  Incidentally, this
failure to read the index upfront means that the safety to refuse doing
anything when the index is unmerged does not kick in, either.

These two problem are fixed by using read_cache_unmerged() that does read
the index before checking if it is unmerged at the beginning of
cmd_merge().

The primary logic of the merge, however, assumes that the process never
reads the index in-core, and the call to write_cache_as_tree() it makes
from write_tree_trivial() will always read from the on-disk index that is
prepared the strategy back-ends.  This assumption is now broken by the
above fix.  To fix this issue, we now call discard_cache() before calling
write_tree_trivial() when it wants to write the on-disk index as a tree.

When multiple strategies are tried, their results are evaluated by reading
the resulting index and inspecting it.  The codepath needs to make a call
to read_cache() for each successful strategy, and for that to work, they
need to discard_cache() the one read by the previous round.

Also the "trivial merge" forgot that the current commit is one of the
parents of the resulting commit.

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

diff --git a/builtin-merge.c b/builtin-merge.c
index a201c6628d6b2c736e53d69ef75accec7d73b4b6..b280444e10d67355da6fd0d9e1a2dd2d7a29d440 100644 (file)
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -564,8 +564,6 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote)
        struct dir_struct dir;
        struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
-       if (read_cache_unmerged())
-               die("you need to resolve your current index first");
        refresh_cache(REFRESH_QUIET);
 
        fd = hold_locked_index(lock_file, 1);
@@ -651,13 +649,15 @@ static void add_strategies(const char *string, unsigned attr)
 static int merge_trivial(void)
 {
        unsigned char result_tree[20], result_commit[20];
-       struct commit_list parent;
+       struct commit_list *parent = xmalloc(sizeof(struct commit_list *));
 
        write_tree_trivial(result_tree);
        printf("Wonderful.\n");
-       parent.item = remoteheads->item;
-       parent.next = NULL;
-       commit_tree(merge_msg.buf, result_tree, &parent, result_commit);
+       parent->item = lookup_commit(head);
+       parent->next = xmalloc(sizeof(struct commit_list *));
+       parent->next->item = remoteheads->item;
+       parent->next->next = NULL;
+       commit_tree(merge_msg.buf, result_tree, parent, result_commit);
        finish(result_commit, "In-index merge");
        drop_save();
        return 0;
@@ -743,6 +743,7 @@ static int evaluate_result(void)
        int cnt = 0;
        struct rev_info rev;
 
+       discard_cache();
        if (read_cache() < 0)
                die("failed to read the cache");
 
@@ -776,7 +777,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
        struct commit_list **remotes = &remoteheads;
 
        setup_work_tree();
-       if (unmerged_cache())
+       if (read_cache_unmerged())
                die("You are in the middle of a conflicted merge.");
 
        /*
@@ -1073,6 +1074,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
                }
 
                /* Automerge succeeded. */
+               discard_cache();
                write_tree_trivial(result_tree);
                automerge_was_ok = 1;
                break;

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index aff360303ae2a304bff4799def6906defdb85843..f2880152b019f4d77e3b743d7a7b02ba6d197d11 100755 (executable)
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -269,6 +269,17 @@ test_expect_success 'merge-recursive result' '
 
 '
 
+test_expect_success 'fail if the index has unresolved entries' '
+
+       rm -fr [abcd] &&
+       git checkout -f "$c1" &&
+
+       test_must_fail git merge "$c5" &&
+       test_must_fail git merge "$c5" 2> out &&
+       grep "You are in the middle of a conflicted merge" out
+
+'
+
 test_expect_success 'merge-recursive remove conflict' '
 
        rm -fr [abcd] &&

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index fee8fb77d48ef7d4fa08e77f879718d7ab5b1bc0..dbc90bc41625fccb4119cd6c2fc49668fe784122 100755 (executable)
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -498,4 +498,13 @@ test_expect_success 'merge fast-forward in a dirty tree' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'in-index merge' '
+       git reset --hard c0 &&
+       git merge --no-ff -s resolve c1 > out &&
+       grep "Wonderful." out &&
+       verify_parents $c0 $c1
+'
+
+test_debug 'gitk --all'
+
 test_done

diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh
index ee21a107fd99e35b9d6325a040b7549b7a6c2502..f1f86ddb2381ce70e6845298e86ed96d6399fdf9 100755 (executable)
--- a/t/t7605-merge-resolve.sh
+++ b/t/t7605-merge-resolve.sh
@@ -36,7 +36,9 @@ test_expect_success 'merge c1 to c2' '
        git diff --exit-code &&
        test -f c0.c &&
        test -f c1.c &&
-       test -f c2.c
+       test -f c2.c &&
+       test 3 = $(git ls-tree -r HEAD | wc -l) &&
+       test 3 = $(git ls-files | wc -l)
 '
 
 test_expect_success 'merge c2 to c3 (fails)' '