Skip to content

Commit 7077c38

Browse files
rscharfegitster
authored andcommitted
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. d1f2d7e (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>
1 parent 9a2fb14 commit 7077c38

File tree

2 files changed

+31
-4
lines changed

2 files changed

+31
-4
lines changed

diff-lib.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,12 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
226226
continue;
227227
}
228228

229-
if (ce_uptodate(ce) || ce_skip_worktree(ce))
229+
if (ce_uptodate(ce) || ce_skip_worktree(ce)) {
230+
if (revs->diffopt.flags.find_copies_harder)
231+
diff_same(&revs->diffopt, ce->ce_mode,
232+
&ce->oid, ce->name);
230233
continue;
234+
}
231235

232236
/*
233237
* 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)
272276
if (!changed && !dirty_submodule) {
273277
ce_mark_uptodate(ce);
274278
mark_fsmonitor_valid(istate, ce);
275-
if (!revs->diffopt.flags.find_copies_harder)
276-
continue;
279+
if (revs->diffopt.flags.find_copies_harder)
280+
diff_same(&revs->diffopt, newmode,
281+
&ce->oid, ce->name);
282+
continue;
277283
}
278284
oldmode = ce->ce_mode;
279285
old_oid = &ce->oid;

t/t4007-rename-3.sh

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,28 @@ test_expect_success 'copy, limited to a subtree' '
5757
'
5858

5959
test_expect_success 'tweak work tree' '
60-
rm -f path0/COPYING &&
60+
rm -f path0/COPYING
61+
'
62+
63+
cat >expected <<EOF
64+
:100644 100644 $blob $blob C100 path1/COPYING path0/COPYING
65+
EOF
66+
67+
# The cache has path0/COPYING and path1/COPYING, the working tree only
68+
# path1/COPYING. This is a deletion -- we don't treat deduplication
69+
# specially. In reverse it should be detected as a copy, though.
70+
test_expect_success 'copy detection, files to index' '
71+
git diff-files -C --find-copies-harder -R >current &&
72+
compare_diff_raw current expected
73+
'
74+
75+
test_expect_success 'copy detection, files to preloaded index' '
76+
GIT_TEST_PRELOAD_INDEX=1 \
77+
git diff-files -C --find-copies-harder -R >current &&
78+
compare_diff_raw current expected
79+
'
80+
81+
test_expect_success 'tweak index' '
6182
git update-index --remove path0/COPYING
6283
'
6384
# In the tree, there is only path0/COPYING. In the cache, path0 does

0 commit comments

Comments
 (0)