merge-recursive: avoid directory rename detection in recursive case
Ever since commit 8c8e5bd6eb33 ("merge-recursive: switch directory rename detection default", 2019-04-05), the default handling with directory rename detection was to report a conflict and leave unstaged entries in the index. However, when creating a virtual merge base in the recursive case, we absolutely need a tree, and the only way a tree can be written is if we have no unstaged entries -- otherwise we hit a BUG(). There are a few fixes possible here which at least fix the BUG(), but none of them seem optimal for other reasons; see the comments with the new testcase 13e in t6043 for details (which testcase triggered a BUG() prior to this patch). As such, just opt for a very conservative and simple choice that is still relatively reasonable: have the recursive case treat 'conflict' as 'false' for opt->detect_directory_renames. Reported-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren committed
Aug 5, 2019 at 15:33 UTC
ff6d54771af2c92867f773a264200f22b4cb2ab9
2 files changed
+113
-1
merge-recursive.c
+2
-1
@@ -2856,7 +2856,8 @@ static int detect_and_process_renames(struct merge_options *opt,
2856
head_pairs = get_diffpairs(opt, common, head);
2857
merge_pairs = get_diffpairs(opt, common, merge);
2858
2859
- if (opt->detect_directory_renames) {
2859
+ if ((opt->detect_directory_renames == 2) ||
2860
+ (opt->detect_directory_renames == 1 && !opt->call_depth)) {
2861
dir_re_head = get_directory_renames(head_pairs);
2862
dir_re_merge = get_directory_renames(merge_pairs);
2863
t/t6043-merge-rename-directories.sh
+111
@@ -4403,4 +4403,115 @@ test_expect_success '13d-check(info): messages for rename/rename(1to1) via dual
4403
)
4404
'
4405
4406
+# Testcase 13e, directory rename in virtual merge base
4407
+#
4408
+# This testcase has a slightly different setup than all the above cases, in
4409
+# order to include a recursive case:
4410
+#
4411
+# A C
4412
+# o - o
4413
+# / \ / \
4414
+# O o X ?
4415
+# \ / \ /
4416
+# o o
4417
+# B D
4418
+#
4419
+# Commit O: a/{z,y}
4420
+# Commit A: b/{z,y}
4421
+# Commit B: a/{z,y,x}
4422
+# Commit C: b/{z,y,x}
4423
+# Commit D: b/{z,y}, a/x
4424
+# Expected: b/{z,y,x} (sort of; see below for why this might not be expected)
4425
+#
4426
+# NOTES: 'X' represents a virtual merge base. With the default of
4427
+# directory rename detection yielding conflicts, merging A and B
4428
+# results in a conflict complaining about whether 'x' should be
4429
+# under 'a/' or 'b/'. However, when creating the virtual merge
4430
+# base 'X', since virtual merge bases need to be written out as a
4431
+# tree, we cannot have a conflict, so some resolution has to be
4432
+# picked.
4433
+#
4434
+# In choosing the right resolution, it's worth noting here that
4435
+# commits C & D are merges of A & B that choose different
4436
+# locations for 'x' (i.e. they resolve the conflict differently),
4437
+# and so it would be nice when merging C & D if git could detect
4438
+# this difference of opinion and report a conflict. But the only
4439
+# way to do so that I can think of would be to have the virtual
4440
+# merge base place 'x' in some directory other than either 'a/' or
4441
+# 'b/', which seems a little weird -- especially since it'd result
4442
+# in a rename/rename(1to2) conflict with a source path that never
4443
+# existed in any version.
4444
+#
4445
+# So, for now, when directory rename detection is set to
4446
+# 'conflict' just avoid doing directory rename detection at all in
4447
+# the recursive case. This will not allow us to detect a conflict
4448
+# in the outer merge for this special kind of setup, but it at
4449
+# least avoids hitting a BUG().
4450
+#
4451
+test_expect_success '13e-setup: directory rename detection in recursive case' '
4452
+ test_create_repo 13e &&
4453
+ (
4454
+ cd 13e &&
4455
+
4456
+ mkdir a &&
4457
+ echo z >a/z &&
4458
+ echo y >a/y &&
4459
+ git add a &&
4460
+ test_tick &&
4461
+ git commit -m "O" &&
4462
+
4463
+ git branch O &&
4464
+ git branch A &&
4465
+ git branch B &&
4466
+
4467
+ git checkout A &&
4468
+ git mv a/ b/ &&
4469
+ test_tick &&
4470
+ git commit -m "A" &&
4471
+
4472
+ git checkout B &&
4473
+ echo x >a/x &&
4474
+ git add a &&
4475
+ test_tick &&
4476
+ git commit -m "B" &&
4477
+
4478
+ git branch C A &&
4479
+ git branch D B &&
4480
+
4481
+ git checkout C &&
4482
+ test_must_fail git -c merge.directoryRenames=conflict merge B &&
4483
+ git add b/x &&
4484
+ test_tick &&
4485
+ git commit -m "C" &&
4486
+
4487
+
4488
+ git checkout D &&
4489
+ test_must_fail git -c merge.directoryRenames=conflict merge A &&
4490
+ git add b/x &&
4491
+ mkdir a &&
4492
+ git mv b/x a/x &&
4493
+ test_tick &&
4494
+ git commit -m "D"
4495
+ )
4496
+'
4497
+
4498
+test_expect_success '13e-check: directory rename detection in recursive case' '
4499
+ (
4500
+ cd 13e &&
4501
+
4502
+ git checkout --quiet D^0 &&
4503
+
4504
+ git -c merge.directoryRenames=conflict merge -s recursive C^0 >out 2>err &&
4505
+
4506
+ test_i18ngrep ! CONFLICT out &&
4507
+ test_i18ngrep ! BUG: err &&
4508
+ test_i18ngrep ! core.dumped err &&
4509
+ test_must_be_empty err &&
4510
+
4511
+ git ls-files >paths &&
4512
+ ! grep a/x paths &&
4513
+ grep b/x paths
4514
+ )
4515
+'
4516
+
4517
test_done