Don't update unchanged merge entries

In commit 34110cd4e394e3f92c01a4709689b384c34645d8 ("Make 'unpack_trees()'
have a separate source and destination index") I introduced a really
stupid bug in that it would always add merged entries with the CE_UPDATE
flag set. That caused us to always re-write the file, even when it was
already up-to-date in the source index.

Not only is that really stupid from a performance angle, but more
importantly it's actively wrong: if we have dirty state in the tree when
we merge, overwriting it with the result of the merge will incorrectly
overwrite that dirty state.

This trivially fixes the problem - simply don't set the CE_UPDATE flag
when the merge result matches the old state.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
index d609a551ae3b8c75714f6cf4e9cefe4f6af61c5b..9d1371c2c61fd9fb57e3719590d601c3b9ad21b2 100755 (executable)
--- a/t/t1004-read-tree-m-u-wf.sh
+++ b/t/t1004-read-tree-m-u-wf.sh
@@ -116,4 +116,45 @@ test_expect_success 'three-way not complaining on an untracked file' '
        git read-tree -m -u --exclude-per-directory=.gitignore branch-point master side
 '
 
        git read-tree -m -u --exclude-per-directory=.gitignore branch-point master side
 '
 

+test_expect_success '3-way not overwriting local changes (setup)' '
+
+       git reset --hard &&
+       git checkout -b side-a branch-point &&
+       echo >>file1 "new line to be kept in the merge result" &&
+       git commit -a -m "side-a changes file1" &&
+       git checkout -b side-b branch-point &&
+       echo >>file2 "new line to be kept in the merge result" &&
+       git commit -a -m "side-b changes file2" &&
+       git checkout side-a
+
+'
+
+test_expect_success '3-way not overwriting local changes (our side)' '
+
+       # At this point, file1 from side-a should be kept as side-b
+       # did not touch it.
+
+       git reset --hard &&
+
+       echo >>file1 "local changes" &&
+       git read-tree -m -u branch-point side-a side-b &&
+       grep "new line to be kept" file1 &&
+       grep "local changes" file1
+
+'
+
+test_expect_success '3-way not overwriting local changes (their side)' '
+
+       # At this point, file2 from side-b should be taken as side-a
+       # did not touch it.
+
+       git reset --hard &&
+
+       echo >>file2 "local changes" &&
+       test_must_fail git read-tree -m -u branch-point side-a side-b &&
+       ! grep "new line to be kept" file2 &&
+       grep "local changes" file2
+
+'
+

 test_done
 test_done

diff --git a/unpack-trees.c b/unpack-trees.c
index 93019abdc1192fbb4649573ad507205b26652109..4b359e0832cc07af7c10322e796ff1ed245c2294 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -595,16 +595,19 @@ static int verify_absent(struct cache_entry *ce, const char *action,
 static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
                struct unpack_trees_options *o)
 {
 static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
                struct unpack_trees_options *o)
 {

+       int update = CE_UPDATE;
+

        if (old) {
                /*
                 * See if we can re-use the old CE directly?
                 * That way we get the uptodate stat info.
                 *
        if (old) {
                /*
                 * See if we can re-use the old CE directly?
                 * That way we get the uptodate stat info.
                 *

-                * This also removes the UPDATE flag on
-                * a match.
+                * This also removes the UPDATE flag on a match; otherwise
+                * we will end up overwriting local changes in the work tree.

                 */
                if (same(old, merge)) {
                        copy_cache_entry(merge, old);
                 */
                if (same(old, merge)) {
                        copy_cache_entry(merge, old);

+                       update = 0;

                } else {
                        if (verify_uptodate(old, o))
                                return -1;
                } else {
                        if (verify_uptodate(old, o))
                                return -1;


@@ -617,7 +620,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
                invalidate_ce_path(merge, o);
        }
 
                invalidate_ce_path(merge, o);
        }
 

-       add_entry(o, merge, CE_UPDATE, CE_STAGEMASK);
+       add_entry(o, merge, update, CE_STAGEMASK);

        return 1;
 }
 
        return 1;
 }