diff-files: fix copy detection

Copy detection cannot work when comparing the index to the working tree
because Git ignores files that it is not explicitly told to track.  It
should work in the other direction, though, i.e. for a reverse diff of
the deletion of a copy from the index.

d1f2d7e8ca (Make run_diff_index() use unpack_trees(), not read_tree(),
2008-01-19) broke it with a seemingly stray change to run_diff_files().

We didn't notice because there's no test for that.  But even if we had
one, it might have gone unnoticed because the breakage only happens
with index preloading, which requires at least 1000 entries (more than
most test repos have) and is racy because it runs in parallel with the
actual command.

Fix copy detection by queuing up-to-date and skip-worktree entries using
diff_same().

While at it, use diff_same() also for queuing unchanged files not
flagged as up-to-date, i.e. clean submodules and entries where
preloading was not done at all or not quickly enough.  It uses less
memory than diff_change() and doesn't unnecessarily set the diff flag
has_changes.

Add two tests to cover running both without and with preloading.  The
first one passes reliably with the original code.  The second one
enables preloading and thus is racy.  It has a good chance to pass even
without the fix, but fails within seconds when running the test script
with --stress.  With the fix it runs fine for several minutes, until
my patience runs out.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff-lib.c b/diff-lib.c
index 8e624f38c6d6f34bb499bbd8b6a96fb6c0ca09da..5307390ff3db7bd90cc01913cefb76eb5e1a5d9d 100644 (file)
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -226,8 +226,12 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
                                continue;
                }
 
-               if (ce_uptodate(ce) || ce_skip_worktree(ce))
+               if (ce_uptodate(ce) || ce_skip_worktree(ce)) {
+                       if (revs->diffopt.flags.find_copies_harder)
+                               diff_same(&revs->diffopt, ce->ce_mode,
+                                         &ce->oid, ce->name);
                        continue;
+               }
 
                /*
                 * When CE_VALID is set (via "update-index --assume-unchanged"
@@ -272,8 +276,10 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
                if (!changed && !dirty_submodule) {
                        ce_mark_uptodate(ce);
                        mark_fsmonitor_valid(istate, ce);
-                       if (!revs->diffopt.flags.find_copies_harder)
-                               continue;
+                       if (revs->diffopt.flags.find_copies_harder)
+                               diff_same(&revs->diffopt, newmode,
+                                         &ce->oid, ce->name);
+                       continue;
                }
                oldmode = ce->ce_mode;
                old_oid = &ce->oid;

diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
index e8faf0dd2ef1c5e0dc3aebe19276843d62a658e5..34f7d276d116e070d0d972384e30a55de904b54b 100755 (executable)
--- a/t/t4007-rename-3.sh
+++ b/t/t4007-rename-3.sh
@@ -57,7 +57,28 @@ test_expect_success 'copy, limited to a subtree' '
 '
 
 test_expect_success 'tweak work tree' '
-       rm -f path0/COPYING &&
+       rm -f path0/COPYING
+'
+
+cat >expected <<EOF
+:100644 100644 $blob $blob C100        path1/COPYING   path0/COPYING
+EOF
+
+# The cache has path0/COPYING and path1/COPYING, the working tree only
+# path1/COPYING.  This is a deletion -- we don't treat deduplication
+# specially.  In reverse it should be detected as a copy, though.
+test_expect_success 'copy detection, files to index' '
+       git diff-files -C --find-copies-harder -R >current &&
+       compare_diff_raw current expected
+'
+
+test_expect_success 'copy detection, files to preloaded index' '
+       GIT_TEST_PRELOAD_INDEX=1 \
+       git diff-files -C --find-copies-harder -R >current &&
+       compare_diff_raw current expected
+'
+
+test_expect_success 'tweak index' '
        git update-index --remove path0/COPYING
 '
 # In the tree, there is only path0/COPYING.  In the cache, path0 does