Skip to content

Commit c64432a

Browse files
newrengitster
newren
authored and
gitster
committed
t6423: more involved rules for renaming directories into each other
Testcases 12b and 12c were both slightly weird; they were marked as having a weird resolution, but with the note that even straightforward simple rules can give weird results when the input is bizarre. However, during optimization work for merge-ort, I discovered a significant speedup that is possible if we add one more fairly straightforward rule: we don't bother doing directory rename detection if there are no new files added to the directory on the other side of the history to be affected by the directory rename. This seems like an obvious and straightforward rule, but there was one funny corner case where directory rename detection could affect only existing files: the funny corner case where two directories are renamed into each other on opposite sides of history. In other words, it only results in a different output for testcases 12b and 12c. Since we already thought testcases 12b and 12c were weird anyway, and because the optimization often has a significant effect on common cases (but is entirely prevented if we can't change how 12b and 12c function), let's add the additional rule and tweak how 12b and 12c work. Split both testcases into two (one where we add no new files, and one where the side that doesn't rename a given directory will add files to it), and mark them with the new expectation. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 8536821 commit c64432a

File tree

2 files changed

+230
-26
lines changed

2 files changed

+230
-26
lines changed

‎Documentation/technical/directory-rename-detection.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ additional rules:
7373
b) Only apply implicit directory renames to directories if the other side
7474
of history is the one doing the renaming.
7575

76+
c) Do not perform directory rename detection for directories which had no
77+
new paths added to them.
78+
7679
Limitations -- support in different commands
7780
--------------------------------------------
7881

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

Lines changed: 227 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4049,31 +4049,124 @@ test_expect_success '12a: Moving one directory hierarchy into another' '
40494049
)
40504050
'
40514051

4052-
# Testcase 12b, Moving two directory hierarchies into each other
4052+
# Testcase 12b1, Moving two directory hierarchies into each other
40534053
# (Related to testcases 1c and 12c)
40544054
# Commit O: node1/{leaf1, leaf2}, node2/{leaf3, leaf4}
40554055
# Commit A: node1/{leaf1, leaf2, node2/{leaf3, leaf4}}
40564056
# Commit B: node2/{leaf3, leaf4, node1/{leaf1, leaf2}}
4057-
# Expected: node1/node2/node1/{leaf1, leaf2},
4057+
# Expected: node1/node2/{leaf3, leaf4}
4058+
# node2/node1/{leaf1, leaf2}
4059+
# NOTE: If there were new files added to the old node1/ or node2/ directories,
4060+
# then we would need to detect renames for those directories and would
4061+
# find that:
4062+
# commit A renames node2/ -> node1/node2/
4063+
# commit B renames node1/ -> node2/node1/
4064+
# Applying those directory renames to the initial result (making all
4065+
# four paths experience a transitive renaming), yields
4066+
# node1/node2/node1/{leaf1, leaf2}
40584067
# node2/node1/node2/{leaf3, leaf4}
4068+
# as the result. It may be really weird to have two directories
4069+
# rename each other, but simple rules give weird results when given
4070+
# weird inputs. HOWEVER, the "If" at the beginning of those NOTE was
4071+
# false; there were no new files added and thus there is no directory
4072+
# rename detection to perform. As such, we just have simple renames
4073+
# and the expected answer is:
4074+
# node1/node2/{leaf3, leaf4}
4075+
# node2/node1/{leaf1, leaf2}
4076+
4077+
test_setup_12b1 () {
4078+
test_create_repo 12b1 &&
4079+
(
4080+
cd 12b1 &&
4081+
4082+
mkdir -p node1 node2 &&
4083+
echo leaf1 >node1/leaf1 &&
4084+
echo leaf2 >node1/leaf2 &&
4085+
echo leaf3 >node2/leaf3 &&
4086+
echo leaf4 >node2/leaf4 &&
4087+
git add node1 node2 &&
4088+
test_tick &&
4089+
git commit -m "O" &&
4090+
4091+
git branch O &&
4092+
git branch A &&
4093+
git branch B &&
4094+
4095+
git checkout A &&
4096+
git mv node2/ node1/ &&
4097+
test_tick &&
4098+
git commit -m "A" &&
4099+
4100+
git checkout B &&
4101+
git mv node1/ node2/ &&
4102+
test_tick &&
4103+
git commit -m "B"
4104+
)
4105+
}
4106+
4107+
test_expect_failure '12b1: Moving two directory hierarchies into each other' '
4108+
test_setup_12b1 &&
4109+
(
4110+
cd 12b1 &&
4111+
4112+
git checkout A^0 &&
4113+
4114+
git -c merge.directoryRenames=true merge -s recursive B^0 &&
4115+
4116+
git ls-files -s >out &&
4117+
test_line_count = 4 out &&
4118+
4119+
git rev-parse >actual \
4120+
HEAD:node2/node1/leaf1 \
4121+
HEAD:node2/node1/leaf2 \
4122+
HEAD:node1/node2/leaf3 \
4123+
HEAD:node1/node2/leaf4 &&
4124+
git rev-parse >expect \
4125+
O:node1/leaf1 \
4126+
O:node1/leaf2 \
4127+
O:node2/leaf3 \
4128+
O:node2/leaf4 &&
4129+
test_cmp expect actual
4130+
)
4131+
'
4132+
4133+
# Testcase 12b2, Moving two directory hierarchies into each other
4134+
# (Related to testcases 1c and 12c)
4135+
# Commit O: node1/{leaf1, leaf2}, node2/{leaf3, leaf4}
4136+
# Commit A: node1/{leaf1, leaf2, leaf5, node2/{leaf3, leaf4}}
4137+
# Commit B: node2/{leaf3, leaf4, leaf6, node1/{leaf1, leaf2}}
4138+
# Expected: node1/node2/{node1/{leaf1, leaf2}, leaf6}
4139+
# node2/node1/{node2/{leaf3, leaf4}, leaf5}
40594140
# NOTE: Without directory renames, we would expect
4060-
# node2/node1/{leaf1, leaf2},
4061-
# node1/node2/{leaf3, leaf4}
4141+
# A: node2/leaf3 -> node1/node2/leaf3
4142+
# A: node2/leaf1 -> node1/node2/leaf4
4143+
# A: Adds node1/leaf5
4144+
# B: node1/leaf1 -> node2/node1/leaf1
4145+
# B: node1/leaf2 -> node2/node1/leaf2
4146+
# B: Adds node2/leaf6
40624147
# with directory rename detection, we note that
40634148
# commit A renames node2/ -> node1/node2/
40644149
# commit B renames node1/ -> node2/node1/
4065-
# therefore, applying those directory renames to the initial result
4066-
# (making all four paths experience a transitive renaming), yields
4067-
# the expected result.
4150+
# therefore, applying A's directory rename to the paths added in B gives:
4151+
# B: node1/leaf1 -> node1/node2/node1/leaf1
4152+
# B: node1/leaf2 -> node1/node2/node1/leaf2
4153+
# B: Adds node1/node2/leaf6
4154+
# and applying B's directory rename to the paths added in A gives:
4155+
# A: node2/leaf3 -> node2/node1/node2/leaf3
4156+
# A: node2/leaf1 -> node2/node1/node2/leaf4
4157+
# A: Adds node2/node1/leaf5
4158+
# resulting in the expected
4159+
# node1/node2/{node1/{leaf1, leaf2}, leaf6}
4160+
# node2/node1/{node2/{leaf3, leaf4}, leaf5}
40684161
#
40694162
# You may ask, is it weird to have two directories rename each other?
40704163
# To which, I can do no more than shrug my shoulders and say that
40714164
# even simple rules give weird results when given weird inputs.
40724165

4073-
test_setup_12b () {
4074-
test_create_repo 12b &&
4166+
test_setup_12b2 () {
4167+
test_create_repo 12b2 &&
40754168
(
4076-
cd 12b &&
4169+
cd 12b2 &&
40774170

40784171
mkdir -p node1 node2 &&
40794172
echo leaf1 >node1/leaf1 &&
@@ -4090,57 +4183,65 @@ test_setup_12b () {
40904183

40914184
git checkout A &&
40924185
git mv node2/ node1/ &&
4186+
echo leaf5 >node1/leaf5 &&
4187+
git add node1/leaf5 &&
40934188
test_tick &&
40944189
git commit -m "A" &&
40954190

40964191
git checkout B &&
40974192
git mv node1/ node2/ &&
4193+
echo leaf6 >node2/leaf6 &&
4194+
git add node2/leaf6 &&
40984195
test_tick &&
40994196
git commit -m "B"
41004197
)
41014198
}
41024199

4103-
test_expect_success '12b: Moving two directory hierarchies into each other' '
4104-
test_setup_12b &&
4200+
test_expect_success '12b2: Moving two directory hierarchies into each other' '
4201+
test_setup_12b2 &&
41054202
(
4106-
cd 12b &&
4203+
cd 12b2 &&
41074204
41084205
git checkout A^0 &&
41094206
41104207
git -c merge.directoryRenames=true merge -s recursive B^0 &&
41114208
41124209
git ls-files -s >out &&
4113-
test_line_count = 4 out &&
4210+
test_line_count = 6 out &&
41144211
41154212
git rev-parse >actual \
41164213
HEAD:node1/node2/node1/leaf1 \
41174214
HEAD:node1/node2/node1/leaf2 \
41184215
HEAD:node2/node1/node2/leaf3 \
4119-
HEAD:node2/node1/node2/leaf4 &&
4216+
HEAD:node2/node1/node2/leaf4 \
4217+
HEAD:node2/node1/leaf5 \
4218+
HEAD:node1/node2/leaf6 &&
41204219
git rev-parse >expect \
41214220
O:node1/leaf1 \
41224221
O:node1/leaf2 \
41234222
O:node2/leaf3 \
4124-
O:node2/leaf4 &&
4223+
O:node2/leaf4 \
4224+
A:node1/leaf5 \
4225+
B:node2/leaf6 &&
41254226
test_cmp expect actual
41264227
)
41274228
'
41284229

4129-
# Testcase 12c, Moving two directory hierarchies into each other w/ content merge
4230+
# Testcase 12c1, Moving two directory hierarchies into each other w/ content merge
41304231
# (Related to testcase 12b)
41314232
# Commit O: node1/{ leaf1_1, leaf2_1}, node2/{leaf3_1, leaf4_1}
41324233
# Commit A: node1/{ leaf1_2, leaf2_2, node2/{leaf3_2, leaf4_2}}
41334234
# Commit B: node2/{node1/{leaf1_3, leaf2_3}, leaf3_3, leaf4_3}
41344235
# Expected: Content merge conflicts for each of:
41354236
# node1/node2/node1/{leaf1, leaf2},
41364237
# node2/node1/node2/{leaf3, leaf4}
4137-
# NOTE: This is *exactly* like 12c, except that every path is modified on
4238+
# NOTE: This is *exactly* like 12b1, except that every path is modified on
41384239
# each side of the merge.
41394240

4140-
test_setup_12c () {
4141-
test_create_repo 12c &&
4241+
test_setup_12c1 () {
4242+
test_create_repo 12c1 &&
41424243
(
4143-
cd 12c &&
4244+
cd 12c1 &&
41444245

41454246
mkdir -p node1 node2 &&
41464247
printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf1\n" >node1/leaf1 &&
@@ -4171,10 +4272,10 @@ test_setup_12c () {
41714272
)
41724273
}
41734274

4174-
test_expect_success '12c: Moving one directory hierarchy into another w/ content merge' '
4175-
test_setup_12c &&
4275+
test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' '
4276+
test_setup_12c1 &&
41764277
(
4177-
cd 12c &&
4278+
cd 12c1 &&
41784279
41794280
git checkout A^0 &&
41804281
@@ -4183,6 +4284,102 @@ test_expect_success '12c: Moving one directory hierarchy into another w/ content
41834284
git ls-files -u >out &&
41844285
test_line_count = 12 out &&
41854286
4287+
git rev-parse >actual \
4288+
:1:node2/node1/leaf1 \
4289+
:1:node2/node1/leaf2 \
4290+
:1:node1/node2/leaf3 \
4291+
:1:node1/node2/leaf4 \
4292+
:2:node2/node1/leaf1 \
4293+
:2:node2/node1/leaf2 \
4294+
:2:node1/node2/leaf3 \
4295+
:2:node1/node2/leaf4 \
4296+
:3:node2/node1/leaf1 \
4297+
:3:node2/node1/leaf2 \
4298+
:3:node1/node2/leaf3 \
4299+
:3:node1/node2/leaf4 &&
4300+
git rev-parse >expect \
4301+
O:node1/leaf1 \
4302+
O:node1/leaf2 \
4303+
O:node2/leaf3 \
4304+
O:node2/leaf4 \
4305+
A:node1/leaf1 \
4306+
A:node1/leaf2 \
4307+
A:node1/node2/leaf3 \
4308+
A:node1/node2/leaf4 \
4309+
B:node2/node1/leaf1 \
4310+
B:node2/node1/leaf2 \
4311+
B:node2/leaf3 \
4312+
B:node2/leaf4 &&
4313+
test_cmp expect actual
4314+
)
4315+
'
4316+
4317+
# Testcase 12c2, Moving two directory hierarchies into each other w/ content merge
4318+
# (Related to testcase 12b)
4319+
# Commit O: node1/{ leaf1_1, leaf2_1}, node2/{leaf3_1, leaf4_1}
4320+
# Commit A: node1/{ leaf1_2, leaf2_2, node2/{leaf3_2, leaf4_2}, leaf5}
4321+
# Commit B: node2/{node1/{leaf1_3, leaf2_3}, leaf3_3, leaf4_3, leaf6}
4322+
# Expected: Content merge conflicts for each of:
4323+
# node1/node2/node1/{leaf1, leaf2}
4324+
# node2/node1/node2/{leaf3, leaf4}
4325+
# plus
4326+
# node2/node1/leaf5
4327+
# node1/node2/leaf6
4328+
# NOTE: This is *exactly* like 12b2, except that every path from O is modified
4329+
# on each side of the merge.
4330+
4331+
test_setup_12c2 () {
4332+
test_create_repo 12c2 &&
4333+
(
4334+
cd 12c2 &&
4335+
4336+
mkdir -p node1 node2 &&
4337+
printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf1\n" >node1/leaf1 &&
4338+
printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf2\n" >node1/leaf2 &&
4339+
printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf3\n" >node2/leaf3 &&
4340+
printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf4\n" >node2/leaf4 &&
4341+
git add node1 node2 &&
4342+
test_tick &&
4343+
git commit -m "O" &&
4344+
4345+
git branch O &&
4346+
git branch A &&
4347+
git branch B &&
4348+
4349+
git checkout A &&
4350+
git mv node2/ node1/ &&
4351+
for i in `git ls-files`; do echo side A >>$i; done &&
4352+
git add -u &&
4353+
echo leaf5 >node1/leaf5 &&
4354+
git add node1/leaf5 &&
4355+
test_tick &&
4356+
git commit -m "A" &&
4357+
4358+
git checkout B &&
4359+
git mv node1/ node2/ &&
4360+
for i in `git ls-files`; do echo side B >>$i; done &&
4361+
git add -u &&
4362+
echo leaf6 >node2/leaf6 &&
4363+
git add node2/leaf6 &&
4364+
test_tick &&
4365+
git commit -m "B"
4366+
)
4367+
}
4368+
4369+
test_expect_success '12c2: Moving one directory hierarchy into another w/ content merge' '
4370+
test_setup_12c2 &&
4371+
(
4372+
cd 12c2 &&
4373+
4374+
git checkout A^0 &&
4375+
4376+
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 &&
4377+
4378+
git ls-files -s >out &&
4379+
test_line_count = 14 out &&
4380+
git ls-files -u >out &&
4381+
test_line_count = 12 out &&
4382+
41864383
git rev-parse >actual \
41874384
:1:node1/node2/node1/leaf1 \
41884385
:1:node1/node2/node1/leaf2 \
@@ -4195,7 +4392,9 @@ test_expect_success '12c: Moving one directory hierarchy into another w/ content
41954392
:3:node1/node2/node1/leaf1 \
41964393
:3:node1/node2/node1/leaf2 \
41974394
:3:node2/node1/node2/leaf3 \
4198-
:3:node2/node1/node2/leaf4 &&
4395+
:3:node2/node1/node2/leaf4 \
4396+
:0:node2/node1/leaf5 \
4397+
:0:node1/node2/leaf6 &&
41994398
git rev-parse >expect \
42004399
O:node1/leaf1 \
42014400
O:node1/leaf2 \
@@ -4208,7 +4407,9 @@ test_expect_success '12c: Moving one directory hierarchy into another w/ content
42084407
B:node2/node1/leaf1 \
42094408
B:node2/node1/leaf2 \
42104409
B:node2/leaf3 \
4211-
B:node2/leaf4 &&
4410+
B:node2/leaf4 \
4411+
A:node1/leaf5 \
4412+
B:node2/leaf6 &&
42124413
test_cmp expect actual
42134414
)
42144415
'

0 commit comments

Comments
 (0)