Skip to content

Commit 902c521

Browse files
newrengitster
newren
authored and
gitster
committed
t6423: more involved directory rename test
Add a new testcase modelled on a real world repository example that served multiple purposes: * it uncovered a bug in the current directory rename detection implementation. * it is a good test of needing to do directory rename detection for a series of commits instead of just one (and uses rebase instead of just merge like all the other tests in this testfile). * it is an excellent stress test for some of the optimizations in my new merge-ort engine I can expand on the final item later when I have submitted more of merge-ort, but the bug is the main immediate concern. It arises as follows: * dir/subdir/ has several files * almost all files in dir/subdir/ are renamed to folder/subdir/ * one of the files in dir/subdir/ is renamed to folder/subdir/newsubdir/ * If the other side of history (that doesn't do the renames) adds a new file to dir/subdir/, where should it be placed after the merge? The most obvious two choices are: (1) leave the new file in dir/subdir/, don't make it follow the rename, and (2) move the new file to folder/subdir/, following the rename of most the files. However, there's a possible third choice here: (3) move the new file to folder/subdir/newsubdir/. The choice reinforce the fact that merge.directoryRenames=conflict is a good default, but when the merge machinery needs to stick it somewhere and notify the user of the possibility that they might want to place it elsewhere. Surprisingly, the current code would always choose (3), while the real world repository was clearly expecting (2) -- move the file along with where the herd of files was going, not with the special exception. The problem here is that for the majority of the file renames, dir/subdir/ -> folder/subdir/ is actually represented as dir/ -> folder/ This directory rename would have a big weight associated with it since most the files followed that rename. However, we always consult the most immediate directory first, and there is only one rename rule for it: dir/subdir/ -> folder/subdir/newsubdir/ Since this rule is the only one for mapping from dir/subdir/, it automatically wins and that directory rename was followed instead of the desired dir/subdir/ -> folder/subdir/. Unfortunately, the fix is a bit involved so for now just add the testcase documenting the issue. Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b9718d0 commit 902c521

File tree

1 file changed

+195
-0
lines changed

1 file changed

+195
-0
lines changed

‎t/t6423-merge-rename-directories.sh

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4227,6 +4227,201 @@ test_expect_success '12e: Rename/merge subdir into the root, variant 2' '
42274227
)
42284228
'
42294229

4230+
# Testcase 12f, Rebase of patches with big directory rename
4231+
# Commit O:
4232+
# dir/subdir/{a,b,c,d,e_O,Makefile_TOP_O}
4233+
# dir/subdir/tweaked/{f,g,h,Makefile_SUB_O}
4234+
# dir/unchanged/<LOTS OF FILES>
4235+
# Commit A:
4236+
# (Remove f & g, move e into newsubdir, rename dir/->folder/, modify files)
4237+
# folder/subdir/{a,b,c,d,Makefile_TOP_A}
4238+
# folder/subdir/newsubdir/e_A
4239+
# folder/subdir/tweaked/{h,Makefile_SUB_A}
4240+
# folder/unchanged/<LOTS OF FILES>
4241+
# Commit B1:
4242+
# (add newfile.{c,py}, modify underscored files)
4243+
# dir/{a,b,c,d,e_B1,Makefile_TOP_B1,newfile.c}
4244+
# dir/tweaked/{f,g,h,Makefile_SUB_B1,newfile.py}
4245+
# dir/unchanged/<LOTS OF FILES>
4246+
# Commit B2:
4247+
# (Modify e further, add newfile.rs)
4248+
# dir/{a,b,c,d,e_B2,Makefile_TOP_B1,newfile.c,newfile.rs}
4249+
# dir/tweaked/{f,g,h,Makefile_SUB_B1,newfile.py}
4250+
# dir/unchanged/<LOTS OF FILES>
4251+
# Expected:
4252+
# B1-picked:
4253+
# folder/subdir/{a,b,c,d,Makefile_TOP_Merge1,newfile.c}
4254+
# folder/subdir/newsubdir/e_Merge1
4255+
# folder/subdir/tweaked/{h,Makefile_SUB_Merge1,newfile.py}
4256+
# folder/unchanged/<LOTS OF FILES>
4257+
# B2-picked:
4258+
# folder/subdir/{a,b,c,d,Makefile_TOP_Merge1,newfile.c,newfile.rs}
4259+
# folder/subdir/newsubdir/e_Merge2
4260+
# folder/subdir/tweaked/{h,Makefile_SUB_Merge1,newfile.py}
4261+
# folder/unchanged/<LOTS OF FILES>
4262+
#
4263+
# Notes: This testcase happens to exercise lots of the
4264+
# optimization-specific codepaths in merge-ort, and also
4265+
# demonstrated a failing of the directory rename detection algorithm
4266+
# in merge-recursive; newfile.c should not get pushed into
4267+
# folder/subdir/newsubdir/, yet merge-recursive put it there because
4268+
# the rename of dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d}
4269+
# looks like
4270+
# dir/ -> folder/,
4271+
# whereas the rename of dir/subdir/e -> folder/subdir/newsubdir/e
4272+
# looks like
4273+
# dir/subdir/ -> folder/subdir/newsubdir/
4274+
# and if we note that newfile.c is found in dir/subdir/, we might
4275+
# overlook the dir/ -> folder/ rule that has more weight.
4276+
4277+
test_setup_12f () {
4278+
test_create_repo 12f &&
4279+
(
4280+
cd 12f &&
4281+
4282+
mkdir -p dir/unchanged &&
4283+
mkdir -p dir/subdir/tweaked &&
4284+
echo a >dir/subdir/a &&
4285+
echo b >dir/subdir/b &&
4286+
echo c >dir/subdir/c &&
4287+
echo d >dir/subdir/d &&
4288+
test_seq 1 10 >dir/subdir/e &&
4289+
test_seq 10 20 >dir/subdir/Makefile &&
4290+
echo f >dir/subdir/tweaked/f &&
4291+
echo g >dir/subdir/tweaked/g &&
4292+
echo h >dir/subdir/tweaked/h &&
4293+
test_seq 20 30 >dir/subdir/tweaked/Makefile &&
4294+
for i in `test_seq 1 88`; do
4295+
echo content $i >dir/unchanged/file_$i
4296+
done &&
4297+
git add . &&
4298+
git commit -m "O" &&
4299+
4300+
git branch O &&
4301+
git branch A &&
4302+
git branch B &&
4303+
4304+
git switch A &&
4305+
git rm dir/subdir/tweaked/f dir/subdir/tweaked/g &&
4306+
test_seq 2 10 >dir/subdir/e &&
4307+
test_seq 11 20 >dir/subdir/Makefile &&
4308+
test_seq 21 30 >dir/subdir/tweaked/Makefile &&
4309+
mkdir dir/subdir/newsubdir &&
4310+
git mv dir/subdir/e dir/subdir/newsubdir/ &&
4311+
git mv dir folder &&
4312+
git add . &&
4313+
git commit -m "A" &&
4314+
4315+
git switch B &&
4316+
mkdir dir/subdir/newsubdir/ &&
4317+
echo c code >dir/subdir/newfile.c &&
4318+
echo python code >dir/subdir/newsubdir/newfile.py &&
4319+
test_seq 1 11 >dir/subdir/e &&
4320+
test_seq 10 21 >dir/subdir/Makefile &&
4321+
test_seq 20 31 >dir/subdir/tweaked/Makefile &&
4322+
git add . &&
4323+
git commit -m "B1" &&
4324+
4325+
echo rust code >dir/subdir/newfile.rs &&
4326+
test_seq 1 12 >dir/subdir/e &&
4327+
git add . &&
4328+
git commit -m "B2"
4329+
)
4330+
}
4331+
4332+
test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' '
4333+
test_setup_12f &&
4334+
(
4335+
cd 12f &&
4336+
4337+
git checkout A^0 &&
4338+
git branch Bmod B &&
4339+
4340+
git -c merge.directoryRenames=true rebase A Bmod &&
4341+
4342+
echo Checking the pick of B1... &&
4343+
4344+
test_must_fail git rev-parse Bmod~1:dir &&
4345+
4346+
git ls-tree -r Bmod~1 >out &&
4347+
test_line_count = 98 out &&
4348+
4349+
git diff --name-status A Bmod~1 >actual &&
4350+
q_to_tab >expect <<-\EOF &&
4351+
MQfolder/subdir/Makefile
4352+
AQfolder/subdir/newfile.c
4353+
MQfolder/subdir/newsubdir/e
4354+
AQfolder/subdir/newsubdir/newfile.py
4355+
MQfolder/subdir/tweaked/Makefile
4356+
EOF
4357+
test_cmp expect actual &&
4358+
4359+
# Three-way merged files
4360+
test_seq 2 11 >e_Merge1 &&
4361+
test_seq 11 21 >Makefile_TOP &&
4362+
test_seq 21 31 >Makefile_SUB &&
4363+
git hash-object >expect \
4364+
e_Merge1 \
4365+
Makefile_TOP \
4366+
Makefile_SUB &&
4367+
git rev-parse >actual \
4368+
Bmod~1:folder/subdir/newsubdir/e \
4369+
Bmod~1:folder/subdir/Makefile \
4370+
Bmod~1:folder/subdir/tweaked/Makefile &&
4371+
test_cmp expect actual &&
4372+
4373+
# New files showed up at the right location with right contents
4374+
git rev-parse >expect \
4375+
B~1:dir/subdir/newfile.c \
4376+
B~1:dir/subdir/newsubdir/newfile.py &&
4377+
git rev-parse >actual \
4378+
Bmod~1:folder/subdir/newfile.c \
4379+
Bmod~1:folder/subdir/newsubdir/newfile.py &&
4380+
test_cmp expect actual &&
4381+
4382+
# Removed files
4383+
test_path_is_missing folder/subdir/tweaked/f &&
4384+
test_path_is_missing folder/subdir/tweaked/g &&
4385+
4386+
# Unchanged files or directories
4387+
git rev-parse >actual \
4388+
Bmod~1:folder/subdir/a \
4389+
Bmod~1:folder/subdir/b \
4390+
Bmod~1:folder/subdir/c \
4391+
Bmod~1:folder/subdir/d \
4392+
Bmod~1:folder/unchanged \
4393+
Bmod~1:folder/subdir/tweaked/h &&
4394+
git rev-parse >expect \
4395+
O:dir/subdir/a \
4396+
O:dir/subdir/b \
4397+
O:dir/subdir/c \
4398+
O:dir/subdir/d \
4399+
O:dir/unchanged \
4400+
O:dir/subdir/tweaked/h &&
4401+
test_cmp expect actual &&
4402+
4403+
echo Checking the pick of B2... &&
4404+
4405+
test_must_fail git rev-parse Bmod:dir &&
4406+
4407+
git ls-tree -r Bmod >out &&
4408+
test_line_count = 99 out &&
4409+
4410+
git diff --name-status Bmod~1 Bmod >actual &&
4411+
q_to_tab >expect <<-\EOF &&
4412+
AQfolder/subdir/newfile.rs
4413+
MQfolder/subdir/newsubdir/e
4414+
EOF
4415+
test_cmp expect actual &&
4416+
4417+
# Three-way merged file
4418+
test_seq 2 12 >e_Merge2 &&
4419+
git hash-object e_Merge2 >expect &&
4420+
git rev-parse Bmod:folder/subdir/newsubdir/e >actual &&
4421+
test_cmp expect actual
4422+
)
4423+
'
4424+
42304425
###########################################################################
42314426
# SECTION 13: Checking informational and conflict messages
42324427
#

0 commit comments

Comments
 (0)