Merge branch 'jk/two-way-merge-corner-case-fix'

Fix a rather longstanding corner-case bug in twoway "reset to
there" merge, which is most often seen in "git am --abort".

* jk/two-way-merge-corner-case-fix:
t1005: add test for "read-tree --reset -u A B"
t1005: reindent
unpack-trees: fix "read-tree -u --reset A B" with conflicted index

diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh
index abc2c6f57fe29d124455227a16e241c49ae40c5c..ef079afc4638a4f02fa0c12f1d54ad622047ed56 100644 (file)
--- a/t/lib-read-tree.sh
+++ b/t/lib-read-tree.sh
@@ -5,39 +5,39 @@
 # write the index and that together with -u it doesn't touch the work tree.
 #
 read_tree_must_succeed () {
-    git ls-files -s >pre-dry-run &&
-    git read-tree -n "$@" &&
-    git ls-files -s >post-dry-run &&
-    test_cmp pre-dry-run post-dry-run &&
-    git read-tree "$@"
+       git ls-files -s >pre-dry-run &&
+       git read-tree -n "$@" &&
+       git ls-files -s >post-dry-run &&
+       test_cmp pre-dry-run post-dry-run &&
+       git read-tree "$@"
 }
 
 read_tree_must_fail () {
-    git ls-files -s >pre-dry-run &&
-    test_must_fail git read-tree -n "$@" &&
-    git ls-files -s >post-dry-run &&
-    test_cmp pre-dry-run post-dry-run &&
-    test_must_fail git read-tree "$@"
+       git ls-files -s >pre-dry-run &&
+       test_must_fail git read-tree -n "$@" &&
+       git ls-files -s >post-dry-run &&
+       test_cmp pre-dry-run post-dry-run &&
+       test_must_fail git read-tree "$@"
 }
 
 read_tree_u_must_succeed () {
-    git ls-files -s >pre-dry-run &&
-    git diff-files -p >pre-dry-run-wt &&
-    git read-tree -n "$@" &&
-    git ls-files -s >post-dry-run &&
-    git diff-files -p >post-dry-run-wt &&
-    test_cmp pre-dry-run post-dry-run &&
-    test_cmp pre-dry-run-wt post-dry-run-wt &&
-    git read-tree "$@"
+       git ls-files -s >pre-dry-run &&
+       git diff-files -p >pre-dry-run-wt &&
+       git read-tree -n "$@" &&
+       git ls-files -s >post-dry-run &&
+       git diff-files -p >post-dry-run-wt &&
+       test_cmp pre-dry-run post-dry-run &&
+       test_cmp pre-dry-run-wt post-dry-run-wt &&
+       git read-tree "$@"
 }
 
 read_tree_u_must_fail () {
-    git ls-files -s >pre-dry-run &&
-    git diff-files -p >pre-dry-run-wt &&
-    test_must_fail git read-tree -n "$@" &&
-    git ls-files -s >post-dry-run &&
-    git diff-files -p >post-dry-run-wt &&
-    test_cmp pre-dry-run post-dry-run &&
-    test_cmp pre-dry-run-wt post-dry-run-wt &&
-    test_must_fail git read-tree "$@"
+       git ls-files -s >pre-dry-run &&
+       git diff-files -p >pre-dry-run-wt &&
+       test_must_fail git read-tree -n "$@" &&
+       git ls-files -s >post-dry-run &&
+       git diff-files -p >post-dry-run-wt &&
+       test_cmp pre-dry-run post-dry-run &&
+       test_cmp pre-dry-run-wt post-dry-run-wt &&
+       test_must_fail git read-tree "$@"
 }

diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh
index f53de79e565afcbe53a016a56a59b27831a3ce42..074568500a357d3bae69a953aa59155687483a10 100755 (executable)
--- a/t/t1005-read-tree-reset.sh
+++ b/t/t1005-read-tree-reset.sh
@@ -8,84 +8,99 @@ test_description='read-tree -u --reset'
 # two-tree test
 
 test_expect_success 'setup' '
-  git init &&
-  mkdir df &&
-  echo content >df/file &&
-  git add df/file &&
-  git commit -m one &&
-  git ls-files >expect &&
-  rm -rf df &&
-  echo content >df &&
-  git add df &&
-  echo content >new &&
-  git add new &&
-  git commit -m two
+       git init &&
+       mkdir df &&
+       echo content >df/file &&
+       git add df/file &&
+       git commit -m one &&
+       git ls-files >expect &&
+       rm -rf df &&
+       echo content >df &&
+       git add df &&
+       echo content >new &&
+       git add new &&
+       git commit -m two
 '
 
 test_expect_success 'reset should work' '
-  read_tree_u_must_succeed -u --reset HEAD^ &&
-  git ls-files >actual &&
-  test_cmp expect actual
+       read_tree_u_must_succeed -u --reset HEAD^ &&
+       git ls-files >actual &&
+       test_cmp expect actual
 '
 
 test_expect_success 'reset should remove remnants from a failed merge' '
-  read_tree_u_must_succeed --reset -u HEAD &&
-  git ls-files -s >expect &&
-  sha1=$(git rev-parse :new) &&
-  (
-       echo "100644 $sha1 1    old"
-       echo "100644 $sha1 3    old"
-  ) | git update-index --index-info &&
-  >old &&
-  git ls-files -s &&
-  read_tree_u_must_succeed --reset -u HEAD &&
-  git ls-files -s >actual &&
-  ! test -f old
+       read_tree_u_must_succeed --reset -u HEAD &&
+       git ls-files -s >expect &&
+       sha1=$(git rev-parse :new) &&
+       (
+               echo "100644 $sha1 1    old"
+               echo "100644 $sha1 3    old"
+       ) | git update-index --index-info &&
+       >old &&
+       git ls-files -s &&
+       read_tree_u_must_succeed --reset -u HEAD &&
+       git ls-files -s >actual &&
+       ! test -f old
+'
+
+test_expect_success 'two-way reset should remove remnants too' '
+       read_tree_u_must_succeed --reset -u HEAD &&
+       git ls-files -s >expect &&
+       sha1=$(git rev-parse :new) &&
+       (
+               echo "100644 $sha1 1    old"
+               echo "100644 $sha1 3    old"
+       ) | git update-index --index-info &&
+       >old &&
+       git ls-files -s &&
+       read_tree_u_must_succeed --reset -u HEAD HEAD &&
+       git ls-files -s >actual &&
+       ! test -f old
 '
 
 test_expect_success 'Porcelain reset should remove remnants too' '
-  read_tree_u_must_succeed --reset -u HEAD &&
-  git ls-files -s >expect &&
-  sha1=$(git rev-parse :new) &&
-  (
-       echo "100644 $sha1 1    old"
-       echo "100644 $sha1 3    old"
-  ) | git update-index --index-info &&
-  >old &&
-  git ls-files -s &&
-  git reset --hard &&
-  git ls-files -s >actual &&
-  ! test -f old
+       read_tree_u_must_succeed --reset -u HEAD &&
+       git ls-files -s >expect &&
+       sha1=$(git rev-parse :new) &&
+       (
+               echo "100644 $sha1 1    old"
+               echo "100644 $sha1 3    old"
+       ) | git update-index --index-info &&
+       >old &&
+       git ls-files -s &&
+       git reset --hard &&
+       git ls-files -s >actual &&
+       ! test -f old
 '
 
 test_expect_success 'Porcelain checkout -f should remove remnants too' '
-  read_tree_u_must_succeed --reset -u HEAD &&
-  git ls-files -s >expect &&
-  sha1=$(git rev-parse :new) &&
-  (
-       echo "100644 $sha1 1    old"
-       echo "100644 $sha1 3    old"
-  ) | git update-index --index-info &&
-  >old &&
-  git ls-files -s &&
-  git checkout -f &&
-  git ls-files -s >actual &&
-  ! test -f old
+       read_tree_u_must_succeed --reset -u HEAD &&
+       git ls-files -s >expect &&
+       sha1=$(git rev-parse :new) &&
+       (
+               echo "100644 $sha1 1    old"
+               echo "100644 $sha1 3    old"
+       ) | git update-index --index-info &&
+       >old &&
+       git ls-files -s &&
+       git checkout -f &&
+       git ls-files -s >actual &&
+       ! test -f old
 '
 
 test_expect_success 'Porcelain checkout -f HEAD should remove remnants too' '
-  read_tree_u_must_succeed --reset -u HEAD &&
-  git ls-files -s >expect &&
-  sha1=$(git rev-parse :new) &&
-  (
-       echo "100644 $sha1 1    old"
-       echo "100644 $sha1 3    old"
-  ) | git update-index --index-info &&
-  >old &&
-  git ls-files -s &&
-  git checkout -f HEAD &&
-  git ls-files -s >actual &&
-  ! test -f old
+       read_tree_u_must_succeed --reset -u HEAD &&
+       git ls-files -s >expect &&
+       sha1=$(git rev-parse :new) &&
+       (
+               echo "100644 $sha1 1    old"
+               echo "100644 $sha1 3    old"
+       ) | git update-index --index-info &&
+       >old &&
+       git ls-files -s &&
+       git checkout -f HEAD &&
+       git ls-files -s >actual &&
+       ! test -f old
 '
 
 test_done

diff --git a/unpack-trees.c b/unpack-trees.c
index 35cb05e92bed9fb273747adc4718458b9f500898..ad3e9a04fe8ccb9286b94480f4484b0569f70a57 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1763,14 +1763,23 @@ int twoway_merge(const struct cache_entry * const *src,
                newtree = NULL;
 
        if (current) {
-               if ((!oldtree && !newtree) || /* 4 and 5 */
-                   (!oldtree && newtree &&
-                    same(current, newtree)) || /* 6 and 7 */
-                   (oldtree && newtree &&
-                    same(oldtree, newtree)) || /* 14 and 15 */
-                   (oldtree && newtree &&
-                    !same(oldtree, newtree) && /* 18 and 19 */
-                    same(current, newtree))) {
+               if (current->ce_flags & CE_CONFLICTED) {
+                       if (same(oldtree, newtree) || o->reset) {
+                               if (!newtree)
+                                       return deleted_entry(current, current, o);
+                               else
+                                       return merged_entry(newtree, current, o);
+                       }
+                       return o->gently ? -1 : reject_merge(current, o);
+               }
+               else if ((!oldtree && !newtree) || /* 4 and 5 */
+                        (!oldtree && newtree &&
+                         same(current, newtree)) || /* 6 and 7 */
+                        (oldtree && newtree &&
+                         same(oldtree, newtree)) || /* 14 and 15 */
+                        (oldtree && newtree &&
+                         !same(oldtree, newtree) && /* 18 and 19 */
+                         same(current, newtree))) {
                        return keep_entry(current, o);
                }
                else if (oldtree && !newtree && same(current, oldtree)) {