This patch series builds on top of en/dir-rename-tests. It can be applied independent of my "new merge-ort" API series I submitted a couple days ago[1], but this series uses the same environment variable as patch 4 of that series. [Also, if gitgitgadget emails more than 8 patches, then my attempts to use it to send out a patch series based on en/dir-rename-tests went wrong.] As promised, here is a series that makes the testsuite changes needed to simultaneously support both merge backends, keyed off a GIT_TEST_MERGE_ALGORITHM environment variable setting. NOTE: The tests do not yet pass with GIT_TEST_MERGE_ALGORITHM=ort, because I haven't submitted the implementation of the merge-ort functions -- yet. I figured they are useful as a high level overview of the differences in behavior between the two backends, and thus am providing these before the implementation. [1] https://lore.kernel.org/git/pull.895.git.git.1603286555.gitgitgadget@gmail.com/ Elijah Newren (9): t/: new helper for tests that pass with ort but fail with recursive merge tests: expect improved directory/file conflict handling in ort t6416: correct expectation for rename/rename(1to2) + directory/file t6404, t6423: expect improved rename/delete handling in ort backend t6423: expect improved conflict markers labels in the ort backend merge tests: expect slight differences in output for recursive vs. ort t6423, t6436: note improved ort handling with dirty files t6423: note improved ort handling with untracked files t6423: add more details about direct resolution of directories t/lib-merge.sh | 15 + t/t6400-merge-df.sh | 14 +- t/t6402-merge-rename.sh | 122 ++++- t/t6404-recursive-merge.sh | 14 +- t/t6416-recursive-corner-cases.sh | 200 ++++--- t/t6422-merge-rename-corner-cases.sh | 37 +- t/t6423-merge-rename-directories.sh | 704 +++++++++++++++---------- t/t6426-merge-skip-unneeded-updates.sh | 3 +- t/t6430-merge-recursive.sh | 3 +- t/t6436-merge-overwrite.sh | 18 +- t/t6437-submodule-merge.sh | 25 +- t/t7602-merge-octopus-many.sh | 4 + t/t7610-mergetool.sh | 32 +- 13 files changed, 805 insertions(+), 386 deletions(-) create mode 100644 t/lib-merge.sh base-commit: c64432aacda9054459ce550eca95929897c301bd Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-769%2Fnewren%2Ftests-support-both-merge-backends-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-769/newren/tests-support-both-merge-backends-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/769 -- gitgitgadget
From: Elijah Newren <newren@gmail.com> There are a number of tests that the "recursive" backend does not handle correctly but which the redesign in "ort" will. Add a new helper in lib-merge.sh for selecting a different test expectation based on the setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to document which ones we expect to fail under recursive but pass under ort. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/lib-merge.sh | 15 +++++++++++++++ t/t6416-recursive-corner-cases.sh | 11 ++++++----- t/t6422-merge-rename-corner-cases.sh | 7 ++++--- t/t6423-merge-rename-directories.sh | 13 +++++++------ t/t6426-merge-skip-unneeded-updates.sh | 3 ++- t/t6430-merge-recursive.sh | 3 ++- 6 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 t/lib-merge.sh diff --git a/t/lib-merge.sh b/t/lib-merge.sh new file mode 100644 index 0000000000..fac2bc5919 --- /dev/null +++ b/t/lib-merge.sh @@ -0,0 +1,15 @@ +# Helper functions used by merge tests. + +test_expect_merge_algorithm () { + status_for_recursive=$1 + shift + status_for_ort=$1 + shift + + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_expect_${status_for_ort} "$@" + else + test_expect_${status_for_recursive} "$@" + fi +} diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index fd98989b14..8b3a4fc843 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -3,6 +3,7 @@ test_description='recursive merge corner cases involving criss-cross merges' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh # # L1 L2 @@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' ' ) ' -test_expect_failure 'check symlink modify/modify' ' +test_expect_merge_algorithm failure success 'check symlink modify/modify' ' ( cd symlink-modify-modify && @@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' ' ) ' -test_expect_failure 'check symlink add/add' ' +test_expect_merge_algorithm failure success 'check symlink add/add' ' ( cd symlink-add-add && @@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' ' ) ' -test_expect_failure 'check submodule modify/modify' ' +test_expect_merge_algorithm failure success 'check submodule modify/modify' ' ( cd submodule-modify-modify && @@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' ' ) ' -test_expect_failure 'check submodule add/add' ' +test_expect_merge_algorithm failure success 'check submodule add/add' ' ( cd submodule-add-add && @@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' ' ) ' -test_expect_failure 'check conflicting entry types (submodule vs symlink)' ' +test_expect_merge_algorithm failure success 'check conflicting entry types (submodule vs symlink)' ' ( cd submodule-symlink-add-add && diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh index 3375eaf4e7..58729593ba 100755 --- a/t/t6422-merge-rename-corner-cases.sh +++ b/t/t6422-merge-rename-corner-cases.sh @@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses" # t6036 has corner cases that involve both criss-cross merges and renames . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh test_setup_rename_delete_untracked () { test_create_repo rename-delete-untracked && @@ -878,7 +879,7 @@ test_setup_rad () { ) } -test_expect_failure 'rad-check: rename/add/delete conflict' ' +test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' ' test_setup_rad && ( cd rad && @@ -951,7 +952,7 @@ test_setup_rrdd () { ) } -test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' +test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' test_setup_rrdd && ( cd rrdd && @@ -1040,7 +1041,7 @@ test_setup_mod6 () { ) } -test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' +test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' test_setup_mod6 && ( cd mod6 && diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 06b46af765..807a424a52 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -26,6 +26,7 @@ test_description="recursive merge with directory renames" # files that might be renamed into each other's paths.) . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh ########################################################################### @@ -1339,7 +1340,7 @@ test_setup_6b1 () { ) } -test_expect_failure '6b1: Same renames done on both sides, plus another rename' ' +test_expect_merge_algorithm failure success '6b1: Same renames done on both sides, plus another rename' ' test_setup_6b1 && ( cd 6b1 && @@ -1412,7 +1413,7 @@ test_setup_6b2 () { ) } -test_expect_failure '6b2: Same rename done on both sides' ' +test_expect_merge_algorithm failure success '6b2: Same rename done on both sides' ' test_setup_6b2 && ( cd 6b2 && @@ -3471,7 +3472,7 @@ test_setup_10e () { ) } -test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' ' +test_expect_merge_algorithm failure success '10e: Does git complain about untracked file that is not really in the way?' ' test_setup_10e && ( cd 10e && @@ -4104,7 +4105,7 @@ test_setup_12b1 () { ) } -test_expect_failure '12b1: Moving two directory hierarchies into each other' ' +test_expect_merge_algorithm failure success '12b1: Moving two directory hierarchies into each other' ' test_setup_12b1 && ( cd 12b1 && @@ -4272,7 +4273,7 @@ test_setup_12c1 () { ) } -test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' ' +test_expect_merge_algorithm failure success '12c1: Moving one directory hierarchy into another w/ content merge' ' test_setup_12c1 && ( cd 12c1 && @@ -4632,7 +4633,7 @@ test_setup_12f () { ) } -test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' ' +test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' ' test_setup_12f && ( cd 12f && diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh index 699813671c..d7eeee4310 100755 --- a/t/t6426-merge-skip-unneeded-updates.sh +++ b/t/t6426-merge-skip-unneeded-updates.sh @@ -23,6 +23,7 @@ test_description="merge cases" # files that might be renamed into each other's paths.) . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh ########################################################################### @@ -666,7 +667,7 @@ test_setup_4a () { # correct requires doing the merge in-memory first, then realizing that no # updates to the file are necessary, and thus that we can just leave the path # alone. -test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' ' +test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' ' test_setup_4a && ( cd 4a && diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh index a328260d42..9c08e63af2 100755 --- a/t/t6430-merge-recursive.sh +++ b/t/t6430-merge-recursive.sh @@ -3,6 +3,7 @@ test_description='merge-recursive backend test' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh test_expect_success 'setup 1' ' @@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' ' test_cmp expected actual ' -test_expect_failure 'merge-recursive rename vs. rename/symlink' ' +test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' ' git checkout -f rename && git merge rename-ln && -- gitgitgadget
From: Elijah Newren <newren@gmail.com> 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 <newren@gmail.com> --- t/t6400-merge-df.sh | 14 +- t/t6402-merge-rename.sh | 108 ++++++++++++---- t/t6416-recursive-corner-cases.sh | 185 +++++++++++++++++++-------- t/t6422-merge-rename-corner-cases.sh | 30 +++-- t/t6423-merge-rename-directories.sh | 53 +++++--- t/t7610-mergetool.sh | 32 ++++- 6 files changed, 314 insertions(+), 108 deletions(-) diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh index f1b84617af..9da0838216 100755 --- a/t/t6400-merge-df.sh +++ b/t/t6400-merge-df.sh @@ -81,7 +81,12 @@ test_expect_success 'modify/delete + directory/file conflict' ' test 5 -eq $(git ls-files -s | wc -l) && test 4 -eq $(git ls-files -u | wc -l) && - test 1 -eq $(git ls-files -o | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 0 -eq $(git ls-files -o | wc -l) + else + test 1 -eq $(git ls-files -o | wc -l) + fi && test_path_is_file letters/file && test_path_is_file letters.txt && @@ -97,7 +102,12 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' test 5 -eq $(git ls-files -s | wc -l) && test 4 -eq $(git ls-files -u | wc -l) && - test 1 -eq $(git ls-files -o | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 0 -eq $(git ls-files -o | wc -l) + else + test 1 -eq $(git ls-files -o | wc -l) + fi && test_path_is_file letters/file && test_path_is_file letters.txt && diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index bbbba3dcbf..47d4434d64 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -397,7 +397,12 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t test_must_fail git merge --strategy=recursive dir-in-way && test 5 -eq "$(git ls-files -u | wc -l)" && - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)" + else + test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" + fi && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && test_must_fail git diff --quiet && @@ -415,7 +420,12 @@ test_expect_success 'Same as previous, but merged other way' ' test_must_fail git merge --strategy=recursive renamed-file-has-conflicts && test 5 -eq "$(git ls-files -u | wc -l)" && - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)" + else + test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" + fi && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && test_must_fail git diff --quiet && @@ -471,7 +481,12 @@ test_expect_success 'both rename source and destination involved in D/F conflict git checkout -q rename-dest^0 && test_must_fail git merge --strategy=recursive source-conflict && - test 1 -eq "$(git ls-files -u | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 2 -eq "$(git ls-files -u | wc -l)" + else + test 1 -eq "$(git ls-files -u | wc -l)" + fi && test_must_fail git diff --quiet && @@ -505,34 +520,63 @@ test_expect_success 'setup pair rename to parent of other (D/F conflicts)' ' git commit -m "Rename one/file -> two" ' -test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' ' - git checkout -q rename-one^0 && - mkdir one && - test_must_fail git merge --strategy=recursive rename-two && +if test "$GIT_TEST_MERGE_ALGORITHM" = ort +then + test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' ' + git checkout -q rename-one^0 && + mkdir one && + test_must_fail git merge --strategy=recursive rename-two && - test 2 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && + test 4 -eq "$(git ls-files -u | wc -l)" && + test 2 -eq "$(git ls-files -u one | wc -l)" && + test 2 -eq "$(git ls-files -u two | wc -l)" && - test_must_fail git diff --quiet && + test_must_fail git diff --quiet && - test 4 -eq $(find . | grep -v .git | wc -l) && + test 3 -eq $(find . | grep -v .git | wc -l) && - test_path_is_dir one && - test_path_is_file one~rename-two && - test_path_is_file two && - test "other" = $(cat one~rename-two) && - test "stuff" = $(cat two) -' + test_path_is_file one && + test_path_is_file two && + test "other" = $(cat one) && + test "stuff" = $(cat two) + ' +else + test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' ' + git checkout -q rename-one^0 && + mkdir one && + test_must_fail git merge --strategy=recursive rename-two && + + test 2 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one | wc -l)" && + test 1 -eq "$(git ls-files -u two | wc -l)" && + + test_must_fail git diff --quiet && + + test 4 -eq $(find . | grep -v .git | wc -l) && + + test_path_is_dir one && + test_path_is_file one~rename-two && + test_path_is_file two && + test "other" = $(cat one~rename-two) && + test "stuff" = $(cat two) + ' +fi test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean start' ' git reset --hard && git clean -fdqx && test_must_fail git merge --strategy=recursive rename-two && - test 2 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 4 -eq "$(git ls-files -u | wc -l)" && + test 2 -eq "$(git ls-files -u one | wc -l)" && + test 2 -eq "$(git ls-files -u two | wc -l)" + else + test 2 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one | wc -l)" && + test 1 -eq "$(git ls-files -u two | wc -l)" + fi && test_must_fail git diff --quiet && @@ -572,12 +616,22 @@ test_expect_success 'check handling of differently renamed file with D/F conflic git checkout -q first-rename^0 && test_must_fail git merge --strategy=recursive second-rename && - test 5 -eq "$(git ls-files -s | wc -l)" && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && - test 1 -eq "$(git ls-files -u original | wc -l)" && - test 2 -eq "$(git ls-files -o | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 5 -eq "$(git ls-files -s | wc -l)" && + test 3 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" && + test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" && + test 1 -eq "$(git ls-files -u original | wc -l)" && + test 0 -eq "$(git ls-files -o | wc -l)" + else + test 5 -eq "$(git ls-files -s | wc -l)" && + test 3 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one | wc -l)" && + test 1 -eq "$(git ls-files -u two | wc -l)" && + test 1 -eq "$(git ls-files -u original | wc -l)" && + test 2 -eq "$(git ls-files -o | wc -l)" + fi && test_path_is_file one/file && test_path_is_file two/file && diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index 8b3a4fc843..0317e83970 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -538,9 +538,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict git checkout B^0 && test_must_fail git merge C^0 && - git clean -fd && - git rm -rf a/ && - git rm a && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rm -rf a/ && + git rm a~HEAD + else + git clean -fd && + git rm -rf a/ && + git rm a + fi && git cat-file -p B:a >a2 && git add a2 && git commit -m D2 && @@ -559,7 +565,12 @@ test_expect_success 'setup differently handled merges of directory/file conflict git checkout C^0 && test_must_fail git merge B^0 && - git clean -fd && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rm a~B^0 + else + git clean -fd + fi && git rm -rf a/ && test_write_lines 1 2 3 4 5 6 7 8 >a && git add a && @@ -568,9 +579,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict git checkout C^0 && test_must_fail git merge B^0 && - git clean -fd && - git rm -rf a/ && - git rm a && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rm -rf a/ && + git rm a~B^0 + else + git clean -fd && + git rm -rf a/ && + git rm a + fi && test_write_lines 1 2 3 4 5 6 7 8 >a2 && git add a2 && git commit -m E4 && @@ -588,18 +605,34 @@ test_expect_success 'merge of D1 & E1 fails but has appropriate contents' ' test_must_fail git merge -s recursive E1^0 && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && - - git rev-parse >expect \ - A:ignore-me B:a && - git rev-parse >actual \ - :0:ignore-me :2:a && - test_cmp expect actual + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a D1:a && + git rev-parse >actual \ + :0:ignore-me :1:a :2:a && + test_cmp expect actual + else + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a && + git rev-parse >actual \ + :0:ignore-me :2:a && + test_cmp expect actual + fi ) ' @@ -613,18 +646,34 @@ test_expect_success 'merge of E1 & D1 fails but has appropriate contents' ' test_must_fail git merge -s recursive D1^0 && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && - - git rev-parse >expect \ - A:ignore-me B:a && - git rev-parse >actual \ - :0:ignore-me :3:a && - test_cmp expect actual + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a D1:a && + git rev-parse >actual \ + :0:ignore-me :1:a :3:a && + test_cmp expect actual + else + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a && + git rev-parse >actual \ + :0:ignore-me :3:a && + test_cmp expect actual + fi ) ' @@ -638,17 +687,32 @@ test_expect_success 'merge of D1 & E2 fails but has appropriate contents' ' test_must_fail git merge -s recursive E2^0 && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >expect \ - B:a E2:a/file C:a/file A:ignore-me && - git rev-parse >actual \ - :2:a :3:a/file :1:a/file :0:ignore-me && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 5 out && + git ls-files -u >out && + test_line_count = 4 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + B:a D1:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :1:a~HEAD :2:a~HEAD :3:a/file :1:a/file :0:ignore-me + else + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 2 out && + + git rev-parse >expect \ + B:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :2:a :3:a/file :1:a/file :0:ignore-me + fi && test_cmp expect actual && test_path_is_file a~HEAD @@ -665,17 +729,32 @@ test_expect_success 'merge of E2 & D1 fails but has appropriate contents' ' test_must_fail git merge -s recursive D1^0 && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >expect \ - B:a E2:a/file C:a/file A:ignore-me && - git rev-parse >actual \ - :3:a :2:a/file :1:a/file :0:ignore-me && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 5 out && + git ls-files -u >out && + test_line_count = 4 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + B:a D1:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :1:a~D1^0 :3:a~D1^0 :2:a/file :1:a/file :0:ignore-me + else + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 2 out && + + git rev-parse >expect \ + B:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :3:a :2:a/file :1:a/file :0:ignore-me + fi && test_cmp expect actual && test_path_is_file a~D1^0 diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh index 58729593ba..78bfaf17f0 100755 --- a/t/t6422-merge-rename-corner-cases.sh +++ b/t/t6422-merge-rename-corner-cases.sh @@ -313,15 +313,18 @@ test_expect_success 'rename/directory conflict + clean content merge' ' git ls-files -u >out && test_line_count = 1 out && git ls-files -o >out && - test_line_count = 2 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_line_count = 1 out + else + test_line_count = 2 out + fi && echo 0 >expect && git cat-file -p base:file >>expect && echo 7 >>expect && test_cmp expect newfile~HEAD && - test $(git rev-parse :2:newfile) = $(git hash-object expect) && - test_path_is_file newfile/realfile && test_path_is_file newfile~HEAD ) @@ -344,7 +347,12 @@ test_expect_success 'rename/directory conflict + content merge conflict' ' git ls-files -u >out && test_line_count = 3 out && git ls-files -o >out && - test_line_count = 2 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_line_count = 1 out + else + test_line_count = 2 out + fi && git cat-file -p left-conflict:newfile >left && git cat-file -p base:file >base && @@ -356,10 +364,16 @@ test_expect_success 'rename/directory conflict + content merge conflict' ' left base right && test_cmp left newfile~HEAD && - git rev-parse >expect \ - base:file left-conflict:newfile right:file && - git rev-parse >actual \ - :1:newfile :2:newfile :3:newfile && + git rev-parse >expect \ + base:file left-conflict:newfile right:file && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rev-parse >actual \ + :1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD + else + git rev-parse >actual \ + :1:newfile :2:newfile :3:newfile + fi && test_cmp expect actual && test_path_is_file newfile/realfile && diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 807a424a52..5ea77564d7 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -1177,10 +1177,18 @@ test_expect_success '5d: Directory/file/file conflict due to directory rename' ' git ls-files -u >out && test_line_count = 1 out && git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >actual \ - :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_line_count = 1 out && + + git rev-parse >actual \ + :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d~HEAD :0:y/d/e + else + test_line_count = 2 out && + + git rev-parse >actual \ + :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e + fi && git rev-parse >expect \ O:z/b O:z/c B:z/d B:z/f A:y/d B:y/d/e && test_cmp expect actual && @@ -2017,17 +2025,32 @@ test_expect_success '7e: transitive rename in rename/delete AND dirs in the way' test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out && test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out && - git ls-files -s >out && - test_line_count = 5 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >actual \ - :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d && - git rev-parse >expect \ - A:x/d/f A:y/d/g O:z/b O:z/c O:x/d && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >actual \ + :0:x/d/f :0:y/d/g :0:y/b :0:y/c :1:y/d~B^0 :3:y/d~B^0 && + git rev-parse >expect \ + A:x/d/f A:y/d/g O:z/b O:z/c O:x/d O:x/d + else + git ls-files -s >out && + test_line_count = 5 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 2 out && + + git rev-parse >actual \ + :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d && + git rev-parse >expect \ + A:x/d/f A:y/d/g O:z/b O:z/c O:x/d + fi && test_cmp expect actual && git hash-object y/d~B^0 >actual && diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index ad288ddc69..70afdd06fa 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -532,7 +532,14 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && - yes "l" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + yes "c" | git mergetool submod~HEAD && + git rm submod && + git mv submod~HEAD submod + else + yes "l" | git mergetool submod + fi && git submodule update -N && echo "not a submodule" >expect && test_cmp expect submod && @@ -549,7 +556,15 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && - yes "r" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + mv submod submod.orig && + git rm --cached submod && + yes "c" | git mergetool submod~test19 && + git mv submod~test19 submod + else + yes "r" | git mergetool submod + fi && test -d submod.orig && git submodule update -N && echo "not a submodule" >expect && @@ -567,6 +582,10 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && yes "l" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + yes "d" | git mergetool submod~test19 + fi && echo "master submodule" >expect && test_cmp expect submod/bar && git submodule update -N && @@ -664,7 +683,14 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && test ! -e submod.orig && - yes "r" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + yes "r" | git mergetool submod~master && + git mv submod submod.orig && + git mv submod~master submod + else + yes "r" | git mergetool submod + fi && test -d submod.orig && echo "not a submodule" >expect && test_cmp expect submod.orig/file16 && -- gitgitgadget
From: Elijah Newren <newren@gmail.com> When files are renamed and modified, we need to do three-way content merges to get the appropriate content in the right location. When we have a rename/rename(1to2) conflict (both sides rename the same file, but differently), that merged content should be placed in each of the two resulting files. merge-recursive handled that fine when that was all that was involved, but when one or more of the two resulting files were ALSO involved in a directory/file conflict, it failed to propagate the merged content to that file. Unfortunately, the one test in t6416 that touched on this combination of cases had been coded to not expect the merged contents to be present. Fix the test to check for the right behavior, and record how the different merge backends will be expected to handle it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6416-recursive-corner-cases.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index 0317e83970..887c2195a9 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -786,7 +786,7 @@ test_expect_success 'merge of D1 & E3 succeeds' ' ) ' -test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' ' +test_expect_merge_algorithm failure success 'merge of D1 & E4 puts merge of a and a2 in both a and a2' ' test_when_finished "git -C directory-file reset --hard" && test_when_finished "git -C directory-file clean -fdqx" && ( @@ -804,7 +804,7 @@ test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' ' test_line_count = 1 out && git rev-parse >expect \ - A:ignore-me B:a D1:a E4:a2 && + A:ignore-me B:a E4:a2 E4:a2 && git rev-parse >actual \ :0:ignore-me :1:a~Temporary\ merge\ branch\ 2 :2:a :3:a2 && test_cmp expect actual -- gitgitgadget
From: Elijah Newren <newren@gmail.com> When a file is renamed and has content conflicts, merge-recursive does not have some stages for the old filename and some stages for the new filename in the index; instead it copies all the stages corresponding to the old filename over to the corresponding locations for the new filename, so that there are three higher order stages all corresponding to the new filename. Doing things this way makes it easier for the user to access the different versions and to resolve the conflict (no need to manually 'git rm' the old version as well as 'git add' the new one). rename/deletes should be handled similarly -- there should be two stages for the renamed file rather than just one. We do not want to destabilize merge-recursive right now, so instead update relevant tests to have different expectations depending on whether the "recursive" or "ort" merge strategies are in use. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6404-recursive-merge.sh | 14 +++++- t/t6423-merge-rename-directories.sh | 70 ++++++++++++++++++++--------- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh index 332cfc53fd..b1c3d4dda4 100755 --- a/t/t6404-recursive-merge.sh +++ b/t/t6404-recursive-merge.sh @@ -118,12 +118,22 @@ test_expect_success 'mark rename/delete as unmerged' ' test_tick && git commit -m rename && test_must_fail git merge delete && - test 1 = $(git ls-files --unmerged | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 2 = $(git ls-files --unmerged | wc -l) + else + test 1 = $(git ls-files --unmerged | wc -l) + fi && git rev-parse --verify :2:a2 && test_must_fail git rev-parse --verify :3:a2 && git checkout -f delete && test_must_fail git merge rename && - test 1 = $(git ls-files --unmerged | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 2 = $(git ls-files --unmerged | wc -l) + else + test 1 = $(git ls-files --unmerged | wc -l) + fi && test_must_fail git rev-parse --verify :2:a2 && git rev-parse --verify :3:a2 ' diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 5ea77564d7..f9a3f24039 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -1271,17 +1271,32 @@ test_expect_success '6a: Tricky rename/delete' ' test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out && test_i18ngrep "CONFLICT (rename/delete).*z/c.*y/c" out && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && - git rev-parse >actual \ - :0:y/b :3:y/c && - git rev-parse >expect \ - O:z/b O:z/c && + git rev-parse >actual \ + :0:y/b :1:y/c :3:y/c && + git rev-parse >expect \ + O:z/b O:z/c O:z/c + else + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >actual \ + :0:y/b :3:y/c && + git rev-parse >expect \ + O:z/b O:z/c + fi && test_cmp expect actual ) ' @@ -1934,17 +1949,32 @@ test_expect_success '7d: transitive rename involved in rename/delete; how is it test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out && test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out && - git ls-files -s >out && - test_line_count = 3 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && - git rev-parse >actual \ - :0:y/b :0:y/c :3:y/d && - git rev-parse >expect \ - O:z/b O:z/c O:x/d && + git rev-parse >actual \ + :0:y/b :0:y/c :1:y/d :3:y/d && + git rev-parse >expect \ + O:z/b O:z/c O:x/d O:x/d + else + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >actual \ + :0:y/b :0:y/c :3:y/d && + git rev-parse >expect \ + O:z/b O:z/c O:x/d + fi && test_cmp expect actual ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> Conflict markers carry an extra annotation of the form REF-OR-COMMIT:FILENAME to help distinguish where the content is coming from, with the :FILENAME piece being left off if it is the same for both sides of history (thus only renames with content conflicts carry that part of the annotation). However, there were cases where the :FILENAME annotation was accidentally left off, due to merge-recursive's every-codepath-needs-a-copy-of-all-special-case-code format. Update a few tests to have the correct :FILENAME extension on relevant paths with the ort backend, while leaving the expectation for merge-recursive the same to avoid destabilizing it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 38 +++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index f9a3f24039..7e32626913 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -302,11 +302,20 @@ test_expect_success '1d: Directory renames cause a rename/rename(2to1) conflict' git cat-file -p :2:x/wham >expect && git cat-file -p :3:x/wham >other && >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge-file \ + -L "HEAD:y/wham" \ + -L "" \ + -L "B^0:z/wham" \ + expect empty other + else + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other + fi && test_cmp expect x/wham ) ' @@ -1823,11 +1832,20 @@ test_expect_success '7b: rename/rename(2to1), but only due to transitive rename' git cat-file -p :2:y/d >expect && git cat-file -p :3:y/d >other && >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge-file \ + -L "HEAD:y/d" \ + -L "" \ + -L "B^0:z/d" \ + expect empty other + else + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other + fi && test_cmp expect y/d ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> The ort merge strategy has some slight differences in commit descriptions (shortened hashes), stdout vs stderr, and in conflict messages. Also, builtin/merge.c reports usage of "ort" as "Merge made by the 'ort' strategy" -- while it is meant as a drop in replacement for "recursive" it is not yet treated as though it is recursive. Update the testcases to expect different output for the different merge backends. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6402-merge-rename.sh | 14 ++++++++++++-- t/t6437-submodule-merge.sh | 25 +++++++++++++++++++++---- t/t7602-merge-octopus-many.sh | 4 ++++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index 47d4434d64..3f64f62224 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -320,7 +320,12 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' ' test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output && test_i18ngrep "Auto-merging dir" output && - test_i18ngrep "Adding as dir~HEAD instead" output && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_i18ngrep "moving it to dir~HEAD instead" output + else + test_i18ngrep "Adding as dir~HEAD instead" output + fi && test 3 -eq "$(git ls-files -u | wc -l)" && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && @@ -342,7 +347,12 @@ test_expect_success 'Same as previous, but merged other way' ' ! grep "error: refusing to lose untracked file at" errors && test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output && test_i18ngrep "Auto-merging dir" output && - test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_i18ngrep "moving it to dir~renamed-file-has-no-conflicts instead" output + else + test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output + fi && test 3 -eq "$(git ls-files -u | wc -l)" && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh index 6a1e5f8232..3ead2b726f 100755 --- a/t/t6437-submodule-merge.sh +++ b/t/t6437-submodule-merge.sh @@ -127,7 +127,12 @@ test_expect_success 'merging should conflict for non fast-forward' ' git checkout -b test-nonforward b && (cd sub && git rev-parse sub-d > ../expect) && - test_must_fail git merge c 2> actual && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge c >actual + else + test_must_fail git merge c 2> actual + fi && grep $(cat expect) actual > /dev/null && git reset --hard) ' @@ -138,9 +143,21 @@ test_expect_success 'merging should fail for ambiguous common parent' ' (cd sub && git checkout -b ambiguous sub-b && git merge sub-c && - git rev-parse sub-d > ../expect1 && - git rev-parse ambiguous > ../expect2) && - test_must_fail git merge c 2> actual && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rev-parse --short sub-d >../expect1 && + git rev-parse --short ambiguous >../expect2 + else + git rev-parse sub-d > ../expect1 && + git rev-parse ambiguous > ../expect2 + fi + ) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge c >actual + else + test_must_fail git merge c 2> actual + fi && grep $(cat expect1) actual > /dev/null && grep $(cat expect2) actual > /dev/null && git reset --hard) diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh index 6abe441ae3..b205b5619e 100755 --- a/t/t7602-merge-octopus-many.sh +++ b/t/t7602-merge-octopus-many.sh @@ -77,6 +77,10 @@ Merge made by the 'recursive' strategy. EOF test_expect_success 'merge reduces irrelevant remote heads' ' + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + sed -i'' s/recursive/ort/ expected + fi && GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual && test_i18ncmp expected actual ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> The "recursive" backend relies on unpack_trees() to check if unstaged changes would be overwritten by a merge, but unpack_trees() does not understand renames -- and once it returns, it has already written many updates to the working tree and index. As such, "recursive" had to do a special 4-way merge where it would need to also treat the working copy as an extra source of differences that we had to carefully avoid overwriting and resulting in moving files to new locations to avoid conflicts. The "ort" backend, by contrast, does the complete merge inmemory, and only updates the index and working copy as a post-processing step. If there are dirty files in the way, it can simply abort the merge. Update t6423 and t6436 to reflect the better merge abilities and expectations we have for ort, while still leaving the best-case-as-good-as-recursive-can-do expectations there for the recursive backend so we retain its stability until we are ready to deprecate and remove it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 266 ++++++++++++++++------------ t/t6436-merge-overwrite.sh | 18 +- 2 files changed, 165 insertions(+), 119 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 7e32626913..32e6925ca4 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -3634,28 +3634,35 @@ test_expect_success '11a: Avoid losing dirty contents with simple rename' ' echo stuff >>z/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "Refusing to lose dirty file at z/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "Refusing to lose dirty file at z/c" out && - test_seq 1 10 >expected && - echo stuff >>expected && - test_cmp expected z/c && + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 3 out && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 4 out && + git rev-parse >actual \ + :0:z/a :2:z/c && + git rev-parse >expect \ + O:z/a B:z/b && + test_cmp expect actual && - git rev-parse >actual \ - :0:z/a :2:z/c && - git rev-parse >expect \ - O:z/a B:z/b && - test_cmp expect actual && + git hash-object z/c~HEAD >actual && + git rev-parse B:z/b >expect && + test_cmp expect actual + fi && + + test_seq 1 10 >expected && + echo stuff >>expected && + test_cmp expected z/c - git hash-object z/c~HEAD >actual && - git rev-parse B:z/b >expect && - test_cmp expect actual ) ' @@ -3706,32 +3713,39 @@ test_expect_success '11b: Avoid losing dirty file involved in directory rename' git checkout A^0 && echo stuff >>z/c && - git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "Refusing to lose dirty file at z/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && + test_i18ngrep "Refusing to lose dirty file at z/c" out && - grep -q stuff z/c && - test_seq 1 10 >expected && - echo stuff >>expected && - test_cmp expected z/c && + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -m >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out && - git ls-files -s >out && - test_line_count = 3 out && - git ls-files -u >out && - test_line_count = 0 out && - git ls-files -m >out && - test_line_count = 0 out && - git ls-files -o >out && - test_line_count = 4 out && + git rev-parse >actual \ + :0:x/b :0:y/a :0:y/c && + git rev-parse >expect \ + O:x/b O:z/a B:x/c && + test_cmp expect actual && - git rev-parse >actual \ - :0:x/b :0:y/a :0:y/c && - git rev-parse >expect \ - O:x/b O:z/a B:x/c && - test_cmp expect actual && + git hash-object y/c >actual && + git rev-parse B:x/c >expect && + test_cmp expect actual + fi && - git hash-object y/c >actual && - git rev-parse B:x/c >expect && - test_cmp expect actual + grep -q stuff z/c && + test_seq 1 10 >expected && + echo stuff >>expected && + test_cmp expected z/c ) ' @@ -3783,7 +3797,13 @@ test_expect_success '11c: Avoid losing not-uptodate with rename + D/F conflict' echo stuff >>y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "following files would be overwritten by merge" err && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "following files would be overwritten by merge" err + fi && grep -q stuff y/c && test_seq 1 10 >expected && @@ -3851,29 +3871,35 @@ test_expect_success '11d: Avoid losing not-uptodate with rename + D/F conflict' echo stuff >>z/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "Refusing to lose dirty file at z/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "Refusing to lose dirty file at z/c" out && - grep -q stuff z/c && - test_seq 1 10 >expected && - echo stuff >>expected && - test_cmp expected z/c && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 4 out && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 5 out && + git rev-parse >actual \ + :0:x/b :0:y/a :0:y/c/d :3:y/c && + git rev-parse >expect \ + O:x/b O:z/a B:y/c/d B:x/c && + test_cmp expect actual && - git rev-parse >actual \ - :0:x/b :0:y/a :0:y/c/d :3:y/c && - git rev-parse >expect \ - O:x/b O:z/a B:y/c/d B:x/c && - test_cmp expect actual && + git hash-object y/c~HEAD >actual && + git rev-parse B:x/c >expect && + test_cmp expect actual + fi && - git hash-object y/c~HEAD >actual && - git rev-parse B:x/c >expect && - test_cmp expect actual + grep -q stuff z/c && + test_seq 1 10 >expected && + echo stuff >>expected && + test_cmp expected z/c ) ' @@ -3931,37 +3957,43 @@ test_expect_success '11e: Avoid deleting not-uptodate with dir rename/rename(1to echo mods >>y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose dirty file at y/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose dirty file at y/c" out && - git ls-files -s >out && - test_line_count = 7 out && - git ls-files -u >out && - test_line_count = 4 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 7 out && + git ls-files -u >out && + test_line_count = 4 out && + git ls-files -o >out && + test_line_count = 3 out && + + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c && + test_cmp expect actual && + + # See if y/c~merged has expected contents; requires manually + # doing the expected file merge + git cat-file -p A:y/c >c1 && + git cat-file -p B:z/c >c2 && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + c1 empty c2 && + test_cmp c1 y/c~merged + fi && echo different >expected && echo mods >>expected && - test_cmp expected y/c && - - git rev-parse >actual \ - :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c && - test_cmp expect actual && - - # See if y/c~merged has expected contents; requires manually - # doing the expected file merge - git cat-file -p A:y/c >c1 && - git cat-file -p B:z/c >c2 && - >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - c1 empty c2 && - test_cmp c1 y/c~merged + test_cmp expected y/c ) ' @@ -4014,38 +4046,44 @@ test_expect_success '11f: Avoid deleting not-uptodate with dir rename/rename(2to echo important >>y/wham && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose dirty file at y/wham" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose dirty file at y/wham" out && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 2 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 3 out && - test_seq 1 10 >expected && - echo important >>expected && - test_cmp expected y/wham && + test_must_fail git rev-parse :1:y/wham && - test_must_fail git rev-parse :1:y/wham && + git rev-parse >actual \ + :0:y/a :0:y/b :2:y/wham :3:y/wham && + git rev-parse >expect \ + O:z/a O:z/b O:x/c O:x/d && + test_cmp expect actual && - git rev-parse >actual \ - :0:y/a :0:y/b :2:y/wham :3:y/wham && - git rev-parse >expect \ - O:z/a O:z/b O:x/c O:x/d && - test_cmp expect actual && + # Test that two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged + fi && - # Test that the two-way merge in y/wham~merged is as expected - git cat-file -p :2:y/wham >expect && - git cat-file -p :3:y/wham >other && - >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && - test_cmp expect y/wham~merged + test_seq 1 10 >expected && + echo important >>expected && + test_cmp expected y/wham ) ' diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh index dd8ab7ede1..dd9376842f 100755 --- a/t/t6436-merge-overwrite.sh +++ b/t/t6436-merge-overwrite.sh @@ -97,11 +97,19 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' ' git mv c1.c other.c && git commit -m rename && cp important other.c && - test_must_fail git merge c1a >out && - test_i18ngrep "Refusing to lose dirty file at other.c" out && - test_path_is_file other.c~HEAD && - test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) && - test_cmp important other.c + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge c1a >out 2>err && + test_i18ngrep "would be overwritten by merge" err && + test_cmp important other.c && + test_path_is_missing .git/MERGE_HEAD + else + test_must_fail git merge c1a >out && + test_i18ngrep "Refusing to lose dirty file at other.c" out && + test_path_is_file other.c~HEAD && + test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) && + test_cmp important other.c + fi ' test_expect_success 'will not overwrite untracked subtree' ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> Similar to the previous commit, since the "recursive" backend relies on unpack_trees() to check if unstaged or untracked files would be overwritten by a merge, and unpack_trees() does not understand renames -- it has false positives and false negatives. Once it has run, since it updates as it goes, merge-recursive then has to handle completing the merge as best it can despite extra changes in the working copy. However, this is not just an issue for dirty files, but also for untracked files because directory renames can cause file contents to need to be written to a location that was not tracked on either side of history. Since the "ort" backend does the complete merge inmemory, and only updates the index and working copy as a post-processing step, if there are untracked files in the way it can simply abort the merge much like checkout does. Update t6423 to reflect the better merge abilities and expectations for ort, while still leaving the best-case-as-good-as-recursive-can-do expectations there for the recursive backend so we retain its stability until we are ready to deprecate and remove it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 195 ++++++++++++++++++---------- 1 file changed, 124 insertions(+), 71 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 32e6925ca4..db694a6316 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -3214,6 +3214,7 @@ test_expect_success '10a: Overwrite untracked with normal rename/delete' ' echo important >z/d && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && + test_path_is_missing .git/MERGE_HEAD && test_i18ngrep "The following untracked working tree files would be overwritten by merge" err && git ls-files -s >out && @@ -3283,21 +3284,34 @@ test_expect_success '10b: Overwrite untracked with dir rename + delete' ' echo contents >y/e && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out && - test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 3 out && - git ls-files -u >out && - test_line_count = 2 out && - git ls-files -o >out && - test_line_count = 5 out && + git ls-files -s >out && + test_line_count = 1 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 5 out + else + test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out && + test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out && - git rev-parse >actual \ - :0:y/b :3:y/d :3:y/e && - git rev-parse >expect \ - O:z/b O:z/c B:z/e && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 5 out && + + git rev-parse >actual \ + :0:y/b :3:y/d :3:y/e && + git rev-parse >expect \ + O:z/b O:z/c B:z/e && + test_cmp expect actual + fi && echo very >expect && test_cmp expect y/c && @@ -3360,25 +3374,38 @@ test_expect_success '10c1: Overwrite untracked with dir rename/rename(1to2)' ' echo important >y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 6 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out && - git rev-parse >actual \ - :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 3 out && - git hash-object y/c~B^0 >actual && - git rev-parse O:x/c >expect && - test_cmp expect actual && + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && + test_cmp expect actual && + + git hash-object y/c~B^0 >actual && + git rev-parse O:x/c >expect && + test_cmp expect actual + fi && echo important >expect && test_cmp expect y/c @@ -3398,25 +3425,38 @@ test_expect_success '10c2: Overwrite untracked with dir rename/rename(1to2), oth echo important >y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive A^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 6 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out && - git rev-parse >actual \ - :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 3 out && - git hash-object y/c~HEAD >actual && - git rev-parse O:x/c >expect && - test_cmp expect actual && + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && + test_cmp expect actual && + + git hash-object y/c~HEAD >actual && + git rev-parse O:x/c >expect && + test_cmp expect actual + fi && echo important >expect && test_cmp expect y/c @@ -3474,37 +3514,50 @@ test_expect_success '10d: Delete untracked with dir rename/rename(2to1)' ' echo important >y/wham && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose untracked file at y/wham" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 6 out && - git ls-files -u >out && - test_line_count = 2 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/wham" out && - git rev-parse >actual \ - :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 3 out && + + git rev-parse >actual \ + :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f && + test_cmp expect actual && - test_must_fail git rev-parse :1:y/wham && + test_must_fail git rev-parse :1:y/wham && - echo important >expect && - test_cmp expect y/wham && + # Test that two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged + fi && - # Test that the two-way merge in y/wham~merged is as expected - git cat-file -p :2:y/wham >expect && - git cat-file -p :3:y/wham >other && - >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && - test_cmp expect y/wham~merged + echo important >expect && + test_cmp expect y/wham ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 37 ++++++++++++++++------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index db694a6316..9e51320ee0 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -4725,20 +4725,22 @@ test_expect_success '12e: Rename/merge subdir into the root, variant 2' ' # folder/subdir/newsubdir/e_Merge2 # folder/subdir/tweaked/{h,Makefile_SUB_Merge1,newfile.py} # folder/unchanged/<LOTS OF FILES> -# -# Notes: This testcase happens to exercise lots of the -# optimization-specific codepaths in merge-ort, and also -# demonstrated a failing of the directory rename detection algorithm -# in merge-recursive; newfile.c should not get pushed into -# folder/subdir/newsubdir/, yet merge-recursive put it there because -# the rename of dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d} -# looks like -# dir/ -> folder/, -# whereas the rename of dir/subdir/e -> folder/subdir/newsubdir/e -# looks like -# dir/subdir/ -> folder/subdir/newsubdir/ -# and if we note that newfile.c is found in dir/subdir/, we might -# overlook the dir/ -> folder/ rule that has more weight. +# Things being checked here: +# 1. dir/subdir/newfile.c does not get pushed into folder/subdir/newsubdir/. +# dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d} looks like +# dir/ -> folder/, +# whereas dir/subdir/e -> folder/subdir/newsubdir/e looks like +# dir/subdir/ -> folder/subdir/newsubdir/ +# and if we note that newfile.c is found in dir/subdir/, we might overlook +# the dir/ -> folder/ rule that has more weight. Older git versions did +# this. +# 2. The code to do trivial directory resolves. Note that +# dir/subdir/unchanged/ is unchanged and can be deleted, and files in the +# new folder/subdir/unchanged/ are not needed as a target to any renames. +# Thus, in the second collect_merge_info_callback() we can just resolve +# these two directories trivially without recursing.) +# 3. Exercising the codepaths for caching renames and deletes from one cherry +# pick and re-applying them in the subsequent one. test_setup_12f () { test_create_repo 12f && @@ -4803,7 +4805,7 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac git checkout A^0 && git branch Bmod B && - git -c merge.directoryRenames=true rebase A Bmod && + GIT_TRACE2_PERF="$(pwd)/trace.output" git -c merge.directoryRenames=true rebase A Bmod && echo Checking the pick of B1... && @@ -4884,7 +4886,10 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac test_seq 2 12 >e_Merge2 && git hash-object e_Merge2 >expect && git rev-parse Bmod:folder/subdir/newsubdir/e >actual && - test_cmp expect actual + test_cmp expect actual && + + grep region_enter.*collect_merge_info trace.output >collect && + test_line_count = 4 collect ) ' -- gitgitgadget
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Elijah Newren <newren@gmail.com> > > There are a number of tests that the "recursive" backend does not handle > correctly but which the redesign in "ort" will. Add a new helper in > lib-merge.sh for selecting a different test expectation based on the > setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to > document which ones we expect to fail under recursive but pass under > ort. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > t/lib-merge.sh | 15 +++++++++++++++ > t/t6416-recursive-corner-cases.sh | 11 ++++++----- > t/t6422-merge-rename-corner-cases.sh | 7 ++++--- > t/t6423-merge-rename-directories.sh | 13 +++++++------ > t/t6426-merge-skip-unneeded-updates.sh | 3 ++- > t/t6430-merge-recursive.sh | 3 ++- > 6 files changed, 36 insertions(+), 16 deletions(-) > create mode 100644 t/lib-merge.sh > > diff --git a/t/lib-merge.sh b/t/lib-merge.sh > new file mode 100644 > index 0000000000..fac2bc5919 > --- /dev/null > +++ b/t/lib-merge.sh > @@ -0,0 +1,15 @@ > +# Helper functions used by merge tests. > + > +test_expect_merge_algorithm () { > + status_for_recursive=$1 > + shift > + status_for_ort=$1 > + shift Smaller than minor, but I'd find status_for_recursive=$1 status_for_ort=$2 shift 2 easier to see that which one is for which by matching the order in which the calling sites, e.g. test_expect_merge_algorithm success failure \ here comes the commands being tested lists them. > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort > + then > + test_expect_${status_for_ort} "$@" > + else > + test_expect_${status_for_recursive} "$@" > + fi I expect this to be purely transitory, so it is fine. If not, something along the lines of ... eval test_expect='$'status_for_"$GIT_TEST_MERGE_ALGORITHM" $test_expect "$@" ... might be what I would suggest, though ;-). And the users are just too pleasant to see, with full of "failure sucess", which is the second best outcome we want to see ;-) > +test_expect_merge_algorithm failure success 'check symlink mo... > +test_expect_merge_algorithm failure success 'check symlink ad... > +test_expect_merge_algorithm failure success 'check submodule ... > +test_expect_merge_algorithm failure success 'check submodule ... > +test_expect_merge_algorithm failure success 'check conflictin... > +test_expect_merge_algorithm failure success 'rad-check: renam... > +test_expect_merge_algorithm failure success 'rrdd-check: rena... > +test_expect_merge_algorithm failure success 'mod6-check: chai... > +test_expect_merge_algorithm failure success '6b1: Same rename... > +test_expect_merge_algorithm failure success '6b2: Same rename... > +test_expect_merge_algorithm failure success '10e: Does git co... > +test_expect_merge_algorithm failure success '12b1: Moving two... > +test_expect_merge_algorithm failure success '12c1: Moving one... > +test_expect_merge_algorithm failure success '12f: Trivial dir... > +test_expect_merge_algorithm failure success '4a: Change on A,... > +test_expect_merge_algorithm failure success 'merge-recursive ...
On Fri, Oct 23, 2020 at 9:48 AM Junio C Hamano <gitster@pobox.com> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: Elijah Newren <newren@gmail.com> > > > > There are a number of tests that the "recursive" backend does not handle > > correctly but which the redesign in "ort" will. Add a new helper in > > lib-merge.sh for selecting a different test expectation based on the > > setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to > > document which ones we expect to fail under recursive but pass under > > ort. > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > t/lib-merge.sh | 15 +++++++++++++++ > > t/t6416-recursive-corner-cases.sh | 11 ++++++----- > > t/t6422-merge-rename-corner-cases.sh | 7 ++++--- > > t/t6423-merge-rename-directories.sh | 13 +++++++------ > > t/t6426-merge-skip-unneeded-updates.sh | 3 ++- > > t/t6430-merge-recursive.sh | 3 ++- > > 6 files changed, 36 insertions(+), 16 deletions(-) > > create mode 100644 t/lib-merge.sh > > > > diff --git a/t/lib-merge.sh b/t/lib-merge.sh > > new file mode 100644 > > index 0000000000..fac2bc5919 > > --- /dev/null > > +++ b/t/lib-merge.sh > > @@ -0,0 +1,15 @@ > > +# Helper functions used by merge tests. > > + > > +test_expect_merge_algorithm () { > > + status_for_recursive=$1 > > + shift > > + status_for_ort=$1 > > + shift > > Smaller than minor, but I'd find > > status_for_recursive=$1 status_for_ort=$2 > shift 2 > > easier to see that which one is for which by matching the order in > which the calling sites, e.g. > > test_expect_merge_algorithm success failure \ > here comes the commands being tested > > lists them. I can fix that up. > > > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort > > + then > > + test_expect_${status_for_ort} "$@" > > + else > > + test_expect_${status_for_recursive} "$@" > > + fi > > I expect this to be purely transitory, so it is fine. If not, > something along the lines of ... > > eval test_expect='$'status_for_"$GIT_TEST_MERGE_ALGORITHM" > $test_expect "$@" > > ... might be what I would suggest, though ;-). I also expect it to be transitory, but how long that transition is depends more on how long it takes for others to become comfortable with removing merge-recursive.c once merge-ort.c is in place. I suspect at a minimum it's a cycle or two after ort becomes the default (which at a minimum would come a cycle or two after ort is finished and available). If for some reason folks don't eventually become comfortable with removing merge-recursive, then we might need to revisit. > And the users are just too pleasant to see, with full of "failure > sucess", which is the second best outcome we want to see ;-) > > > +test_expect_merge_algorithm failure success 'check symlink mo... > > +test_expect_merge_algorithm failure success 'check symlink ad... > > +test_expect_merge_algorithm failure success 'check submodule ... > > +test_expect_merge_algorithm failure success 'check submodule ... > > +test_expect_merge_algorithm failure success 'check conflictin... > > +test_expect_merge_algorithm failure success 'rad-check: renam... > > +test_expect_merge_algorithm failure success 'rrdd-check: rena... > > +test_expect_merge_algorithm failure success 'mod6-check: chai... > > +test_expect_merge_algorithm failure success '6b1: Same rename... > > +test_expect_merge_algorithm failure success '6b2: Same rename... > > +test_expect_merge_algorithm failure success '10e: Does git co... > > +test_expect_merge_algorithm failure success '12b1: Moving two... > > +test_expect_merge_algorithm failure success '12c1: Moving one... > > +test_expect_merge_algorithm failure success '12f: Trivial dir... > > +test_expect_merge_algorithm failure success '4a: Change on A,... > > +test_expect_merge_algorithm failure success 'merge-recursive ... :-)
On Fri, Oct 23, 2020 at 9:01 AM Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Elijah Newren <newren@gmail.com> > > 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 <newren@gmail.com> Perhaps I should also mention that these changes to directory/file conflict handling were discussed previously in the thread at https://lore.kernel.org/git/xmqqbmabcuhf.fsf@gitster-ct.c.googlers.com/. I just never got around to a complete implementation within merge-recursive.c, but did implement it in merge-ort.c I still haven't gotten around to fixing up git-mv and git-rm as suggested by Junio in that thread; but at least I've finally gotten the merge machinery side written...
One more comment... On Fri, Oct 23, 2020 at 10:25 AM Elijah Newren <newren@gmail.com> wrote: > > On Fri, Oct 23, 2020 at 9:48 AM Junio C Hamano <gitster@pobox.com> wrote: > > > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > > And the users are just too pleasant to see, with full of "failure > > sucess", which is the second best outcome we want to see ;-) > > > > > +test_expect_merge_algorithm failure success 'check symlink mo... > > > +test_expect_merge_algorithm failure success 'check symlink ad... > > > +test_expect_merge_algorithm failure success 'check submodule ... > > > +test_expect_merge_algorithm failure success 'check submodule ... > > > +test_expect_merge_algorithm failure success 'check conflictin... > > > +test_expect_merge_algorithm failure success 'rad-check: renam... > > > +test_expect_merge_algorithm failure success 'rrdd-check: rena... > > > +test_expect_merge_algorithm failure success 'mod6-check: chai... > > > +test_expect_merge_algorithm failure success '6b1: Same rename... > > > +test_expect_merge_algorithm failure success '6b2: Same rename... > > > +test_expect_merge_algorithm failure success '10e: Does git co... > > > +test_expect_merge_algorithm failure success '12b1: Moving two... > > > +test_expect_merge_algorithm failure success '12c1: Moving one... > > > +test_expect_merge_algorithm failure success '12f: Trivial dir... > > > +test_expect_merge_algorithm failure success '4a: Change on A,... > > > +test_expect_merge_algorithm failure success 'merge-recursive ... > > :-) Actually, there are another 12 submodule-related tests that pass under ort but not under recursive, spread across t3512, t3513, t5572, t6437, and t6438. I didn't (yet) apply the same change there, so they all show up as "TODO passed" if you check out the 'ort' branch of my repo and run the tests with GIT_TEST_MERGE_ALGORITHM=ort. I delayed marking them as expecting success under ort because I suspect that nearby tests should also pass but are just coded too stringently. (For example, perhaps they expected a directory/submodule conflict to result in all files within the conflicting directory to be renamed out of the way instead of expecting the submodule to be moved aside -- moving the submodule aside results in massively less rename handling pressure and is an easier way to make sure that the files under the conflicting directory aren't written into and over entries within the submodule.) I was hoping to get a submodule expert to look over those tests and provide some opinions...
On Fri, Oct 23, 2020 at 9:01 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> t/t6423-merge-rename-directories.sh | 37 ++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
Whoops, I had meant for this last patch to be kept local for now and
only send it in after adding more of the merge-ort implementation. To
make matters worse, it's slightly out of date with what I have on my
'ort' branch. Oh, well, I guess it doesn't hurt...at least not if I
get the up-to-date version of the changes. I'll include those in v2,
along with the small change Junio highlighted for patch 1, but wait a
few days for more feedback to come in on this series.
On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote: > +test_expect_merge_algorithm () { > + status_for_recursive=$1 > + shift > + status_for_ort=$1 > + shift > + > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort > + then > + test_expect_${status_for_ort} "$@" > + else > + test_expect_${status_for_recursive} "$@" > -test_expect_failure 'check symlink modify/modify' ' > +test_expect_merge_algorithm failure success 'check symlink modify/modify' ' I find this series of "failure success" hard to decode without understanding what it would be, then I need to keep rememberring which status is corresponding with with algorithm. Perhaps this patch is a bit easier to read. This is largely based on your patch. (I haven't read other patches, yet). What do you think? -------------8<------------ From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= <congdanhqx@gmail.com> Date: Sat, 24 Oct 2020 17:41:02 +0700 Subject: [PATCH] t/: new helper for testing merge that allow failure for some algorithm There are a number of tests that the "recursive" backend does not handle correctly but which the redesign in "ort" will. Add a new helper in lib-merge.sh for selecting a different test expectation based on the setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to document that we expect them to be success by default but failure with certain algorithm. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/lib-merge.sh | 19 +++++++++++++++++++ t/t6416-recursive-corner-cases.sh | 11 ++++++----- t/t6422-merge-rename-corner-cases.sh | 7 ++++--- t/t6423-merge-rename-directories.sh | 13 +++++++------ t/t6426-merge-skip-unneeded-updates.sh | 3 ++- t/t6430-merge-recursive.sh | 3 ++- 6 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 t/lib-merge.sh diff --git a/t/lib-merge.sh b/t/lib-merge.sh new file mode 100644 index 0000000000..efd8b9615c --- /dev/null +++ b/t/lib-merge.sh @@ -0,0 +1,19 @@ +# Helper functions used by merge tests. + +test_expect_merge_success() { + exception="$1" + : "${GIT_TEST_MERGE_ALGORITHM:=recursive}" + case ",$exception," in + *,$GIT_TEST_MERGE_ALGORITHM=failure,*) + shift + test_expect_failure "$@" ;; + *,$GIT_TEST_MERGE_ALGORITHM=*) + BUG "exception must be failure only" ;; + *=failure,) + shift + test_expect_success "$@" ;; + *) + # No exception + test_expect_success "$@" ;; + esac +} diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index fd98989b14..d13c1afb4a 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -3,6 +3,7 @@ test_description='recursive merge corner cases involving criss-cross merges' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh # # L1 L2 @@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' ' ) ' -test_expect_failure 'check symlink modify/modify' ' +test_expect_merge_success recursive=failure 'check symlink modify/modify' ' ( cd symlink-modify-modify && @@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' ' ) ' -test_expect_failure 'check symlink add/add' ' +test_expect_merge_success recursive=failure 'check symlink add/add' ' ( cd symlink-add-add && @@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' ' ) ' -test_expect_failure 'check submodule modify/modify' ' +test_expect_merge_success recursive=failure 'check submodule modify/modify' ' ( cd submodule-modify-modify && @@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' ' ) ' -test_expect_failure 'check submodule add/add' ' +test_expect_merge_success recursive=failure 'check submodule add/add' ' ( cd submodule-add-add && @@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' ' ) ' -test_expect_failure 'check conflicting entry types (submodule vs symlink)' ' +test_expect_merge_success recursive=failure 'check conflicting entry types (submodule vs symlink)' ' ( cd submodule-symlink-add-add && diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh index 3375eaf4e7..0c8eb4df8a 100755 --- a/t/t6422-merge-rename-corner-cases.sh +++ b/t/t6422-merge-rename-corner-cases.sh @@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses" # t6036 has corner cases that involve both criss-cross merges and renames . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh test_setup_rename_delete_untracked () { test_create_repo rename-delete-untracked && @@ -878,7 +879,7 @@ test_setup_rad () { ) } -test_expect_failure 'rad-check: rename/add/delete conflict' ' +test_expect_merge_success recursive=failure 'rad-check: rename/add/delete conflict' ' test_setup_rad && ( cd rad && @@ -951,7 +952,7 @@ test_setup_rrdd () { ) } -test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' +test_expect_merge_success recursive=failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' test_setup_rrdd && ( cd rrdd && @@ -1040,7 +1041,7 @@ test_setup_mod6 () { ) } -test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' +test_expect_merge_success recursive=failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' test_setup_mod6 && ( cd mod6 && diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 06b46af765..249fbb6853 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -26,6 +26,7 @@ test_description="recursive merge with directory renames" # files that might be renamed into each other's paths.) . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh ########################################################################### @@ -1339,7 +1340,7 @@ test_setup_6b1 () { ) } -test_expect_failure '6b1: Same renames done on both sides, plus another rename' ' +test_expect_merge_success recursive=failure '6b1: Same renames done on both sides, plus another rename' ' test_setup_6b1 && ( cd 6b1 && @@ -1412,7 +1413,7 @@ test_setup_6b2 () { ) } -test_expect_failure '6b2: Same rename done on both sides' ' +test_expect_merge_success recursive=failure '6b2: Same rename done on both sides' ' test_setup_6b2 && ( cd 6b2 && @@ -3471,7 +3472,7 @@ test_setup_10e () { ) } -test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' ' +test_expect_merge_success recursive=failure '10e: Does git complain about untracked file that is not really in the way?' ' test_setup_10e && ( cd 10e && @@ -4104,7 +4105,7 @@ test_setup_12b1 () { ) } -test_expect_failure '12b1: Moving two directory hierarchies into each other' ' +test_expect_merge_success recursive=failure '12b1: Moving two directory hierarchies into each other' ' test_setup_12b1 && ( cd 12b1 && @@ -4272,7 +4273,7 @@ test_setup_12c1 () { ) } -test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' ' +test_expect_merge_success recursive=failure '12c1: Moving one directory hierarchy into another w/ content merge' ' test_setup_12c1 && ( cd 12c1 && @@ -4632,7 +4633,7 @@ test_setup_12f () { ) } -test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' ' +test_expect_merge_success recursive=failure '12f: Trivial directory resolve, caching, all kinds of fun' ' test_setup_12f && ( cd 12f && diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh index 699813671c..8510d4da8b 100755 --- a/t/t6426-merge-skip-unneeded-updates.sh +++ b/t/t6426-merge-skip-unneeded-updates.sh @@ -23,6 +23,7 @@ test_description="merge cases" # files that might be renamed into each other's paths.) . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh ########################################################################### @@ -666,7 +667,7 @@ test_setup_4a () { # correct requires doing the merge in-memory first, then realizing that no # updates to the file are necessary, and thus that we can just leave the path # alone. -test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' ' +test_expect_merge_success recursive=failure '4a: Change on A, change on B subset of A, dirty mods present' ' test_setup_4a && ( cd 4a && diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh index a328260d42..4795a7abd0 100755 --- a/t/t6430-merge-recursive.sh +++ b/t/t6430-merge-recursive.sh @@ -3,6 +3,7 @@ test_description='merge-recursive backend test' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh test_expect_success 'setup 1' ' @@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' ' test_cmp expected actual ' -test_expect_failure 'merge-recursive rename vs. rename/symlink' ' +test_expect_merge_success recursive=failure 'merge-recursive rename vs. rename/symlink' ' git checkout -f rename && git merge rename-ln && -- 2.29.0.rc1
On Fri, Oct 23, 2020 at 9:01 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
> index 6abe441ae3..b205b5619e 100755
> --- a/t/t7602-merge-octopus-many.sh
> +++ b/t/t7602-merge-octopus-many.sh
> @@ -77,6 +77,10 @@ Merge made by the 'recursive' strategy.
> EOF
>
> test_expect_success 'merge reduces irrelevant remote heads' '
> + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> + then
> + sed -i'' s/recursive/ort/ expected
> + fi &&
> GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
> test_i18ncmp expected actual
> '
I've got a portability fix for this local hunk that I'll include along
with fixes for other feedback. (I could have sworn I had this fixed
before I sent it out, but I guess I sent an older version somehow?)
On Sat, Oct 24, 2020 at 3:49 AM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote: > > On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote: > > +test_expect_merge_algorithm () { > > + status_for_recursive=$1 > > + shift > > + status_for_ort=$1 > > + shift > > + > > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort > > + then > > + test_expect_${status_for_ort} "$@" > > + else > > + test_expect_${status_for_recursive} "$@" > > -test_expect_failure 'check symlink modify/modify' ' > > +test_expect_merge_algorithm failure success 'check symlink modify/modify' ' > > I find this series of "failure success" hard to decode without > understanding what it would be, then I need to keep rememberring which > status is corresponding with with algorithm. > > Perhaps this patch is a bit easier to read. This is largely based on > your patch. (I haven't read other patches, yet). > > What do you think? It is easier to read and I think something along these lines would make a lot of sense if this weren't a transient change (the idea is to eventually drop the recursive backend in favor of ort, and then these can all switch to just using test_expect_success). Maybe it still makes sense to make further changes here anyway, but if we do go this route, there are 1-2 things we can/should change: First, while a lot of my contributions aren't that important, and the new test_expect_* function certainly falls in that category, one of the driving goals behind a new merge algorithm was fixing up edge and corner cases that were just too problematic in the recursive backend. Thus, the patch where I get to flip the test expectation is one that I care about more than most out of the (I'm guessing on this number) 100+ patches that will be part of this new merge algorithm. Having you take over ownership of that patch thus isn't right; we should instead keep my original patch and apply your suggested changes on top (or have a patch from you introducing a new function first, and then have a patch from me using it to flip test expectations on top). Second, I think that lines like test_expect_merge_success recursive=failure ... read like a contradiction and are also confusing. I think it'd be better if it read something like test_expect_merge recursive=failure ort=success ... or something along those lines. > -------------8<------------ > From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= > <congdanhqx@gmail.com> > Date: Sat, 24 Oct 2020 17:41:02 +0700 > Subject: [PATCH] t/: new helper for testing merge that allow failure for some > algorithm > > There are a number of tests that the "recursive" backend does not handle > correctly but which the redesign in "ort" will. > > Add a new helper in lib-merge.sh for selecting a different test expectation > based on the setting of GIT_TEST_MERGE_ALGORITHM, and use it in various > testcases to document that we expect them to be success by default > but failure with certain algorithm. > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > t/lib-merge.sh | 19 +++++++++++++++++++ > t/t6416-recursive-corner-cases.sh | 11 ++++++----- > t/t6422-merge-rename-corner-cases.sh | 7 ++++--- > t/t6423-merge-rename-directories.sh | 13 +++++++------ > t/t6426-merge-skip-unneeded-updates.sh | 3 ++- > t/t6430-merge-recursive.sh | 3 ++- > 6 files changed, 40 insertions(+), 16 deletions(-) > create mode 100644 t/lib-merge.sh > > diff --git a/t/lib-merge.sh b/t/lib-merge.sh > new file mode 100644 > index 0000000000..efd8b9615c > --- /dev/null > +++ b/t/lib-merge.sh > @@ -0,0 +1,19 @@ > +# Helper functions used by merge tests. > + > +test_expect_merge_success() { > + exception="$1" > + : "${GIT_TEST_MERGE_ALGORITHM:=recursive}" > + case ",$exception," in > + *,$GIT_TEST_MERGE_ALGORITHM=failure,*) > + shift > + test_expect_failure "$@" ;; > + *,$GIT_TEST_MERGE_ALGORITHM=*) > + BUG "exception must be failure only" ;; > + *=failure,) > + shift > + test_expect_success "$@" ;; > + *) > + # No exception > + test_expect_success "$@" ;; > + esac > +} > diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh > index fd98989b14..d13c1afb4a 100755 > --- a/t/t6416-recursive-corner-cases.sh > +++ b/t/t6416-recursive-corner-cases.sh > @@ -3,6 +3,7 @@ > test_description='recursive merge corner cases involving criss-cross merges' > > . ./test-lib.sh > +. "$TEST_DIRECTORY"/lib-merge.sh > > # > # L1 L2 > @@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' ' > ) > ' > > -test_expect_failure 'check symlink modify/modify' ' > +test_expect_merge_success recursive=failure 'check symlink modify/modify' ' > ( > cd symlink-modify-modify && > > @@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' ' > ) > ' > > -test_expect_failure 'check symlink add/add' ' > +test_expect_merge_success recursive=failure 'check symlink add/add' ' > ( > cd symlink-add-add && > > @@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' ' > ) > ' > > -test_expect_failure 'check submodule modify/modify' ' > +test_expect_merge_success recursive=failure 'check submodule modify/modify' ' > ( > cd submodule-modify-modify && > > @@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' ' > ) > ' > > -test_expect_failure 'check submodule add/add' ' > +test_expect_merge_success recursive=failure 'check submodule add/add' ' > ( > cd submodule-add-add && > > @@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' ' > ) > ' > > -test_expect_failure 'check conflicting entry types (submodule vs symlink)' ' > +test_expect_merge_success recursive=failure 'check conflicting entry types (submodule vs symlink)' ' > ( > cd submodule-symlink-add-add && > > diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh > index 3375eaf4e7..0c8eb4df8a 100755 > --- a/t/t6422-merge-rename-corner-cases.sh > +++ b/t/t6422-merge-rename-corner-cases.sh > @@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses" > # t6036 has corner cases that involve both criss-cross merges and renames > > . ./test-lib.sh > +. "$TEST_DIRECTORY"/lib-merge.sh > > test_setup_rename_delete_untracked () { > test_create_repo rename-delete-untracked && > @@ -878,7 +879,7 @@ test_setup_rad () { > ) > } > > -test_expect_failure 'rad-check: rename/add/delete conflict' ' > +test_expect_merge_success recursive=failure 'rad-check: rename/add/delete conflict' ' > test_setup_rad && > ( > cd rad && > @@ -951,7 +952,7 @@ test_setup_rrdd () { > ) > } > > -test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' > +test_expect_merge_success recursive=failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' > test_setup_rrdd && > ( > cd rrdd && > @@ -1040,7 +1041,7 @@ test_setup_mod6 () { > ) > } > > -test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' > +test_expect_merge_success recursive=failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' > test_setup_mod6 && > ( > cd mod6 && > diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh > index 06b46af765..249fbb6853 100755 > --- a/t/t6423-merge-rename-directories.sh > +++ b/t/t6423-merge-rename-directories.sh > @@ -26,6 +26,7 @@ test_description="recursive merge with directory renames" > # files that might be renamed into each other's paths.) > > . ./test-lib.sh > +. "$TEST_DIRECTORY"/lib-merge.sh > > > ########################################################################### > @@ -1339,7 +1340,7 @@ test_setup_6b1 () { > ) > } > > -test_expect_failure '6b1: Same renames done on both sides, plus another rename' ' > +test_expect_merge_success recursive=failure '6b1: Same renames done on both sides, plus another rename' ' > test_setup_6b1 && > ( > cd 6b1 && > @@ -1412,7 +1413,7 @@ test_setup_6b2 () { > ) > } > > -test_expect_failure '6b2: Same rename done on both sides' ' > +test_expect_merge_success recursive=failure '6b2: Same rename done on both sides' ' > test_setup_6b2 && > ( > cd 6b2 && > @@ -3471,7 +3472,7 @@ test_setup_10e () { > ) > } > > -test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' ' > +test_expect_merge_success recursive=failure '10e: Does git complain about untracked file that is not really in the way?' ' > test_setup_10e && > ( > cd 10e && > @@ -4104,7 +4105,7 @@ test_setup_12b1 () { > ) > } > > -test_expect_failure '12b1: Moving two directory hierarchies into each other' ' > +test_expect_merge_success recursive=failure '12b1: Moving two directory hierarchies into each other' ' > test_setup_12b1 && > ( > cd 12b1 && > @@ -4272,7 +4273,7 @@ test_setup_12c1 () { > ) > } > > -test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' ' > +test_expect_merge_success recursive=failure '12c1: Moving one directory hierarchy into another w/ content merge' ' > test_setup_12c1 && > ( > cd 12c1 && > @@ -4632,7 +4633,7 @@ test_setup_12f () { > ) > } > > -test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' ' > +test_expect_merge_success recursive=failure '12f: Trivial directory resolve, caching, all kinds of fun' ' > test_setup_12f && > ( > cd 12f && > diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh > index 699813671c..8510d4da8b 100755 > --- a/t/t6426-merge-skip-unneeded-updates.sh > +++ b/t/t6426-merge-skip-unneeded-updates.sh > @@ -23,6 +23,7 @@ test_description="merge cases" > # files that might be renamed into each other's paths.) > > . ./test-lib.sh > +. "$TEST_DIRECTORY"/lib-merge.sh > > > ########################################################################### > @@ -666,7 +667,7 @@ test_setup_4a () { > # correct requires doing the merge in-memory first, then realizing that no > # updates to the file are necessary, and thus that we can just leave the path > # alone. > -test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' ' > +test_expect_merge_success recursive=failure '4a: Change on A, change on B subset of A, dirty mods present' ' > test_setup_4a && > ( > cd 4a && > diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh > index a328260d42..4795a7abd0 100755 > --- a/t/t6430-merge-recursive.sh > +++ b/t/t6430-merge-recursive.sh > @@ -3,6 +3,7 @@ > test_description='merge-recursive backend test' > > . ./test-lib.sh > +. "$TEST_DIRECTORY"/lib-merge.sh > > test_expect_success 'setup 1' ' > > @@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' ' > test_cmp expected actual > ' > > -test_expect_failure 'merge-recursive rename vs. rename/symlink' ' > +test_expect_merge_success recursive=failure 'merge-recursive rename vs. rename/symlink' ' > > git checkout -f rename && > git merge rename-ln && > -- > 2.29.0.rc1
On 2020-10-24 09:53:18-0700, Elijah Newren <newren@gmail.com> wrote: > On Sat, Oct 24, 2020 at 3:49 AM Đoàn Trần Công Danh > <congdanhqx@gmail.com> wrote: > > > > On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote: > > > +test_expect_merge_algorithm () { > > > + status_for_recursive=$1 > > > + shift > > > + status_for_ort=$1 > > > + shift > > > + > > > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort > > > + then > > > + test_expect_${status_for_ort} "$@" > > > + else > > > + test_expect_${status_for_recursive} "$@" > > > -test_expect_failure 'check symlink modify/modify' ' > > > +test_expect_merge_algorithm failure success 'check symlink modify/modify' ' > > > > I find this series of "failure success" hard to decode without > > understanding what it would be, then I need to keep rememberring which > > status is corresponding with with algorithm. > > > > Perhaps this patch is a bit easier to read. This is largely based on > > your patch. (I haven't read other patches, yet). > > > > What do you think? > > It is easier to read and I think something along these lines would > make a lot of sense if this weren't a transient change (the idea is to > eventually drop the recursive backend in favor of ort, and then these > can all switch to just using test_expect_success). Maybe it still > makes sense to make further changes here anyway, but if we do go this > route, there are 1-2 things we can/should change: > > First, while a lot of my contributions aren't that important, and the Mine aren't that important, either > new test_expect_* function certainly falls in that category, one of > the driving goals behind a new merge algorithm was fixing up edge and > corner cases that were just too problematic in the recursive backend. > Thus, the patch where I get to flip the test expectation is one that I > care about more than most out of the (I'm guessing on this number) Make sense. > 100+ patches that will be part of this new merge algorithm. Having > you take over ownership of that patch thus isn't right; we should > instead keep my original patch and apply your suggested changes on top > (or have a patch from you introducing a new function first, and then > have a patch from me using it to flip test expectations on top). You can take back the ownership, the patch was based on yours, anyway. I wrote like that since I need to rewrite part of the message to match with my changes ;) No need to generate extra noise of additional patch. > Second, I think that lines like > test_expect_merge_success recursive=failure ... > read like a contradiction and are also confusing. I think it'd be > better if it read something like > test_expect_merge recursive=failure ort=success ... > or something along those lines. When I wrote the patch, I was expecting something like test_expect_merge_success recursive=failure,other=failure ... in order to merge all algorithm into single parameters. How about something like: test_expect_merge_success exception=recursive,other ... Not that we have "other" algorithm to begin with. Thanks, -- Danh
On Sun, Oct 25, 2020 at 6:49 AM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote: > > On 2020-10-24 09:53:18-0700, Elijah Newren <newren@gmail.com> wrote: > > On Sat, Oct 24, 2020 at 3:49 AM Đoàn Trần Công Danh > > <congdanhqx@gmail.com> wrote: > > > > > > On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote: > > > > +test_expect_merge_algorithm () { > > > > + status_for_recursive=$1 > > > > + shift > > > > + status_for_ort=$1 > > > > + shift > > > > + > > > > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort > > > > + then > > > > + test_expect_${status_for_ort} "$@" > > > > + else > > > > + test_expect_${status_for_recursive} "$@" > > > > -test_expect_failure 'check symlink modify/modify' ' > > > > +test_expect_merge_algorithm failure success 'check symlink modify/modify' ' > > > > > > I find this series of "failure success" hard to decode without > > > understanding what it would be, then I need to keep rememberring which > > > status is corresponding with with algorithm. > > > > > > Perhaps this patch is a bit easier to read. This is largely based on > > > your patch. (I haven't read other patches, yet). > > > > > > What do you think? > > > > It is easier to read and I think something along these lines would > > make a lot of sense if this weren't a transient change (the idea is to > > eventually drop the recursive backend in favor of ort, and then these > > can all switch to just using test_expect_success). Maybe it still > > makes sense to make further changes here anyway, but if we do go this > > route, there are 1-2 things we can/should change: > > > > First, while a lot of my contributions aren't that important, and the > > Mine aren't that important, either > > > new test_expect_* function certainly falls in that category, one of > > the driving goals behind a new merge algorithm was fixing up edge and > > corner cases that were just too problematic in the recursive backend. > > Thus, the patch where I get to flip the test expectation is one that I > > care about more than most out of the (I'm guessing on this number) > > Make sense. > > > 100+ patches that will be part of this new merge algorithm. Having > > you take over ownership of that patch thus isn't right; we should > > instead keep my original patch and apply your suggested changes on top > > (or have a patch from you introducing a new function first, and then > > have a patch from me using it to flip test expectations on top). > > You can take back the ownership, the patch was based on yours, anyway. > > I wrote like that since I need to rewrite part of the message to match > with my changes ;) > > No need to generate extra noise of additional patch. Just to be clear, others have made suggestions like yours in the past where they've taken over ownership of a patch in a series and I've been totally fine with it. Your suggestion to do the same would have been fine here, but I'm just kinda attached to being able to flip the test expectation for these tests; I've been working towards it for a _long_ time. I think an extra patch, attributed to you, actually makes the most sense here. > > Second, I think that lines like > > test_expect_merge_success recursive=failure ... > > read like a contradiction and are also confusing. I think it'd be > > better if it read something like > > test_expect_merge recursive=failure ort=success ... > > or something along those lines. > > When I wrote the patch, I was expecting something like > > test_expect_merge_success recursive=failure,other=failure ... > > in order to merge all algorithm into single parameters. > > How about something like: > > test_expect_merge_success exception=recursive,other ... > > Not that we have "other" algorithm to begin with. Sure, sounds great. I wouldn't spend any time trying to make it work with a 3rd backend, though. The goal is to have two merge backends only long enough for people to become comfortable with the new backend and discover any unknown issues with it that we can fix, then we'll rip it out the old "recursive" backend and we'll translate any requests for "recursive" to mean "ort". We'll also rip the test_expect_merge_success() function out since it'll be unneeded (so efforts towards future proofing of that function will be wasted). Then years will go by before another merge backend comes along, if one ever does.
This patch series builds on top of en/dir-rename-tests. It can be applied independent of my "new merge-ort" API series I submitted a couple days ago[1], but this series uses the same environment variable as patch 4 of that series. As promised, here is a series that makes the testsuite changes needed to simultaneously support both merge backends, keyed off a GIT_TEST_MERGE_ALGORITHM environment variable setting. NOTE: The tests do not yet pass with GIT_TEST_MERGE_ALGORITHM=ort, because I haven't submitted the implementation of the merge-ort functions -- yet. I figured they are useful as a high level overview of the differences in behavior between the two backends, and thus am providing these before the implementation. Changes since v1: * Minor tweak to test_expect_merge_algorithm() suggested by Junio * Updated a test for portability * I had meant to leave the final patch, patch 9, out of the submission. But, since I submitted it, update it to match my local version of that test with one additional thing checked in the result. [1] https://lore.kernel.org/git/pull.895.git.git.1603286555.gitgitgadget@gmail.com/ Elijah Newren (9): t/: new helper for tests that pass with ort but fail with recursive merge tests: expect improved directory/file conflict handling in ort t6416: correct expectation for rename/rename(1to2) + directory/file t6404, t6423: expect improved rename/delete handling in ort backend t6423: expect improved conflict markers labels in the ort backend merge tests: expect slight differences in output for recursive vs. ort t6423, t6436: note improved ort handling with dirty files t6423: note improved ort handling with untracked files t6423: add more details about direct resolution of directories t/lib-merge.sh | 13 + t/t6400-merge-df.sh | 14 +- t/t6402-merge-rename.sh | 122 ++++- t/t6404-recursive-merge.sh | 14 +- t/t6416-recursive-corner-cases.sh | 200 ++++--- t/t6422-merge-rename-corner-cases.sh | 37 +- t/t6423-merge-rename-directories.sh | 706 +++++++++++++++---------- t/t6426-merge-skip-unneeded-updates.sh | 3 +- t/t6430-merge-recursive.sh | 3 +- t/t6436-merge-overwrite.sh | 18 +- t/t6437-submodule-merge.sh | 25 +- t/t7602-merge-octopus-many.sh | 6 + t/t7610-mergetool.sh | 32 +- 13 files changed, 807 insertions(+), 386 deletions(-) create mode 100644 t/lib-merge.sh base-commit: c64432aacda9054459ce550eca95929897c301bd Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-769%2Fnewren%2Ftests-support-both-merge-backends-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-769/newren/tests-support-both-merge-backends-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/769 Range-diff vs v1: 1: a8d4825a32 ! 1: f5d3295262 t/: new helper for tests that pass with ort but fail with recursive @@ t/lib-merge.sh (new) +# Helper functions used by merge tests. + +test_expect_merge_algorithm () { -+ status_for_recursive=$1 -+ shift -+ status_for_ort=$1 -+ shift ++ status_for_recursive=$1 status_for_ort=$2 ++ shift 2 + + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then 2: 9394d526f1 = 2: f27f12e8e5 merge tests: expect improved directory/file conflict handling in ort 3: 1eae84b787 = 3: 38919982c4 t6416: correct expectation for rename/rename(1to2) + directory/file 4: 554feb2aa7 = 4: 6e47fa19f8 t6404, t6423: expect improved rename/delete handling in ort backend 5: 5bead7fa0d = 5: e8a7d6ac81 t6423: expect improved conflict markers labels in the ort backend 6: 2aa44c1f13 ! 6: 4f32450b08 merge tests: expect slight differences in output for recursive vs. ort @@ t/t7602-merge-octopus-many.sh: Merge made by the 'recursive' strategy. test_expect_success 'merge reduces irrelevant remote heads' ' + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then -+ sed -i'' s/recursive/ort/ expected ++ mv expected expected.tmp && ++ sed s/recursive/ort/ expected.tmp >expected && ++ rm expected.tmp + fi && GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual && test_i18ncmp expected actual 7: 9d46cc1ef3 = 7: c5350fd0ae t6423, t6436: note improved ort handling with dirty files 8: 436577106e = 8: 6fd224247d t6423: note improved ort handling with untracked files 9: 2659d1cb98 ! 9: 6e308768ff t6423: add more details about direct resolution of directories @@ t/t6423-merge-rename-directories.sh: test_expect_merge_algorithm failure success + test_cmp expect actual && + + grep region_enter.*collect_merge_info trace.output >collect && -+ test_line_count = 4 collect ++ test_line_count = 4 collect && ++ grep region_enter.*process_entries$ trace.output >process && ++ test_line_count = 2 process ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> There are a number of tests that the "recursive" backend does not handle correctly but which the redesign in "ort" will. Add a new helper in lib-merge.sh for selecting a different test expectation based on the setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to document which ones we expect to fail under recursive but pass under ort. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/lib-merge.sh | 13 +++++++++++++ t/t6416-recursive-corner-cases.sh | 11 ++++++----- t/t6422-merge-rename-corner-cases.sh | 7 ++++--- t/t6423-merge-rename-directories.sh | 13 +++++++------ t/t6426-merge-skip-unneeded-updates.sh | 3 ++- t/t6430-merge-recursive.sh | 3 ++- 6 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 t/lib-merge.sh diff --git a/t/lib-merge.sh b/t/lib-merge.sh new file mode 100644 index 0000000000..8734ebfc17 --- /dev/null +++ b/t/lib-merge.sh @@ -0,0 +1,13 @@ +# Helper functions used by merge tests. + +test_expect_merge_algorithm () { + status_for_recursive=$1 status_for_ort=$2 + shift 2 + + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_expect_${status_for_ort} "$@" + else + test_expect_${status_for_recursive} "$@" + fi +} diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index fd98989b14..8b3a4fc843 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -3,6 +3,7 @@ test_description='recursive merge corner cases involving criss-cross merges' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh # # L1 L2 @@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' ' ) ' -test_expect_failure 'check symlink modify/modify' ' +test_expect_merge_algorithm failure success 'check symlink modify/modify' ' ( cd symlink-modify-modify && @@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' ' ) ' -test_expect_failure 'check symlink add/add' ' +test_expect_merge_algorithm failure success 'check symlink add/add' ' ( cd symlink-add-add && @@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' ' ) ' -test_expect_failure 'check submodule modify/modify' ' +test_expect_merge_algorithm failure success 'check submodule modify/modify' ' ( cd submodule-modify-modify && @@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' ' ) ' -test_expect_failure 'check submodule add/add' ' +test_expect_merge_algorithm failure success 'check submodule add/add' ' ( cd submodule-add-add && @@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' ' ) ' -test_expect_failure 'check conflicting entry types (submodule vs symlink)' ' +test_expect_merge_algorithm failure success 'check conflicting entry types (submodule vs symlink)' ' ( cd submodule-symlink-add-add && diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh index 3375eaf4e7..58729593ba 100755 --- a/t/t6422-merge-rename-corner-cases.sh +++ b/t/t6422-merge-rename-corner-cases.sh @@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses" # t6036 has corner cases that involve both criss-cross merges and renames . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh test_setup_rename_delete_untracked () { test_create_repo rename-delete-untracked && @@ -878,7 +879,7 @@ test_setup_rad () { ) } -test_expect_failure 'rad-check: rename/add/delete conflict' ' +test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' ' test_setup_rad && ( cd rad && @@ -951,7 +952,7 @@ test_setup_rrdd () { ) } -test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' +test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' ' test_setup_rrdd && ( cd rrdd && @@ -1040,7 +1041,7 @@ test_setup_mod6 () { ) } -test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' +test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' ' test_setup_mod6 && ( cd mod6 && diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 06b46af765..807a424a52 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -26,6 +26,7 @@ test_description="recursive merge with directory renames" # files that might be renamed into each other's paths.) . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh ########################################################################### @@ -1339,7 +1340,7 @@ test_setup_6b1 () { ) } -test_expect_failure '6b1: Same renames done on both sides, plus another rename' ' +test_expect_merge_algorithm failure success '6b1: Same renames done on both sides, plus another rename' ' test_setup_6b1 && ( cd 6b1 && @@ -1412,7 +1413,7 @@ test_setup_6b2 () { ) } -test_expect_failure '6b2: Same rename done on both sides' ' +test_expect_merge_algorithm failure success '6b2: Same rename done on both sides' ' test_setup_6b2 && ( cd 6b2 && @@ -3471,7 +3472,7 @@ test_setup_10e () { ) } -test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' ' +test_expect_merge_algorithm failure success '10e: Does git complain about untracked file that is not really in the way?' ' test_setup_10e && ( cd 10e && @@ -4104,7 +4105,7 @@ test_setup_12b1 () { ) } -test_expect_failure '12b1: Moving two directory hierarchies into each other' ' +test_expect_merge_algorithm failure success '12b1: Moving two directory hierarchies into each other' ' test_setup_12b1 && ( cd 12b1 && @@ -4272,7 +4273,7 @@ test_setup_12c1 () { ) } -test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' ' +test_expect_merge_algorithm failure success '12c1: Moving one directory hierarchy into another w/ content merge' ' test_setup_12c1 && ( cd 12c1 && @@ -4632,7 +4633,7 @@ test_setup_12f () { ) } -test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' ' +test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' ' test_setup_12f && ( cd 12f && diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh index 699813671c..d7eeee4310 100755 --- a/t/t6426-merge-skip-unneeded-updates.sh +++ b/t/t6426-merge-skip-unneeded-updates.sh @@ -23,6 +23,7 @@ test_description="merge cases" # files that might be renamed into each other's paths.) . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh ########################################################################### @@ -666,7 +667,7 @@ test_setup_4a () { # correct requires doing the merge in-memory first, then realizing that no # updates to the file are necessary, and thus that we can just leave the path # alone. -test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' ' +test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' ' test_setup_4a && ( cd 4a && diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh index a328260d42..9c08e63af2 100755 --- a/t/t6430-merge-recursive.sh +++ b/t/t6430-merge-recursive.sh @@ -3,6 +3,7 @@ test_description='merge-recursive backend test' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh test_expect_success 'setup 1' ' @@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' ' test_cmp expected actual ' -test_expect_failure 'merge-recursive rename vs. rename/symlink' ' +test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' ' git checkout -f rename && git merge rename-ln && -- gitgitgadget
From: Elijah Newren <newren@gmail.com> 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 <newren@gmail.com> --- t/t6400-merge-df.sh | 14 +- t/t6402-merge-rename.sh | 108 ++++++++++++---- t/t6416-recursive-corner-cases.sh | 185 +++++++++++++++++++-------- t/t6422-merge-rename-corner-cases.sh | 30 +++-- t/t6423-merge-rename-directories.sh | 53 +++++--- t/t7610-mergetool.sh | 32 ++++- 6 files changed, 314 insertions(+), 108 deletions(-) diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh index f1b84617af..9da0838216 100755 --- a/t/t6400-merge-df.sh +++ b/t/t6400-merge-df.sh @@ -81,7 +81,12 @@ test_expect_success 'modify/delete + directory/file conflict' ' test 5 -eq $(git ls-files -s | wc -l) && test 4 -eq $(git ls-files -u | wc -l) && - test 1 -eq $(git ls-files -o | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 0 -eq $(git ls-files -o | wc -l) + else + test 1 -eq $(git ls-files -o | wc -l) + fi && test_path_is_file letters/file && test_path_is_file letters.txt && @@ -97,7 +102,12 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' test 5 -eq $(git ls-files -s | wc -l) && test 4 -eq $(git ls-files -u | wc -l) && - test 1 -eq $(git ls-files -o | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 0 -eq $(git ls-files -o | wc -l) + else + test 1 -eq $(git ls-files -o | wc -l) + fi && test_path_is_file letters/file && test_path_is_file letters.txt && diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index bbbba3dcbf..47d4434d64 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -397,7 +397,12 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t test_must_fail git merge --strategy=recursive dir-in-way && test 5 -eq "$(git ls-files -u | wc -l)" && - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)" + else + test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" + fi && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && test_must_fail git diff --quiet && @@ -415,7 +420,12 @@ test_expect_success 'Same as previous, but merged other way' ' test_must_fail git merge --strategy=recursive renamed-file-has-conflicts && test 5 -eq "$(git ls-files -u | wc -l)" && - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)" + else + test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" + fi && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && test_must_fail git diff --quiet && @@ -471,7 +481,12 @@ test_expect_success 'both rename source and destination involved in D/F conflict git checkout -q rename-dest^0 && test_must_fail git merge --strategy=recursive source-conflict && - test 1 -eq "$(git ls-files -u | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 2 -eq "$(git ls-files -u | wc -l)" + else + test 1 -eq "$(git ls-files -u | wc -l)" + fi && test_must_fail git diff --quiet && @@ -505,34 +520,63 @@ test_expect_success 'setup pair rename to parent of other (D/F conflicts)' ' git commit -m "Rename one/file -> two" ' -test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' ' - git checkout -q rename-one^0 && - mkdir one && - test_must_fail git merge --strategy=recursive rename-two && +if test "$GIT_TEST_MERGE_ALGORITHM" = ort +then + test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' ' + git checkout -q rename-one^0 && + mkdir one && + test_must_fail git merge --strategy=recursive rename-two && - test 2 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && + test 4 -eq "$(git ls-files -u | wc -l)" && + test 2 -eq "$(git ls-files -u one | wc -l)" && + test 2 -eq "$(git ls-files -u two | wc -l)" && - test_must_fail git diff --quiet && + test_must_fail git diff --quiet && - test 4 -eq $(find . | grep -v .git | wc -l) && + test 3 -eq $(find . | grep -v .git | wc -l) && - test_path_is_dir one && - test_path_is_file one~rename-two && - test_path_is_file two && - test "other" = $(cat one~rename-two) && - test "stuff" = $(cat two) -' + test_path_is_file one && + test_path_is_file two && + test "other" = $(cat one) && + test "stuff" = $(cat two) + ' +else + test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' ' + git checkout -q rename-one^0 && + mkdir one && + test_must_fail git merge --strategy=recursive rename-two && + + test 2 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one | wc -l)" && + test 1 -eq "$(git ls-files -u two | wc -l)" && + + test_must_fail git diff --quiet && + + test 4 -eq $(find . | grep -v .git | wc -l) && + + test_path_is_dir one && + test_path_is_file one~rename-two && + test_path_is_file two && + test "other" = $(cat one~rename-two) && + test "stuff" = $(cat two) + ' +fi test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean start' ' git reset --hard && git clean -fdqx && test_must_fail git merge --strategy=recursive rename-two && - test 2 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 4 -eq "$(git ls-files -u | wc -l)" && + test 2 -eq "$(git ls-files -u one | wc -l)" && + test 2 -eq "$(git ls-files -u two | wc -l)" + else + test 2 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one | wc -l)" && + test 1 -eq "$(git ls-files -u two | wc -l)" + fi && test_must_fail git diff --quiet && @@ -572,12 +616,22 @@ test_expect_success 'check handling of differently renamed file with D/F conflic git checkout -q first-rename^0 && test_must_fail git merge --strategy=recursive second-rename && - test 5 -eq "$(git ls-files -s | wc -l)" && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && - test 1 -eq "$(git ls-files -u original | wc -l)" && - test 2 -eq "$(git ls-files -o | wc -l)" && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 5 -eq "$(git ls-files -s | wc -l)" && + test 3 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" && + test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" && + test 1 -eq "$(git ls-files -u original | wc -l)" && + test 0 -eq "$(git ls-files -o | wc -l)" + else + test 5 -eq "$(git ls-files -s | wc -l)" && + test 3 -eq "$(git ls-files -u | wc -l)" && + test 1 -eq "$(git ls-files -u one | wc -l)" && + test 1 -eq "$(git ls-files -u two | wc -l)" && + test 1 -eq "$(git ls-files -u original | wc -l)" && + test 2 -eq "$(git ls-files -o | wc -l)" + fi && test_path_is_file one/file && test_path_is_file two/file && diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index 8b3a4fc843..0317e83970 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -538,9 +538,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict git checkout B^0 && test_must_fail git merge C^0 && - git clean -fd && - git rm -rf a/ && - git rm a && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rm -rf a/ && + git rm a~HEAD + else + git clean -fd && + git rm -rf a/ && + git rm a + fi && git cat-file -p B:a >a2 && git add a2 && git commit -m D2 && @@ -559,7 +565,12 @@ test_expect_success 'setup differently handled merges of directory/file conflict git checkout C^0 && test_must_fail git merge B^0 && - git clean -fd && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rm a~B^0 + else + git clean -fd + fi && git rm -rf a/ && test_write_lines 1 2 3 4 5 6 7 8 >a && git add a && @@ -568,9 +579,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict git checkout C^0 && test_must_fail git merge B^0 && - git clean -fd && - git rm -rf a/ && - git rm a && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rm -rf a/ && + git rm a~B^0 + else + git clean -fd && + git rm -rf a/ && + git rm a + fi && test_write_lines 1 2 3 4 5 6 7 8 >a2 && git add a2 && git commit -m E4 && @@ -588,18 +605,34 @@ test_expect_success 'merge of D1 & E1 fails but has appropriate contents' ' test_must_fail git merge -s recursive E1^0 && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && - - git rev-parse >expect \ - A:ignore-me B:a && - git rev-parse >actual \ - :0:ignore-me :2:a && - test_cmp expect actual + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a D1:a && + git rev-parse >actual \ + :0:ignore-me :1:a :2:a && + test_cmp expect actual + else + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a && + git rev-parse >actual \ + :0:ignore-me :2:a && + test_cmp expect actual + fi ) ' @@ -613,18 +646,34 @@ test_expect_success 'merge of E1 & D1 fails but has appropriate contents' ' test_must_fail git merge -s recursive D1^0 && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && - - git rev-parse >expect \ - A:ignore-me B:a && - git rev-parse >actual \ - :0:ignore-me :3:a && - test_cmp expect actual + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a D1:a && + git rev-parse >actual \ + :0:ignore-me :1:a :3:a && + test_cmp expect actual + else + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + A:ignore-me B:a && + git rev-parse >actual \ + :0:ignore-me :3:a && + test_cmp expect actual + fi ) ' @@ -638,17 +687,32 @@ test_expect_success 'merge of D1 & E2 fails but has appropriate contents' ' test_must_fail git merge -s recursive E2^0 && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >expect \ - B:a E2:a/file C:a/file A:ignore-me && - git rev-parse >actual \ - :2:a :3:a/file :1:a/file :0:ignore-me && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 5 out && + git ls-files -u >out && + test_line_count = 4 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + B:a D1:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :1:a~HEAD :2:a~HEAD :3:a/file :1:a/file :0:ignore-me + else + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 2 out && + + git rev-parse >expect \ + B:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :2:a :3:a/file :1:a/file :0:ignore-me + fi && test_cmp expect actual && test_path_is_file a~HEAD @@ -665,17 +729,32 @@ test_expect_success 'merge of E2 & D1 fails but has appropriate contents' ' test_must_fail git merge -s recursive D1^0 && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >expect \ - B:a E2:a/file C:a/file A:ignore-me && - git rev-parse >actual \ - :3:a :2:a/file :1:a/file :0:ignore-me && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 5 out && + git ls-files -u >out && + test_line_count = 4 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + B:a D1:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :1:a~D1^0 :3:a~D1^0 :2:a/file :1:a/file :0:ignore-me + else + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 2 out && + + git rev-parse >expect \ + B:a E2:a/file C:a/file A:ignore-me && + git rev-parse >actual \ + :3:a :2:a/file :1:a/file :0:ignore-me + fi && test_cmp expect actual && test_path_is_file a~D1^0 diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh index 58729593ba..78bfaf17f0 100755 --- a/t/t6422-merge-rename-corner-cases.sh +++ b/t/t6422-merge-rename-corner-cases.sh @@ -313,15 +313,18 @@ test_expect_success 'rename/directory conflict + clean content merge' ' git ls-files -u >out && test_line_count = 1 out && git ls-files -o >out && - test_line_count = 2 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_line_count = 1 out + else + test_line_count = 2 out + fi && echo 0 >expect && git cat-file -p base:file >>expect && echo 7 >>expect && test_cmp expect newfile~HEAD && - test $(git rev-parse :2:newfile) = $(git hash-object expect) && - test_path_is_file newfile/realfile && test_path_is_file newfile~HEAD ) @@ -344,7 +347,12 @@ test_expect_success 'rename/directory conflict + content merge conflict' ' git ls-files -u >out && test_line_count = 3 out && git ls-files -o >out && - test_line_count = 2 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_line_count = 1 out + else + test_line_count = 2 out + fi && git cat-file -p left-conflict:newfile >left && git cat-file -p base:file >base && @@ -356,10 +364,16 @@ test_expect_success 'rename/directory conflict + content merge conflict' ' left base right && test_cmp left newfile~HEAD && - git rev-parse >expect \ - base:file left-conflict:newfile right:file && - git rev-parse >actual \ - :1:newfile :2:newfile :3:newfile && + git rev-parse >expect \ + base:file left-conflict:newfile right:file && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rev-parse >actual \ + :1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD + else + git rev-parse >actual \ + :1:newfile :2:newfile :3:newfile + fi && test_cmp expect actual && test_path_is_file newfile/realfile && diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 807a424a52..5ea77564d7 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -1177,10 +1177,18 @@ test_expect_success '5d: Directory/file/file conflict due to directory rename' ' git ls-files -u >out && test_line_count = 1 out && git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >actual \ - :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_line_count = 1 out && + + git rev-parse >actual \ + :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d~HEAD :0:y/d/e + else + test_line_count = 2 out && + + git rev-parse >actual \ + :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e + fi && git rev-parse >expect \ O:z/b O:z/c B:z/d B:z/f A:y/d B:y/d/e && test_cmp expect actual && @@ -2017,17 +2025,32 @@ test_expect_success '7e: transitive rename in rename/delete AND dirs in the way' test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out && test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out && - git ls-files -s >out && - test_line_count = 5 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 2 out && - - git rev-parse >actual \ - :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d && - git rev-parse >expect \ - A:x/d/f A:y/d/g O:z/b O:z/c O:x/d && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >actual \ + :0:x/d/f :0:y/d/g :0:y/b :0:y/c :1:y/d~B^0 :3:y/d~B^0 && + git rev-parse >expect \ + A:x/d/f A:y/d/g O:z/b O:z/c O:x/d O:x/d + else + git ls-files -s >out && + test_line_count = 5 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 2 out && + + git rev-parse >actual \ + :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d && + git rev-parse >expect \ + A:x/d/f A:y/d/g O:z/b O:z/c O:x/d + fi && test_cmp expect actual && git hash-object y/d~B^0 >actual && diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index ad288ddc69..70afdd06fa 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -532,7 +532,14 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && - yes "l" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + yes "c" | git mergetool submod~HEAD && + git rm submod && + git mv submod~HEAD submod + else + yes "l" | git mergetool submod + fi && git submodule update -N && echo "not a submodule" >expect && test_cmp expect submod && @@ -549,7 +556,15 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && - yes "r" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + mv submod submod.orig && + git rm --cached submod && + yes "c" | git mergetool submod~test19 && + git mv submod~test19 submod + else + yes "r" | git mergetool submod + fi && test -d submod.orig && git submodule update -N && echo "not a submodule" >expect && @@ -567,6 +582,10 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && yes "l" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + yes "d" | git mergetool submod~test19 + fi && echo "master submodule" >expect && test_cmp expect submod/bar && git submodule update -N && @@ -664,7 +683,14 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && test ! -e submod.orig && - yes "r" | git mergetool submod && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + yes "r" | git mergetool submod~master && + git mv submod submod.orig && + git mv submod~master submod + else + yes "r" | git mergetool submod + fi && test -d submod.orig && echo "not a submodule" >expect && test_cmp expect submod.orig/file16 && -- gitgitgadget
From: Elijah Newren <newren@gmail.com> When files are renamed and modified, we need to do three-way content merges to get the appropriate content in the right location. When we have a rename/rename(1to2) conflict (both sides rename the same file, but differently), that merged content should be placed in each of the two resulting files. merge-recursive handled that fine when that was all that was involved, but when one or more of the two resulting files were ALSO involved in a directory/file conflict, it failed to propagate the merged content to that file. Unfortunately, the one test in t6416 that touched on this combination of cases had been coded to not expect the merged contents to be present. Fix the test to check for the right behavior, and record how the different merge backends will be expected to handle it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6416-recursive-corner-cases.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh index 0317e83970..887c2195a9 100755 --- a/t/t6416-recursive-corner-cases.sh +++ b/t/t6416-recursive-corner-cases.sh @@ -786,7 +786,7 @@ test_expect_success 'merge of D1 & E3 succeeds' ' ) ' -test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' ' +test_expect_merge_algorithm failure success 'merge of D1 & E4 puts merge of a and a2 in both a and a2' ' test_when_finished "git -C directory-file reset --hard" && test_when_finished "git -C directory-file clean -fdqx" && ( @@ -804,7 +804,7 @@ test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' ' test_line_count = 1 out && git rev-parse >expect \ - A:ignore-me B:a D1:a E4:a2 && + A:ignore-me B:a E4:a2 E4:a2 && git rev-parse >actual \ :0:ignore-me :1:a~Temporary\ merge\ branch\ 2 :2:a :3:a2 && test_cmp expect actual -- gitgitgadget
From: Elijah Newren <newren@gmail.com> When a file is renamed and has content conflicts, merge-recursive does not have some stages for the old filename and some stages for the new filename in the index; instead it copies all the stages corresponding to the old filename over to the corresponding locations for the new filename, so that there are three higher order stages all corresponding to the new filename. Doing things this way makes it easier for the user to access the different versions and to resolve the conflict (no need to manually 'git rm' the old version as well as 'git add' the new one). rename/deletes should be handled similarly -- there should be two stages for the renamed file rather than just one. We do not want to destabilize merge-recursive right now, so instead update relevant tests to have different expectations depending on whether the "recursive" or "ort" merge strategies are in use. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6404-recursive-merge.sh | 14 +++++- t/t6423-merge-rename-directories.sh | 70 ++++++++++++++++++++--------- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh index 332cfc53fd..b1c3d4dda4 100755 --- a/t/t6404-recursive-merge.sh +++ b/t/t6404-recursive-merge.sh @@ -118,12 +118,22 @@ test_expect_success 'mark rename/delete as unmerged' ' test_tick && git commit -m rename && test_must_fail git merge delete && - test 1 = $(git ls-files --unmerged | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 2 = $(git ls-files --unmerged | wc -l) + else + test 1 = $(git ls-files --unmerged | wc -l) + fi && git rev-parse --verify :2:a2 && test_must_fail git rev-parse --verify :3:a2 && git checkout -f delete && test_must_fail git merge rename && - test 1 = $(git ls-files --unmerged | wc -l) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test 2 = $(git ls-files --unmerged | wc -l) + else + test 1 = $(git ls-files --unmerged | wc -l) + fi && test_must_fail git rev-parse --verify :2:a2 && git rev-parse --verify :3:a2 ' diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 5ea77564d7..f9a3f24039 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -1271,17 +1271,32 @@ test_expect_success '6a: Tricky rename/delete' ' test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out && test_i18ngrep "CONFLICT (rename/delete).*z/c.*y/c" out && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && - git rev-parse >actual \ - :0:y/b :3:y/c && - git rev-parse >expect \ - O:z/b O:z/c && + git rev-parse >actual \ + :0:y/b :1:y/c :3:y/c && + git rev-parse >expect \ + O:z/b O:z/c O:z/c + else + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >actual \ + :0:y/b :3:y/c && + git rev-parse >expect \ + O:z/b O:z/c + fi && test_cmp expect actual ) ' @@ -1934,17 +1949,32 @@ test_expect_success '7d: transitive rename involved in rename/delete; how is it test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out && test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out && - git ls-files -s >out && - test_line_count = 3 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 1 out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 1 out && - git rev-parse >actual \ - :0:y/b :0:y/c :3:y/d && - git rev-parse >expect \ - O:z/b O:z/c O:x/d && + git rev-parse >actual \ + :0:y/b :0:y/c :1:y/d :3:y/d && + git rev-parse >expect \ + O:z/b O:z/c O:x/d O:x/d + else + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >actual \ + :0:y/b :0:y/c :3:y/d && + git rev-parse >expect \ + O:z/b O:z/c O:x/d + fi && test_cmp expect actual ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> Conflict markers carry an extra annotation of the form REF-OR-COMMIT:FILENAME to help distinguish where the content is coming from, with the :FILENAME piece being left off if it is the same for both sides of history (thus only renames with content conflicts carry that part of the annotation). However, there were cases where the :FILENAME annotation was accidentally left off, due to merge-recursive's every-codepath-needs-a-copy-of-all-special-case-code format. Update a few tests to have the correct :FILENAME extension on relevant paths with the ort backend, while leaving the expectation for merge-recursive the same to avoid destabilizing it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 38 +++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index f9a3f24039..7e32626913 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -302,11 +302,20 @@ test_expect_success '1d: Directory renames cause a rename/rename(2to1) conflict' git cat-file -p :2:x/wham >expect && git cat-file -p :3:x/wham >other && >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge-file \ + -L "HEAD:y/wham" \ + -L "" \ + -L "B^0:z/wham" \ + expect empty other + else + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other + fi && test_cmp expect x/wham ) ' @@ -1823,11 +1832,20 @@ test_expect_success '7b: rename/rename(2to1), but only due to transitive rename' git cat-file -p :2:y/d >expect && git cat-file -p :3:y/d >other && >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge-file \ + -L "HEAD:y/d" \ + -L "" \ + -L "B^0:z/d" \ + expect empty other + else + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other + fi && test_cmp expect y/d ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> The ort merge strategy has some slight differences in commit descriptions (shortened hashes), stdout vs stderr, and in conflict messages. Also, builtin/merge.c reports usage of "ort" as "Merge made by the 'ort' strategy" -- while it is meant as a drop in replacement for "recursive" it is not yet treated as though it is recursive. Update the testcases to expect different output for the different merge backends. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6402-merge-rename.sh | 14 ++++++++++++-- t/t6437-submodule-merge.sh | 25 +++++++++++++++++++++---- t/t7602-merge-octopus-many.sh | 6 ++++++ 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index 47d4434d64..3f64f62224 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -320,7 +320,12 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' ' test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output && test_i18ngrep "Auto-merging dir" output && - test_i18ngrep "Adding as dir~HEAD instead" output && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_i18ngrep "moving it to dir~HEAD instead" output + else + test_i18ngrep "Adding as dir~HEAD instead" output + fi && test 3 -eq "$(git ls-files -u | wc -l)" && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && @@ -342,7 +347,12 @@ test_expect_success 'Same as previous, but merged other way' ' ! grep "error: refusing to lose untracked file at" errors && test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output && test_i18ngrep "Auto-merging dir" output && - test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_i18ngrep "moving it to dir~renamed-file-has-no-conflicts instead" output + else + test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output + fi && test 3 -eq "$(git ls-files -u | wc -l)" && test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh index 6a1e5f8232..3ead2b726f 100755 --- a/t/t6437-submodule-merge.sh +++ b/t/t6437-submodule-merge.sh @@ -127,7 +127,12 @@ test_expect_success 'merging should conflict for non fast-forward' ' git checkout -b test-nonforward b && (cd sub && git rev-parse sub-d > ../expect) && - test_must_fail git merge c 2> actual && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge c >actual + else + test_must_fail git merge c 2> actual + fi && grep $(cat expect) actual > /dev/null && git reset --hard) ' @@ -138,9 +143,21 @@ test_expect_success 'merging should fail for ambiguous common parent' ' (cd sub && git checkout -b ambiguous sub-b && git merge sub-c && - git rev-parse sub-d > ../expect1 && - git rev-parse ambiguous > ../expect2) && - test_must_fail git merge c 2> actual && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + git rev-parse --short sub-d >../expect1 && + git rev-parse --short ambiguous >../expect2 + else + git rev-parse sub-d > ../expect1 && + git rev-parse ambiguous > ../expect2 + fi + ) && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge c >actual + else + test_must_fail git merge c 2> actual + fi && grep $(cat expect1) actual > /dev/null && grep $(cat expect2) actual > /dev/null && git reset --hard) diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh index 6abe441ae3..13859ec859 100755 --- a/t/t7602-merge-octopus-many.sh +++ b/t/t7602-merge-octopus-many.sh @@ -77,6 +77,12 @@ Merge made by the 'recursive' strategy. EOF test_expect_success 'merge reduces irrelevant remote heads' ' + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + mv expected expected.tmp && + sed s/recursive/ort/ expected.tmp >expected && + rm expected.tmp + fi && GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual && test_i18ncmp expected actual ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> The "recursive" backend relies on unpack_trees() to check if unstaged changes would be overwritten by a merge, but unpack_trees() does not understand renames -- and once it returns, it has already written many updates to the working tree and index. As such, "recursive" had to do a special 4-way merge where it would need to also treat the working copy as an extra source of differences that we had to carefully avoid overwriting and resulting in moving files to new locations to avoid conflicts. The "ort" backend, by contrast, does the complete merge inmemory, and only updates the index and working copy as a post-processing step. If there are dirty files in the way, it can simply abort the merge. Update t6423 and t6436 to reflect the better merge abilities and expectations we have for ort, while still leaving the best-case-as-good-as-recursive-can-do expectations there for the recursive backend so we retain its stability until we are ready to deprecate and remove it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 266 ++++++++++++++++------------ t/t6436-merge-overwrite.sh | 18 +- 2 files changed, 165 insertions(+), 119 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 7e32626913..32e6925ca4 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -3634,28 +3634,35 @@ test_expect_success '11a: Avoid losing dirty contents with simple rename' ' echo stuff >>z/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "Refusing to lose dirty file at z/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "Refusing to lose dirty file at z/c" out && - test_seq 1 10 >expected && - echo stuff >>expected && - test_cmp expected z/c && + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 3 out && - git ls-files -s >out && - test_line_count = 2 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 4 out && + git rev-parse >actual \ + :0:z/a :2:z/c && + git rev-parse >expect \ + O:z/a B:z/b && + test_cmp expect actual && - git rev-parse >actual \ - :0:z/a :2:z/c && - git rev-parse >expect \ - O:z/a B:z/b && - test_cmp expect actual && + git hash-object z/c~HEAD >actual && + git rev-parse B:z/b >expect && + test_cmp expect actual + fi && + + test_seq 1 10 >expected && + echo stuff >>expected && + test_cmp expected z/c - git hash-object z/c~HEAD >actual && - git rev-parse B:z/b >expect && - test_cmp expect actual ) ' @@ -3706,32 +3713,39 @@ test_expect_success '11b: Avoid losing dirty file involved in directory rename' git checkout A^0 && echo stuff >>z/c && - git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "Refusing to lose dirty file at z/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && + test_i18ngrep "Refusing to lose dirty file at z/c" out && - grep -q stuff z/c && - test_seq 1 10 >expected && - echo stuff >>expected && - test_cmp expected z/c && + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -m >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out && - git ls-files -s >out && - test_line_count = 3 out && - git ls-files -u >out && - test_line_count = 0 out && - git ls-files -m >out && - test_line_count = 0 out && - git ls-files -o >out && - test_line_count = 4 out && + git rev-parse >actual \ + :0:x/b :0:y/a :0:y/c && + git rev-parse >expect \ + O:x/b O:z/a B:x/c && + test_cmp expect actual && - git rev-parse >actual \ - :0:x/b :0:y/a :0:y/c && - git rev-parse >expect \ - O:x/b O:z/a B:x/c && - test_cmp expect actual && + git hash-object y/c >actual && + git rev-parse B:x/c >expect && + test_cmp expect actual + fi && - git hash-object y/c >actual && - git rev-parse B:x/c >expect && - test_cmp expect actual + grep -q stuff z/c && + test_seq 1 10 >expected && + echo stuff >>expected && + test_cmp expected z/c ) ' @@ -3783,7 +3797,13 @@ test_expect_success '11c: Avoid losing not-uptodate with rename + D/F conflict' echo stuff >>y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "following files would be overwritten by merge" err && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "following files would be overwritten by merge" err + fi && grep -q stuff y/c && test_seq 1 10 >expected && @@ -3851,29 +3871,35 @@ test_expect_success '11d: Avoid losing not-uptodate with rename + D/F conflict' echo stuff >>z/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "Refusing to lose dirty file at z/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "Refusing to lose dirty file at z/c" out && - grep -q stuff z/c && - test_seq 1 10 >expected && - echo stuff >>expected && - test_cmp expected z/c && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 1 out && + git ls-files -o >out && + test_line_count = 4 out && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 1 out && - git ls-files -o >out && - test_line_count = 5 out && + git rev-parse >actual \ + :0:x/b :0:y/a :0:y/c/d :3:y/c && + git rev-parse >expect \ + O:x/b O:z/a B:y/c/d B:x/c && + test_cmp expect actual && - git rev-parse >actual \ - :0:x/b :0:y/a :0:y/c/d :3:y/c && - git rev-parse >expect \ - O:x/b O:z/a B:y/c/d B:x/c && - test_cmp expect actual && + git hash-object y/c~HEAD >actual && + git rev-parse B:x/c >expect && + test_cmp expect actual + fi && - git hash-object y/c~HEAD >actual && - git rev-parse B:x/c >expect && - test_cmp expect actual + grep -q stuff z/c && + test_seq 1 10 >expected && + echo stuff >>expected && + test_cmp expected z/c ) ' @@ -3931,37 +3957,43 @@ test_expect_success '11e: Avoid deleting not-uptodate with dir rename/rename(1to echo mods >>y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose dirty file at y/c" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose dirty file at y/c" out && - git ls-files -s >out && - test_line_count = 7 out && - git ls-files -u >out && - test_line_count = 4 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 7 out && + git ls-files -u >out && + test_line_count = 4 out && + git ls-files -o >out && + test_line_count = 3 out && + + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c && + test_cmp expect actual && + + # See if y/c~merged has expected contents; requires manually + # doing the expected file merge + git cat-file -p A:y/c >c1 && + git cat-file -p B:z/c >c2 && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + c1 empty c2 && + test_cmp c1 y/c~merged + fi && echo different >expected && echo mods >>expected && - test_cmp expected y/c && - - git rev-parse >actual \ - :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c && - test_cmp expect actual && - - # See if y/c~merged has expected contents; requires manually - # doing the expected file merge - git cat-file -p A:y/c >c1 && - git cat-file -p B:z/c >c2 && - >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - c1 empty c2 && - test_cmp c1 y/c~merged + test_cmp expected y/c ) ' @@ -4014,38 +4046,44 @@ test_expect_success '11f: Avoid deleting not-uptodate with dir rename/rename(2to echo important >>y/wham && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose dirty file at y/wham" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose dirty file at y/wham" out && - git ls-files -s >out && - test_line_count = 4 out && - git ls-files -u >out && - test_line_count = 2 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 3 out && - test_seq 1 10 >expected && - echo important >>expected && - test_cmp expected y/wham && + test_must_fail git rev-parse :1:y/wham && - test_must_fail git rev-parse :1:y/wham && + git rev-parse >actual \ + :0:y/a :0:y/b :2:y/wham :3:y/wham && + git rev-parse >expect \ + O:z/a O:z/b O:x/c O:x/d && + test_cmp expect actual && - git rev-parse >actual \ - :0:y/a :0:y/b :2:y/wham :3:y/wham && - git rev-parse >expect \ - O:z/a O:z/b O:x/c O:x/d && - test_cmp expect actual && + # Test that two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged + fi && - # Test that the two-way merge in y/wham~merged is as expected - git cat-file -p :2:y/wham >expect && - git cat-file -p :3:y/wham >other && - >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && - test_cmp expect y/wham~merged + test_seq 1 10 >expected && + echo important >>expected && + test_cmp expected y/wham ) ' diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh index dd8ab7ede1..dd9376842f 100755 --- a/t/t6436-merge-overwrite.sh +++ b/t/t6436-merge-overwrite.sh @@ -97,11 +97,19 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' ' git mv c1.c other.c && git commit -m rename && cp important other.c && - test_must_fail git merge c1a >out && - test_i18ngrep "Refusing to lose dirty file at other.c" out && - test_path_is_file other.c~HEAD && - test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) && - test_cmp important other.c + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_must_fail git merge c1a >out 2>err && + test_i18ngrep "would be overwritten by merge" err && + test_cmp important other.c && + test_path_is_missing .git/MERGE_HEAD + else + test_must_fail git merge c1a >out && + test_i18ngrep "Refusing to lose dirty file at other.c" out && + test_path_is_file other.c~HEAD && + test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) && + test_cmp important other.c + fi ' test_expect_success 'will not overwrite untracked subtree' ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> Similar to the previous commit, since the "recursive" backend relies on unpack_trees() to check if unstaged or untracked files would be overwritten by a merge, and unpack_trees() does not understand renames -- it has false positives and false negatives. Once it has run, since it updates as it goes, merge-recursive then has to handle completing the merge as best it can despite extra changes in the working copy. However, this is not just an issue for dirty files, but also for untracked files because directory renames can cause file contents to need to be written to a location that was not tracked on either side of history. Since the "ort" backend does the complete merge inmemory, and only updates the index and working copy as a post-processing step, if there are untracked files in the way it can simply abort the merge much like checkout does. Update t6423 to reflect the better merge abilities and expectations for ort, while still leaving the best-case-as-good-as-recursive-can-do expectations there for the recursive backend so we retain its stability until we are ready to deprecate and remove it. Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 195 ++++++++++++++++++---------- 1 file changed, 124 insertions(+), 71 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 32e6925ca4..db694a6316 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -3214,6 +3214,7 @@ test_expect_success '10a: Overwrite untracked with normal rename/delete' ' echo important >z/d && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && + test_path_is_missing .git/MERGE_HEAD && test_i18ngrep "The following untracked working tree files would be overwritten by merge" err && git ls-files -s >out && @@ -3283,21 +3284,34 @@ test_expect_success '10b: Overwrite untracked with dir rename + delete' ' echo contents >y/e && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out && - test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 3 out && - git ls-files -u >out && - test_line_count = 2 out && - git ls-files -o >out && - test_line_count = 5 out && + git ls-files -s >out && + test_line_count = 1 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 5 out + else + test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out && + test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out && - git rev-parse >actual \ - :0:y/b :3:y/d :3:y/e && - git rev-parse >expect \ - O:z/b O:z/c B:z/e && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 5 out && + + git rev-parse >actual \ + :0:y/b :3:y/d :3:y/e && + git rev-parse >expect \ + O:z/b O:z/c B:z/e && + test_cmp expect actual + fi && echo very >expect && test_cmp expect y/c && @@ -3360,25 +3374,38 @@ test_expect_success '10c1: Overwrite untracked with dir rename/rename(1to2)' ' echo important >y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 6 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out && - git rev-parse >actual \ - :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 3 out && - git hash-object y/c~B^0 >actual && - git rev-parse O:x/c >expect && - test_cmp expect actual && + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && + test_cmp expect actual && + + git hash-object y/c~B^0 >actual && + git rev-parse O:x/c >expect && + test_cmp expect actual + fi && echo important >expect && test_cmp expect y/c @@ -3398,25 +3425,38 @@ test_expect_success '10c2: Overwrite untracked with dir rename/rename(1to2), oth echo important >y/c && test_must_fail git -c merge.directoryRenames=true merge -s recursive A^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 6 out && - git ls-files -u >out && - test_line_count = 3 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 4 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out && - git rev-parse >actual \ - :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 3 out && - git hash-object y/c~HEAD >actual && - git rev-parse O:x/c >expect && - test_cmp expect actual && + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && + test_cmp expect actual && + + git hash-object y/c~HEAD >actual && + git rev-parse O:x/c >expect && + test_cmp expect actual + fi && echo important >expect && test_cmp expect y/c @@ -3474,37 +3514,50 @@ test_expect_success '10d: Delete untracked with dir rename/rename(2to1)' ' echo important >y/wham && test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep "CONFLICT (rename/rename)" out && - test_i18ngrep "Refusing to lose untracked file at y/wham" out && + if test "$GIT_TEST_MERGE_ALGORITHM" = ort + then + test_path_is_missing .git/MERGE_HEAD && + test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err && - git ls-files -s >out && - test_line_count = 6 out && - git ls-files -u >out && - test_line_count = 2 out && - git ls-files -o >out && - test_line_count = 3 out && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 0 out && + git ls-files -o >out && + test_line_count = 3 out + else + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/wham" out && - git rev-parse >actual \ - :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham && - git rev-parse >expect \ - O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f && - test_cmp expect actual && + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 2 out && + git ls-files -o >out && + test_line_count = 3 out && + + git rev-parse >actual \ + :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f && + test_cmp expect actual && - test_must_fail git rev-parse :1:y/wham && + test_must_fail git rev-parse :1:y/wham && - echo important >expect && - test_cmp expect y/wham && + # Test that two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged + fi && - # Test that the two-way merge in y/wham~merged is as expected - git cat-file -p :2:y/wham >expect && - git cat-file -p :3:y/wham >other && - >empty && - test_must_fail git merge-file \ - -L "HEAD" \ - -L "" \ - -L "B^0" \ - expect empty other && - test_cmp expect y/wham~merged + echo important >expect && + test_cmp expect y/wham ) ' -- gitgitgadget
From: Elijah Newren <newren@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> --- t/t6423-merge-rename-directories.sh | 39 +++++++++++++++++------------ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index db694a6316..4ab133f489 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -4725,20 +4725,22 @@ test_expect_success '12e: Rename/merge subdir into the root, variant 2' ' # folder/subdir/newsubdir/e_Merge2 # folder/subdir/tweaked/{h,Makefile_SUB_Merge1,newfile.py} # folder/unchanged/<LOTS OF FILES> -# -# Notes: This testcase happens to exercise lots of the -# optimization-specific codepaths in merge-ort, and also -# demonstrated a failing of the directory rename detection algorithm -# in merge-recursive; newfile.c should not get pushed into -# folder/subdir/newsubdir/, yet merge-recursive put it there because -# the rename of dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d} -# looks like -# dir/ -> folder/, -# whereas the rename of dir/subdir/e -> folder/subdir/newsubdir/e -# looks like -# dir/subdir/ -> folder/subdir/newsubdir/ -# and if we note that newfile.c is found in dir/subdir/, we might -# overlook the dir/ -> folder/ rule that has more weight. +# Things being checked here: +# 1. dir/subdir/newfile.c does not get pushed into folder/subdir/newsubdir/. +# dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d} looks like +# dir/ -> folder/, +# whereas dir/subdir/e -> folder/subdir/newsubdir/e looks like +# dir/subdir/ -> folder/subdir/newsubdir/ +# and if we note that newfile.c is found in dir/subdir/, we might overlook +# the dir/ -> folder/ rule that has more weight. Older git versions did +# this. +# 2. The code to do trivial directory resolves. Note that +# dir/subdir/unchanged/ is unchanged and can be deleted, and files in the +# new folder/subdir/unchanged/ are not needed as a target to any renames. +# Thus, in the second collect_merge_info_callback() we can just resolve +# these two directories trivially without recursing.) +# 3. Exercising the codepaths for caching renames and deletes from one cherry +# pick and re-applying them in the subsequent one. test_setup_12f () { test_create_repo 12f && @@ -4803,7 +4805,7 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac git checkout A^0 && git branch Bmod B && - git -c merge.directoryRenames=true rebase A Bmod && + GIT_TRACE2_PERF="$(pwd)/trace.output" git -c merge.directoryRenames=true rebase A Bmod && echo Checking the pick of B1... && @@ -4884,7 +4886,12 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac test_seq 2 12 >e_Merge2 && git hash-object e_Merge2 >expect && git rev-parse Bmod:folder/subdir/newsubdir/e >actual && - test_cmp expect actual + test_cmp expect actual && + + grep region_enter.*collect_merge_info trace.output >collect && + test_line_count = 4 collect && + grep region_enter.*process_entries$ trace.output >process && + test_line_count = 2 process ) ' -- gitgitgadget
Elijah Newren <newren@gmail.com> writes:
>> When I wrote the patch, I was expecting something like
>>
>> test_expect_merge_success recursive=failure,other=failure ...
>>
>> in order to merge all algorithm into single parameters.
>>
>> How about something like:
>>
>> test_expect_merge_success exception=recursive,other ...
>>
>> Not that we have "other" algorithm to begin with.
>
> Sure, sounds great. I wouldn't spend any time trying to make it work
> with a 3rd backend, though.
Yup. I'd appreciate if the lines become a bit shorter while at it,
though.
test_merge_both [<exception>] '<title>' '
<body>
'
that expects success unless otherwise told, and <exception> like
"failure=recursive" can be used to tell us to expect differently,
would work well?