From fe48efb5fd161babea245415ee4feef60fee4964 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 3 Aug 2020 18:41:17 +0000 Subject: t6038: make tests fail for the right reason t6038 had a pair of tests that were expected to fail, but weren't failing for the expected reason. Both were meant to do a merge that could be done cleanly after renormalization, but were supposed to fail for lack of renormalization. Unfortunately, both tests had staged changes, and checkout -m would abort due to the presence of those staged changes before even attempting a merge. Fix this first issue by utilizing git-restore instead of git-checkout, so that the index is left alone and just the working directory gets the changes we want. However, there is a second issue with these tests. Technically, they just wanted to verify that after renormalization, no conflicts would be present. This could have been checked for by grepping for a lack of conflict markers, but the test instead tried to compare the working directory files to an expected result. Unfortunately, the setting of "text=auto" without setting core.eol to any value meant that the content of the file (in particular, the line endings) would be platform-dependent and the tests could only pass on some platforms. Replace the existing comparison with a call to 'git diff --no-index --ignore-cr-at-eol' to verify that the contents, other than possible carriage returns in the file, match the expected results and in particular that the file has no conflicts from the checkout -m operation. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6038-merge-text-auto.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 't') diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh index 5e8d5fa50c..27cea15533 100755 --- a/t/t6038-merge-text-auto.sh +++ b/t/t6038-merge-text-auto.sh @@ -168,9 +168,9 @@ test_expect_failure 'checkout -m after setting text=auto' ' git rm -fr . && rm -f .gitattributes && git reset --hard initial && - git checkout a -- . && + git restore --source=a -- . && git checkout -m b && - compare_files expected file + git diff --no-index --ignore-cr-at-eol expected file ' test_expect_failure 'checkout -m addition of text=auto' ' @@ -183,9 +183,9 @@ test_expect_failure 'checkout -m addition of text=auto' ' git rm -fr . && rm -f .gitattributes file && git reset --hard initial && - git checkout b -- . && + git restore --source=b -- . && git checkout -m a && - compare_files expected file + git diff --no-index --ignore-cr-at-eol expected file ' test_expect_failure 'cherry-pick patch from after text=auto was added' ' -- cgit v1.2.3 From 6f6e7cfb52a07578c275656a8fd735265eca1a07 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 3 Aug 2020 18:41:18 +0000 Subject: t6038: remove problematic test t6038.11, 'cherry-pick patch from after text=auto' was a test of undefined behavior. To make matters worse, while there are a couple possible correct answers, this test was coded to only check for an obviously incorrect answer. And the final cherry on top is that the test is marked test_expect_failure, meaning it can't provide much value, other than possibly confusing future folks who come along and try to work on attributes and look at existing tests. Because of all these problems, just remove the test. But for any future code spelunkers, here's my understanding of the two possible correct answers: This test was set up so that on a branch with no .gitattributes file, you cherry-picked a patch from a branch that had a .gitattributes file (containing '* text=auto'). Further, the two branches had a file which differed only in line endings. In this situation, correct behavior is not well defined: should the .gitattributes file affect the merge or not? If the .gitattributes file on the other branch should not affect the merge, then we would have a content conflict with all three stages different (the merge base didn't match either side). If the .gitattributes file from the other branch should affect the merge, then we would expect the line endings to be normalized to LF for the version to be recorded in the repository. This would mean that when doing a three-way content merge on the file that differed in line endings, that the three-way content merge would see that the versions on both sides matched and so the cherry-pick has no conflicts and can succeed. The line endings in the file as recorded in the repository will change from CRLF to LF. The version checked out in the working copy will depend on the platform (since there's no eol attribute defined for the file). Also, as a final side note, this test expected an error message that was built assuming cherry-pick was the old scripted version, because cherry-pick no longer uses the error message that was encoded in this test. So it was wrong for yet another reason. Given that the handling of .gitattributes is not well defined and this test was obviously broken and could do nothing but confuse future readers, just remove it. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6038-merge-text-auto.sh | 14 -------------- 1 file changed, 14 deletions(-) (limited to 't') diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh index 27cea15533..9337745793 100755 --- a/t/t6038-merge-text-auto.sh +++ b/t/t6038-merge-text-auto.sh @@ -188,20 +188,6 @@ test_expect_failure 'checkout -m addition of text=auto' ' git diff --no-index --ignore-cr-at-eol expected file ' -test_expect_failure 'cherry-pick patch from after text=auto was added' ' - append_cr <<-\EOF >expected && - first line - same line - EOF - - git config merge.renormalize true && - git rm -fr . && - git reset --hard b && - test_must_fail git cherry-pick a >err 2>&1 && - grep "[Nn]othing added" err && - compare_files expected file -' - test_expect_success 'Test delete/normalize conflict' ' git checkout -f side && git rm -fr . && -- cgit v1.2.3 From 8d552258f45ba3886e8d508ece8585b509a04677 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 3 Aug 2020 18:41:19 +0000 Subject: merge: make merge.renormalize work for all uses of merge machinery The 'merge' command is not the only one that does merges; other commands like checkout -m or rebase do as well. Unfortunately, the only area of the code that checked for the "merge.renormalize" config setting was in builtin/merge.c, meaning it could only affect merges performed by the "merge" command. Move the handling of this config setting to merge_recursive_config() so that other commands can benefit from it as well. Fixes a few tests in t6038. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6038-merge-text-auto.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh index 9337745793..89c86d4e56 100755 --- a/t/t6038-merge-text-auto.sh +++ b/t/t6038-merge-text-auto.sh @@ -158,7 +158,7 @@ test_expect_success 'Detect LF/CRLF conflict from addition of text=auto' ' compare_files expected file.fuzzy ' -test_expect_failure 'checkout -m after setting text=auto' ' +test_expect_success 'checkout -m after setting text=auto' ' cat <<-\EOF >expected && first line same line @@ -173,7 +173,7 @@ test_expect_failure 'checkout -m after setting text=auto' ' git diff --no-index --ignore-cr-at-eol expected file ' -test_expect_failure 'checkout -m addition of text=auto' ' +test_expect_success 'checkout -m addition of text=auto' ' cat <<-\EOF >expected && first line same line -- cgit v1.2.3