Skip to content

Commit f4c7994

Browse files
committed
replay: side-step bug when directory renames are turned off
There is a bug in the way renames are cached that rears its head when directory renames are turned off. This patch comes with a demonstration of this bug which will fail with the following message unless the rename cache is explicitly reset in `merge_check_renames_reusable()` when `merge.directoryRenames=false`: merge-ort.c:2909: process_renames: Assertion `newinfo && !newinfo->merged.clean' failed. Aborted It is quite a curious bug: the same test case will succeed, without any assertion, if instead run with `merge.directoryRenames=true`. Further, the assertion does not manifest while replaying the first commit, it manifests while replaying the _second_ commit of the commit range. But it does _not_ manifest when the second commit is replayed individually. This would indicate that there is an incomplete rename cache left-over from the first replayed commit which is being reused for the second commit, and if directory rename detection is enabled, the missing paths are somehow regenerated. This was a fun, intense hunt. The solution to the riddle is that indeed, there was a stale cached rename information. The fix is easy: explicitly record the diff pair as a potential rename. Helped-by; Elijah Newren <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
1 parent e928c11 commit f4c7994

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

merge-ort.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3292,6 +3292,13 @@ static int collect_renames(struct merge_options *opt,
32923292
continue;
32933293
}
32943294

3295+
if (opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_NONE &&
3296+
p->status == 'R') {
3297+
possibly_cache_new_pair(renames, p, side_index, NULL);
3298+
pool_diff_free_filepair(&opt->priv->pool, p);
3299+
continue;
3300+
}
3301+
32953302
new_path = check_for_directory_rename(opt, p->two->path,
32963303
side_index,
32973304
dir_renames_for_side,

t/t3650-replay-basics.sh

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,26 @@ test_expect_success 'using replay on bare repo to rebase multiple divergent bran
195195
done
196196
'
197197

198+
test_expect_success 'merge.directoryRenames=false' '
199+
# create a test case that stress-tests the rename caching
200+
git switch -c rename-onto &&
201+
202+
mkdir -p to-rename &&
203+
test_commit to-rename/move &&
204+
205+
mkdir -p renamed-directory &&
206+
git mv to-rename/move* renamed-directory/ &&
207+
test_tick &&
208+
git commit -m renamed-directory &&
209+
210+
git switch -c rename-from HEAD^ &&
211+
test_commit to-rename/add-a-file &&
212+
echo modified >to-rename/add-a-file.t &&
213+
test_tick &&
214+
git commit -m modified to-rename/add-a-file.t &&
215+
216+
git -c merge.directoryRenames=false replay \
217+
--onto rename-onto rename-onto..rename-from
218+
'
219+
198220
test_done

0 commit comments

Comments
 (0)