Skip to content

Commit ef52778

Browse files
newrengitster
newren
authored and
gitster
committed
merge tests: expect improved directory/file conflict handling in ort
merge-recursive.c is built on the idea of running unpack_trees() and then "doing minor touch-ups" to get the result. Unfortunately, unpack_trees() was run in an update-as-it-goes mode, leading merge-recursive.c to follow suit and end up with an immediate evaluation and fix-it-up-as-you-go design. Some things like directory/file conflicts are not well representable in the index data structure, and required special extra code to handle. But then when it was discovered that rename/delete conflicts could also be involved in directory/file conflicts, the special directory/file conflict handling code had to be copied to the rename/delete codepath. ...and then it had to be copied for modify/delete, and for rename/rename(1to2) conflicts, ...and yet it still missed some. Further, when it was discovered that there were also file/submodule conflicts and submodule/directory conflicts, we needed to copy the special submodule handling code to all the special cases throughout the codebase. And then it was discovered that our handling of directory/file conflicts was suboptimal because it would create untracked files to store the contents of the conflicting file, which would not be cleaned up if someone were to run a 'git merge --abort' or 'git rebase --abort'. It was also difficult or scary to try to add or remove the index entries corresponding to these files given the directory/file conflict in the index. But changing merge-recursive.c to handle these correctly was a royal pain because there were so many sites in the code with similar but not identical code for handling directory/file/submodule conflicts that would all need to be updated. I have worked hard to push all directory/file/submodule conflict handling in merge-ort through a single codepath, and avoid creating untracked files for storing tracked content (it does record things at alternate paths, but makes sure they have higher-order stages in the index). Since updating merge-recursive is too much work and we don't want to destabilize it, instead update the testsuite to have different expectations for relevant directory/file/submodule conflict tests. Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent f06481f commit ef52778

6 files changed

+314
-108
lines changed

‎t/t6400-merge-df.sh

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,12 @@ test_expect_success 'modify/delete + directory/file conflict' '
8181
8282
test 5 -eq $(git ls-files -s | wc -l) &&
8383
test 4 -eq $(git ls-files -u | wc -l) &&
84-
test 1 -eq $(git ls-files -o | wc -l) &&
84+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
85+
then
86+
test 0 -eq $(git ls-files -o | wc -l)
87+
else
88+
test 1 -eq $(git ls-files -o | wc -l)
89+
fi &&
8590
8691
test_path_is_file letters/file &&
8792
test_path_is_file letters.txt &&
@@ -97,7 +102,12 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
97102
98103
test 5 -eq $(git ls-files -s | wc -l) &&
99104
test 4 -eq $(git ls-files -u | wc -l) &&
100-
test 1 -eq $(git ls-files -o | wc -l) &&
105+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
106+
then
107+
test 0 -eq $(git ls-files -o | wc -l)
108+
else
109+
test 1 -eq $(git ls-files -o | wc -l)
110+
fi &&
101111
102112
test_path_is_file letters/file &&
103113
test_path_is_file letters.txt &&

‎t/t6402-merge-rename.sh

Lines changed: 81 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,12 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
397397
test_must_fail git merge --strategy=recursive dir-in-way &&
398398
399399
test 5 -eq "$(git ls-files -u | wc -l)" &&
400-
test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
400+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
401+
then
402+
test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
403+
else
404+
test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
405+
fi &&
401406
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
402407
403408
test_must_fail git diff --quiet &&
@@ -415,7 +420,12 @@ test_expect_success 'Same as previous, but merged other way' '
415420
test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
416421
417422
test 5 -eq "$(git ls-files -u | wc -l)" &&
418-
test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
423+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
424+
then
425+
test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
426+
else
427+
test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
428+
fi &&
419429
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
420430
421431
test_must_fail git diff --quiet &&
@@ -471,7 +481,12 @@ test_expect_success 'both rename source and destination involved in D/F conflict
471481
git checkout -q rename-dest^0 &&
472482
test_must_fail git merge --strategy=recursive source-conflict &&
473483
474-
test 1 -eq "$(git ls-files -u | wc -l)" &&
484+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
485+
then
486+
test 2 -eq "$(git ls-files -u | wc -l)"
487+
else
488+
test 1 -eq "$(git ls-files -u | wc -l)"
489+
fi &&
475490
476491
test_must_fail git diff --quiet &&
477492
@@ -505,34 +520,63 @@ test_expect_success 'setup pair rename to parent of other (D/F conflicts)' '
505520
git commit -m "Rename one/file -> two"
506521
'
507522

508-
test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
509-
git checkout -q rename-one^0 &&
510-
mkdir one &&
511-
test_must_fail git merge --strategy=recursive rename-two &&
523+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
524+
then
525+
test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
526+
git checkout -q rename-one^0 &&
527+
mkdir one &&
528+
test_must_fail git merge --strategy=recursive rename-two &&
512529
513-
test 2 -eq "$(git ls-files -u | wc -l)" &&
514-
test 1 -eq "$(git ls-files -u one | wc -l)" &&
515-
test 1 -eq "$(git ls-files -u two | wc -l)" &&
530+
test 4 -eq "$(git ls-files -u | wc -l)" &&
531+
test 2 -eq "$(git ls-files -u one | wc -l)" &&
532+
test 2 -eq "$(git ls-files -u two | wc -l)" &&
516533
517-
test_must_fail git diff --quiet &&
534+
test_must_fail git diff --quiet &&
518535
519-
test 4 -eq $(find . | grep -v .git | wc -l) &&
536+
test 3 -eq $(find . | grep -v .git | wc -l) &&
520537
521-
test_path_is_dir one &&
522-
test_path_is_file one~rename-two &&
523-
test_path_is_file two &&
524-
test "other" = $(cat one~rename-two) &&
525-
test "stuff" = $(cat two)
526-
'
538+
test_path_is_file one &&
539+
test_path_is_file two &&
540+
test "other" = $(cat one) &&
541+
test "stuff" = $(cat two)
542+
'
543+
else
544+
test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
545+
git checkout -q rename-one^0 &&
546+
mkdir one &&
547+
test_must_fail git merge --strategy=recursive rename-two &&
548+
549+
test 2 -eq "$(git ls-files -u | wc -l)" &&
550+
test 1 -eq "$(git ls-files -u one | wc -l)" &&
551+
test 1 -eq "$(git ls-files -u two | wc -l)" &&
552+
553+
test_must_fail git diff --quiet &&
554+
555+
test 4 -eq $(find . | grep -v .git | wc -l) &&
556+
557+
test_path_is_dir one &&
558+
test_path_is_file one~rename-two &&
559+
test_path_is_file two &&
560+
test "other" = $(cat one~rename-two) &&
561+
test "stuff" = $(cat two)
562+
'
563+
fi
527564

528565
test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean start' '
529566
git reset --hard &&
530567
git clean -fdqx &&
531568
test_must_fail git merge --strategy=recursive rename-two &&
532569
533-
test 2 -eq "$(git ls-files -u | wc -l)" &&
534-
test 1 -eq "$(git ls-files -u one | wc -l)" &&
535-
test 1 -eq "$(git ls-files -u two | wc -l)" &&
570+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
571+
then
572+
test 4 -eq "$(git ls-files -u | wc -l)" &&
573+
test 2 -eq "$(git ls-files -u one | wc -l)" &&
574+
test 2 -eq "$(git ls-files -u two | wc -l)"
575+
else
576+
test 2 -eq "$(git ls-files -u | wc -l)" &&
577+
test 1 -eq "$(git ls-files -u one | wc -l)" &&
578+
test 1 -eq "$(git ls-files -u two | wc -l)"
579+
fi &&
536580
537581
test_must_fail git diff --quiet &&
538582
@@ -572,12 +616,22 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
572616
git checkout -q first-rename^0 &&
573617
test_must_fail git merge --strategy=recursive second-rename &&
574618
575-
test 5 -eq "$(git ls-files -s | wc -l)" &&
576-
test 3 -eq "$(git ls-files -u | wc -l)" &&
577-
test 1 -eq "$(git ls-files -u one | wc -l)" &&
578-
test 1 -eq "$(git ls-files -u two | wc -l)" &&
579-
test 1 -eq "$(git ls-files -u original | wc -l)" &&
580-
test 2 -eq "$(git ls-files -o | wc -l)" &&
619+
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
620+
then
621+
test 5 -eq "$(git ls-files -s | wc -l)" &&
622+
test 3 -eq "$(git ls-files -u | wc -l)" &&
623+
test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
624+
test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
625+
test 1 -eq "$(git ls-files -u original | wc -l)" &&
626+
test 0 -eq "$(git ls-files -o | wc -l)"
627+
else
628+
test 5 -eq "$(git ls-files -s | wc -l)" &&
629+
test 3 -eq "$(git ls-files -u | wc -l)" &&
630+
test 1 -eq "$(git ls-files -u one | wc -l)" &&
631+
test 1 -eq "$(git ls-files -u two | wc -l)" &&
632+
test 1 -eq "$(git ls-files -u original | wc -l)" &&
633+
test 2 -eq "$(git ls-files -o | wc -l)"
634+
fi &&
581635
582636
test_path_is_file one/file &&
583637
test_path_is_file two/file &&

0 commit comments

Comments
 (0)