[PATCH] read-tree: fix too strong index requirement #5ALT

This fixes too strong index requirement 3-way merge enforces in
one case: the same file is added in both branches.

In this case, the original code insisted that if the index file
has that path, it must match our branch and be up-to-date.
However in this particular case, it only has to match it, and
can be dirty.  We just need to make sure that we keep the
work-tree copy instead of checking out the merge result.

The resolution of such a path, however, cannot be left to
outside script, because we will not keep the original stage0
entries for unmerged paths when read-tree finishes, and at that
point, the knowledge of "if we resolve it to match the new file
added in both branches, the merge succeeds and the work tree
would not lose information, but we should _not_ update the work
tree from the resulting index file" is lost.  For this reason,
the now code needs to resolve this case (#5ALT) internally.

This affects some existing tests in the test suite, but all in
positive ways.  In t1000 (3-way test), this #5ALT case now gets
one stage0 entry, instead of an identical stage2 and stage3
entry pair, for such a path, and one test that checked for merge
failure (because the test assumed the "stricter-than-necessary"
behaviour) does not have to fail anymore.  In t1005 (emu23
test), two tests that involves a case where the work tree
already had a change introduced in the upstream (aka "merged
head"), the merge succeeds instead of failing.

Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/read-tree.c b/read-tree.c
index f2a8bb5..ecd40cc 100644 (file)
--- a/read-tree.c
+++ b/read-tree.c
@@ -102,7 +102,7 @@ static void reject_merge(struct cache_entry *ce)
        die("Entry '%s' would be overwritten by merge. Cannot merge.", ce->name);
 }
 
-static int merged_entry(struct cache_entry *merge, struct cache_entry *old, struct cache_entry **dst)
+static int merged_entry_internal(struct cache_entry *merge, struct cache_entry *old, struct cache_entry **dst, int allow_dirty)
 {
        merge->ce_flags |= htons(CE_UPDATE);
        if (old) {
@@ -115,7 +115,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old, stru
                 */
                if (same(old, merge)) {
                        *merge = *old;
-               } else {
+               } else if (!allow_dirty) {
                        verify_uptodate(old);
                }
        }
@@ -124,6 +124,16 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old, stru
        return 1;
 }
 
+static int merged_entry_allow_dirty(struct cache_entry *merge, struct cache_entry *old, struct cache_entry **dst)
+{
+       return merged_entry_internal(merge, old, dst, 1);
+}
+
+static int merged_entry(struct cache_entry *merge, struct cache_entry *old, struct cache_entry **dst)
+{
+       return merged_entry_internal(merge, old, dst, 0);
+}
+
 static int deleted_entry(struct cache_entry *ce, struct cache_entry *old, struct cache_entry **dst)
 {
        if (old)
@@ -140,6 +150,12 @@ static int threeway_merge(struct cache_entry *stages[4], struct cache_entry **ds
        struct cache_entry *merge;
        int count;
 
+       /* #5ALT */
+       if (!a && b && c && same(b, c)) {
+               if (old && !same(b, old))
+                       return -1;
+               return merged_entry_allow_dirty(b, old, dst);
+       }
        /*
         * If we have an entry in the index cache ("old"), then we want
         * to make sure that it matches any entries in stage 2 ("first

diff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh
index c435887..92833fe 100755 (executable)
--- a/t/t1000-read-tree-m-3way.sh
+++ b/t/t1000-read-tree-m-3way.sh
@@ -75,7 +75,8 @@ In addition:
 . ../lib-read-tree-m-3way.sh
 
 ################################################################
-# This is the "no trivial merge unless all three exists" table.
+# Trivial "majority when 3 stages exist" merge plus #5ALT trivial
+# merge.
 
 cat >expected <<\EOF
 100644 X 2     AA
@@ -88,8 +89,7 @@ cat >expected <<\EOF
 100644 X 3     DM
 100644 X 1     DN
 100644 X 3     DN
-100644 X 2     LL
-100644 X 3     LL
+100644 X 0     LL
 100644 X 1     MD
 100644 X 2     MD
 100644 X 1     MM
@@ -289,23 +289,24 @@ test_expect_failure \
      git-read-tree -m $tree_O $tree_A $tree_B"
 
 test_expect_success \
-    '5 - must match and be up-to-date in !O && A && B && A==B case.' \
+    '5 - must match in !O && A && B && A==B case.' \
     "rm -f .git/index LL &&
      cp .orig-A/LL LL &&
      git-update-cache --add LL &&
      git-read-tree -m $tree_O $tree_A $tree_B &&
      check_result"
 
-test_expect_failure \
-    '5 (fail) - must match and be up-to-date in !O && A && B && A==B case.' \
+test_expect_success \
+    '5 - must match in !O && A && B && A==B case.' \
     "rm -f .git/index LL &&
      cp .orig-A/LL LL &&
      git-update-cache --add LL &&
      echo extra >>LL &&
-     git-read-tree -m $tree_O $tree_A $tree_B"
+     git-read-tree -m $tree_O $tree_A $tree_B &&
+     check_result"
 
 test_expect_failure \
-    '5 (fail) - must match and be up-to-date in !O && A && B && A==B case.' \
+    '5 (fail) - must match A in !O && A && B && A==B case.' \
     "rm -f .git/index LL &&
      cp .orig-A/LL LL &&
      echo extra >>LL &&

diff --git a/t/t1005-read-tree-m-2way-emu23.sh b/t/t1005-read-tree-m-2way-emu23.sh
index 3345c9d..495642e 100644 (file)
--- a/t/t1005-read-tree-m-2way-emu23.sh
+++ b/t/t1005-read-tree-m-2way-emu23.sh
@@ -117,9 +117,8 @@ test_expect_success \
      check_cache_at yomin dirty'
 
 # "read-tree -m H I+H M" where !H && M && (I+H) == M, so this should
-# succeed (even the entry is clean), but without #5ALT this does not
-# work.
-: test_expect_success \
+# succeed (even the entry is clean), now thanks to #5ALT.
+test_expect_success \
     '6 - local addition already has the same.' \
     'rm -f .git/index &&
      git-update-cache --add frotz &&
@@ -129,8 +128,8 @@ test_expect_success \
      check_cache_at frotz clean'
 
 # Exactly the same pattern as above but with dirty cache.  This also
-# should succeed, but without #5ALT it does not.
-: test_expect_success \
+# should succeed, now thanks to #5ALT.
+test_expect_success \
     '7 - local addition already has the same.' \
     'rm -f .git/index &&
      echo frotz >frotz &&