{"thread":{"id":"60604","subject":"[PATCH] tests: prefer host Git to verify chainlint self-checks","startedAt":"2023-12-12T11:32:57Z","lastAt":"2023-12-15T16:48:12Z","messageCount":21,"participants":["Patrick Steinhardt","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"485576","messageId":"4112adbe467c14a8f22a87ea41aa4705f8760cf6.1702380646.git.ps@pks.im","threadId":"60604","inReplyTo":null,"subject":"[PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-12T11:32:50Z","receivedAt":"2023-12-12T11:32:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"check-chainlint\" target runs automatically when running tests and\nperforms self-checks to verify that the chainlinter itself produces the\nexpected output. Originally, the chainlinter was implemented via sed,\nbut the infrastructure has been rewritten in fb41727b7e (t: retire\nunused chainlint.sed, 2022-09-01) to use a Perl script instead.\n\nThe rewrite caused some slight whitespace changes in the output that are\nultimately not of much importance. In order to be able to assert that\nthe actual chainlinter errors match our expectations we thus have to\nignore whitespace characters when diffing them. As the `-w` flag is not\nin POSIX we try to use `git diff -w --no-index` before we fall back to\n`diff -w -u`.\n\nTo accomodate for cases where the host system has no Git installation we\nuse the locally-compiled version of Git. This can result in problems\nthough when the Git project's repository is using extensions that the\nlocally-compiled version of Git doesn't understand. It will refuse to\nrun and thus cause the checks to fail.\n\nFix this issue by prefering the host's Git resolved via PATH. If it\ndoesn't exist, then we fall back to the locally-compiled Git version and\ndiff as before.\n\nSigned-off-by: Patrick Steinhardt \n---\n\nI've started to dogfood the reftable backend on my local machine and\nhave converted many repositories to use the reftable backend. This\nsurfaced the described issue because the repository now sets up the\n\"extensions.refStorage\" extension, and thus \"check-chainlint\" fails\ndepending on which versions of Git I'm trying to compile and test.\n\n t/Makefile | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 225aaf78ed..8b7f7aceaa 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -111,7 +111,9 @@ check-chainlint:\n \tif test -f ../GIT-BUILD-OPTIONS; then \\\n \t\t. ../GIT-BUILD-OPTIONS; \\\n \tfi && \\\n-\tif test -x ../git$$X; then \\\n+\tif command -v git >/dev/null 2>&1; then \\\n+\t\tDIFFW=\"git --no-pager diff -w --no-index\"; \\\n+\telif test -x ../git$$X; then \\\n \t\tDIFFW=\"../git$$X --no-pager diff -w --no-index\"; \\\n \telse \\\n \t\tDIFFW=\"diff -w -u\"; \\\n-- \n2.43.GIT\n\n"},{"id":"485588","messageId":"xmqq8r5zrzg1.fsf@gitster.g","threadId":"60604","inReplyTo":"4112adbe467c14a8f22a87ea41aa4705f8760cf6.1702380646.git.ps@pks.im","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-12T19:30:22Z","receivedAt":"2023-12-12T19:30:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> To accomodate for cases where the host system has no Git installation we\n> use the locally-compiled version of Git. This can result in problems\n> though when the Git project's repository is using extensions that the\n> locally-compiled version of Git doesn't understand. It will refuse to\n> run and thus cause the checks to fail.\n>\n> Fix this issue by prefering the host's Git resolved via PATH. If it\n> doesn't exist, then we fall back to the locally-compiled Git version and\n> diff as before.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n>\n> I've started to dogfood the reftable backend on my local machine and\n> have converted many repositories to use the reftable backend. This\n> surfaced the described issue because the repository now sets up the\n> \"extensions.refStorage\" extension, and thus \"check-chainlint\" fails\n> depending on which versions of Git I'm trying to compile and test.\n\nI do not think \"prefer host Git\" is necessarily a good idea; falling\nback to use host Git is perfectly fine, of course.\n\nOther than that, I agree with the motivation.\n\n> t/Makefile | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/Makefile b/t/Makefile\n> index 225aaf78ed..8b7f7aceaa 100644\n> --- a/t/Makefile\n> +++ b/t/Makefile\n> @@ -111,7 +111,9 @@ check-chainlint:\n> \tif test -f ../GIT-BUILD-OPTIONS; then \\\n> \t\t. ../GIT-BUILD-OPTIONS; \\\n> \tfi && \\\n> -\tif test -x ../git$$X; then \\\n> +\tif command -v git >/dev/null 2>&1; then \\\n> +\t\tDIFFW=\"git --no-pager diff -w --no-index\"; \\\n> +\telif test -x ../git$$X; then \\\n> \t\tDIFFW=\"../git$$X --no-pager diff -w --no-index\"; \\\n> \telse \\\n> \t\tDIFFW=\"diff -w -u\"; \\\n"},{"id":"485605","messageId":"ZXlbNlG28e1sAYPU@tanuki","threadId":"60604","inReplyTo":"xmqq8r5zrzg1.fsf@gitster.g","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-13T07:20:22Z","receivedAt":"2023-12-13T07:20:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Dec 12, 2023 at 11:30:22AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > To accomodate for cases where the host system has no Git installation we\n> > use the locally-compiled version of Git. This can result in problems\n> > though when the Git project's repository is using extensions that the\n> > locally-compiled version of Git doesn't understand. It will refuse to\n> > run and thus cause the checks to fail.\n> >\n> > Fix this issue by prefering the host's Git resolved via PATH. If it\n> > doesn't exist, then we fall back to the locally-compiled Git version and\n> > diff as before.\n> >\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> >\n> > I've started to dogfood the reftable backend on my local machine and\n> > have converted many repositories to use the reftable backend. This\n> > surfaced the described issue because the repository now sets up the\n> > \"extensions.refStorage\" extension, and thus \"check-chainlint\" fails\n> > depending on which versions of Git I'm trying to compile and test.\n> \n> I do not think \"prefer host Git\" is necessarily a good idea; falling\n> back to use host Git is perfectly fine, of course.\n\nWhy is that, though? We already use host Git in other parts of our build\ninfra, and the options we pass to git-diff(1) have been around for ages:\n\n - `--no-pager` was introduced in 463a849d00 (Add and document a global\n --no-pager option for git., 2007-08-19).\n\n - `--no-index` is around since at least fcfa33ec90 (diff: make more\n cases implicit --no-index, 2007-02-25).\n\n - `-w` is around since 0d21efa51c (Teach diff about -b and -w flags,\n 2006-06-14).\n\nSo all options have pretty much been there since forever. Which means\nthat if the host has Git around, and the Git version is at least v1.5.3,\nthen it also understands all options.\n\nFurthermore, we are accessing the Git repository that the user has set\nup with host Git in the first place, so I'd think even conceptually it\nis the correct thing to do.\n\nPatrick\n"},{"id":"485616","messageId":"xmqqr0jqnnmn.fsf@gitster.g","threadId":"60604","inReplyTo":"ZXlbNlG28e1sAYPU@tanuki","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-13T15:11:28Z","receivedAt":"2023-12-13T15:11:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> I do not think \"prefer host Git\" is necessarily a good idea; falling\n>> back to use host Git is perfectly fine, of course.\n>\n> Why is that, though?\n\nMostly because your \"differences in features supported by just-built\none and what happens to be on $PATH can cause problems\" cuts both\nways, and as a general principle to work around such issues, using\njust-built one is a better discipline. The features the build\ninfrastructure (\"self-checks\" being discussed is a part of it) of a\nparticular version of Git source depends on are more likely to be\nfound in the binary that will be built from the matching source,\nthan what happens to be on $PATH that may be from a year-old version.\n\nAs you said, you'd need to accomodate need for those who are\ninitially porting or building Git on a host without an installed\none. If we were doing a cross build where just-built on would not\nbe executable on the host, whatever version on $PATH (or in\n/usr/bin) may have to be used, but then in such a case you would not\nbe testing on host anyway. For these two reasons, it is a given\nthat one of the choices has to be to use just-built one. We can\nsafely give lower precedence to the host tool.\n\nThe only one-and-half practical reasons we may want to fall back to\nwhatever happens to be on $PATH are:\n\n - just-built one is so broken that even the simple use by the build\n infrastructure (like \"self-checks\") does not work (but then it\n becomes \"if it is so broken, fix it before even thinking about\n running tests\", and that is why I count it as half a reason), or\n\n - we are bisecting down to an ancient version, and just-built one\n from such a version may not understand the current repository.\n\nso I do think it is an excellent workaround to fall back to a\nversion of Git with unknown vintage that happens to be on $PATH,\nthan failing and stopping by relying only on just-built one.\n\n> We already use host Git in other parts of our build\n> infra, and the options we pass to git-diff(1) have been around for ages:\n\nIt only argues for \"trying host one, if available, before just-built\none does not hurt for this particular case\". But I was interested\nin laying out a more general principle we can follow in similar\nsituations in the future.\n"},{"id":"485629","messageId":"CAPig+cRc2hW_xhJRPJmEVYik71zWLDQ_EFjBFw095OgPGYrWGg@mail.gmail.com","threadId":"60604","inReplyTo":"xmqqr0jqnnmn.fsf@gitster.g","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-14T03:33:00Z","receivedAt":"2023-12-14T03:33:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 13, 2023 at 10:11 AM Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> >> I do not think \"prefer host Git\" is necessarily a good idea; falling\n> >> back to use host Git is perfectly fine, of course.\n> >\n> > Why is that, though?\n>\n> Mostly because your \"differences in features supported by just-built\n> one and what happens to be on $PATH can cause problems\" cuts both\n> ways [...]\n\nI sent an alternative solution[1] which should sidestep this objection.\n\nAs usual, I forgot to use --in-reply-to= when sending the\npatch, even though I reminded myself to use it only a minute or so\nearlier. Sorry.\n\n[1]: https://lore.kernel.org/git/20231214032248.1615-1-ericsunshine@charter.net/\n"},{"id":"485632","messageId":"ZXq5GL723v4E3_IH@tanuki","threadId":"60604","inReplyTo":"CAPig+cRc2hW_xhJRPJmEVYik71zWLDQ_EFjBFw095OgPGYrWGg@mail.gmail.com","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-14T08:13:12Z","receivedAt":"2023-12-14T08:13:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Dec 13, 2023 at 10:33:00PM -0500, Eric Sunshine wrote:\n> On Wed, Dec 13, 2023 at 10:11 AM Junio C Hamano wrote:\n> > Patrick Steinhardt writes:\n> > >> I do not think \"prefer host Git\" is necessarily a good idea; falling\n> > >> back to use host Git is perfectly fine, of course.\n> > >\n> > > Why is that, though?\n> >\n> > Mostly because your \"differences in features supported by just-built\n> > one and what happens to be on $PATH can cause problems\" cuts both\n> > ways [...]\n> \n> I sent an alternative solution[1] which should sidestep this objection.\n> \n> As usual, I forgot to use --in-reply-to= when sending the\n> patch, even though I reminded myself to use it only a minute or so\n> earlier. Sorry.\n> \n> [1]: https://lore.kernel.org/git/20231214032248.1615-1-ericsunshine@charter.net/\n\nThanks, I've replied to the thread. I think by now there are three\ndifferent ideas:\n\n - Improve the logic to pick some kind of diff implementation, which is\n my patch series. It would need to be improved so that we also probe\n whether the respective Git executables actually understand the repo\n format so that we can fall back from the just-built Git to system's\n Git.\n\n - Munge the whitespace of the expected results with some regexes.\n I like that idea better because we can avoid the git-diff(1)\n problem, but find that the result is somewhat hard to read.\n\n - Fix the \".expect\" files so that we can avoid all of these games.\n\nI actually like the last option most. I'll have a go at it and send this\nthird version out in a bit.\n\nPatrick\n"},{"id":"485633","messageId":"aec86a15c69aa276eee4875fad208ee2fc57635a.1702542564.git.ps@pks.im","threadId":"60604","inReplyTo":"4112adbe467c14a8f22a87ea41aa4705f8760cf6.1702380646.git.ps@pks.im","subject":"[PATCH v2] tests: adjust whitespace in chainlint expectations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-14T08:30:29Z","receivedAt":"2023-12-14T08:30:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"check-chainlint\" target runs automatically when running tests and\nperforms self-checks to verify that the chainlinter itself produces the\nexpected output. Originally, the chainlinter was implemented via sed,\nbut the infrastructure has been rewritten in fb41727b7e (t: retire\nunused chainlint.sed, 2022-09-01) to use a Perl script instead.\n\nThe rewrite caused some slight whitespace changes in the output that are\nultimately not of much importance. In order to be able to assert that\nthe actual chainlinter errors match our expectations we thus have to\nignore whitespace characters when diffing them. As the `-w` flag is not\nin POSIX we try to use `git diff -w --no-index` before we fall back to\n`diff -w -u`.\n\nTo accomodate for cases where the host system has no Git installation we\nuse the locally-compiled version of Git. This can result in problems\nthough when the Git project's repository is using extensions that the\nlocally-compiled version of Git doesn't understand. It will refuse to\nrun and thus cause the checks to fail.\n\nInstead of improving the detection logic, fix our \".expect\" files so\nthat we do not need any post-processing anymore. This allows us to drop\nthe `-w` flag when diffing so that we can always use diff(1) now.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/Makefile | 10 +-------\n t/chainlint/blank-line-before-esac.expect | 8 +++----\n t/chainlint/block.expect | 4 ++--\n t/chainlint/chain-break-background.expect | 4 ++--\n t/chainlint/chain-break-return-exit.expect | 14 +++++------\n t/chainlint/chain-break-status.expect | 4 ++--\n t/chainlint/chained-subshell.expect | 4 ++--\n .../command-substitution-subsubshell.expect | 2 +-\n t/chainlint/dqstring-line-splice.expect | 4 ++--\n t/chainlint/dqstring-no-interpolate.expect | 4 ++--\n t/chainlint/empty-here-doc.expect | 4 ++--\n t/chainlint/exclamation.expect | 2 +-\n t/chainlint/for-loop-abbreviated.expect | 2 +-\n t/chainlint/function.expect | 4 ++--\n t/chainlint/here-doc.expect | 4 ++--\n t/chainlint/loop-detect-status.expect | 20 ++++++++--------\n t/chainlint/nested-loop-detect-failure.expect | 24 +++++++++----------\n t/chainlint/subshell-here-doc.expect | 4 ++--\n t/chainlint/token-pasting.expect | 14 +++++------\n 19 files changed, 64 insertions(+), 72 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 225aaf78ed..3a5d81b7fe 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -108,15 +108,7 @@ check-chainlint:\n \t} >'$(CHAINLINTTMP_SQ)'/expect && \\\n \t$(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n \t\tsed -e 's/^[1-9][0-9]* //;/^[ \t]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n-\tif test -f ../GIT-BUILD-OPTIONS; then \\\n-\t\t. ../GIT-BUILD-OPTIONS; \\\n-\tfi && \\\n-\tif test -x ../git$$X; then \\\n-\t\tDIFFW=\"../git$$X --no-pager diff -w --no-index\"; \\\n-\telse \\\n-\t\tDIFFW=\"diff -w -u\"; \\\n-\tfi && \\\n-\t$$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n+\tdiff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n \n test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \\\n \ttest-lint-filenames\ndiff --git a/t/chainlint/blank-line-before-esac.expect b/t/chainlint/blank-line-before-esac.expect\nindex 48ed4eb124..056e03003d 100644\n--- a/t/chainlint/blank-line-before-esac.expect\n+++ b/t/chainlint/blank-line-before-esac.expect\n@@ -1,11 +1,11 @@\n-test_done ( ) {\n+test_done () {\n \tcase \"$test_failure\" in\n-\t0 )\n+\t0)\n \t\ttest_at_end_hook_\n \n \t\texit 0 ;;\n \n-\t* )\n+\t*)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n \t\t\tsay_color error \"# failed $test_failure among $msg\"\n@@ -14,5 +14,5 @@ test_done ( ) {\n \n \t\texit 1 ;;\n \n-\t\tesac\n+\tesac\n }\ndiff --git a/t/chainlint/block.expect b/t/chainlint/block.expect\nindex a3bcea492a..1c87326364 100644\n--- a/t/chainlint/block.expect\n+++ b/t/chainlint/block.expect\n@@ -12,9 +12,9 @@\n ) &&\n \n {\n-\techo a ; ?!AMP?! echo b\n+\techo a; ?!AMP?! echo b\n } &&\n-{ echo a ; ?!AMP?! echo b ; } &&\n+{ echo a; ?!AMP?! echo b; } &&\n \n {\n \techo \"${var}9\" &&\ndiff --git a/t/chainlint/chain-break-background.expect b/t/chainlint/chain-break-background.expect\nindex 28f9114f42..20d0bb5333 100644\n--- a/t/chainlint/chain-break-background.expect\n+++ b/t/chainlint/chain-break-background.expect\n@@ -1,9 +1,9 @@\n JGIT_DAEMON_PID= &&\n git init --bare empty.git &&\n-> empty.git/git-daemon-export-ok &&\n+>empty.git/git-daemon-export-ok &&\n mkfifo jgit_daemon_output &&\n {\n-\tjgit daemon --port=\"$JGIT_DAEMON_PORT\" . > jgit_daemon_output &\n+\tjgit daemon --port=\"$JGIT_DAEMON_PORT\" . >jgit_daemon_output &\n \tJGIT_DAEMON_PID=$!\n } &&\n test_expect_code 2 git ls-remote --exit-code git://localhost:$JGIT_DAEMON_PORT/empty.git\ndiff --git a/t/chainlint/chain-break-return-exit.expect b/t/chainlint/chain-break-return-exit.expect\nindex 1732d221c3..4cd18e2edf 100644\n--- a/t/chainlint/chain-break-return-exit.expect\n+++ b/t/chainlint/chain-break-return-exit.expect\n@@ -1,16 +1,16 @@\n case \"$(git ls-files)\" in\n-one ) echo pass one ;;\n-* ) echo bad one ; return 1 ;;\n+one) echo pass one ;;\n+*) echo bad one; return 1 ;;\n esac &&\n (\n \tcase \"$(git ls-files)\" in\n-\ttwo ) echo pass two ;;\n-\t* ) echo bad two ; exit 1 ;;\n-esac\n+\ttwo) echo pass two ;;\n+\t*) echo bad two; exit 1 ;;\n+\tesac\n ) &&\n case \"$(git ls-files)\" in\n-dir/two\"$LF\"one ) echo pass both ;;\n-* ) echo bad ; return 1 ;;\n+dir/two\"$LF\"one) echo pass both ;;\n+*) echo bad; return 1 ;;\n esac &&\n \n for i in 1 2 3 4 ; do\ndiff --git a/t/chainlint/chain-break-status.expect b/t/chainlint/chain-break-status.expect\nindex f4bada9463..e6b3b2193e 100644\n--- a/t/chainlint/chain-break-status.expect\n+++ b/t/chainlint/chain-break-status.expect\n@@ -1,7 +1,7 @@\n-OUT=$(( ( large_git ; echo $? 1 >& 3 ) | : ) 3 >& 1) &&\n+OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n test_match_signal 13 \"$OUT\" &&\n \n-{ test-tool sigchain > actual ; ret=$? ; } &&\n+{ test-tool sigchain >actual; ret=$?; } &&\n {\n \ttest_match_signal 15 \"$ret\" ||\n \ttest \"$ret\" = 3\ndiff --git a/t/chainlint/chained-subshell.expect b/t/chainlint/chained-subshell.expect\nindex af0369d328..83810ea7ec 100644\n--- a/t/chainlint/chained-subshell.expect\n+++ b/t/chainlint/chained-subshell.expect\n@@ -4,7 +4,7 @@ mkdir sub && (\n \tnuff said\n ) &&\n \n-cut \"-d \" -f actual | ( read s1 s2 s3 &&\n+cut \"-d \" -f actual | (read s1 s2 s3 &&\n test -f $s1 ?!AMP?!\n test $(cat $s2) = tree2path1 &&\n-test $(cat $s3) = tree3path1 )\n+test $(cat $s3) = tree3path1)\ndiff --git a/t/chainlint/command-substitution-subsubshell.expect b/t/chainlint/command-substitution-subsubshell.expect\nindex ab2f79e845..ec42f2c30c 100644\n--- a/t/chainlint/command-substitution-subsubshell.expect\n+++ b/t/chainlint/command-substitution-subsubshell.expect\n@@ -1,2 +1,2 @@\n-OUT=$(( ( large_git 1 >& 3 ) | : ) 3 >& 1) &&\n+OUT=$( ((large_git 1>&3) | :) 3>&1 ) &&\n test_match_signal 13 \"$OUT\"\ndiff --git a/t/chainlint/dqstring-line-splice.expect b/t/chainlint/dqstring-line-splice.expect\nindex bf9ced60d4..f8fa3bc969 100644\n--- a/t/chainlint/dqstring-line-splice.expect\n+++ b/t/chainlint/dqstring-line-splice.expect\n@@ -1,3 +1,3 @@\n-echo 'fatal: reword option of --fixup is mutually exclusive with' '--patch/--interactive/--all/--include/--only' > expect &&\n-test_must_fail git commit --fixup=reword:HEAD~ $1 2 > actual &&\n+echo 'fatal: reword option of --fixup is mutually exclusive with'\t'--patch/--interactive/--all/--include/--only' >expect &&\n+test_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&\n test_cmp expect actual\ndiff --git a/t/chainlint/dqstring-no-interpolate.expect b/t/chainlint/dqstring-no-interpolate.expect\nindex 10724987a5..2c1851a3c3 100644\n--- a/t/chainlint/dqstring-no-interpolate.expect\n+++ b/t/chainlint/dqstring-no-interpolate.expect\n@@ -6,6 +6,6 @@ grep \"^\\.git$\" output.txt &&\n (\n \tcd client$version &&\n \tGIT_TEST_PROTOCOL_VERSION=$version git fetch-pack --no-progress .. $(cat ../input)\n-) > output &&\n-\tcut -d ' ' -f 2 < output | sort > actual &&\n+) >output &&\n+\tcut -d ' ' -f 2 actual &&\n \ttest_cmp expect actual\ndiff --git a/t/chainlint/empty-here-doc.expect b/t/chainlint/empty-here-doc.expect\nindex e8733c97c6..8507721192 100644\n--- a/t/chainlint/empty-here-doc.expect\n+++ b/t/chainlint/empty-here-doc.expect\n@@ -1,4 +1,4 @@\n-git ls-tree $tree path > current &&\n-cat > expected <<\\EOF &&\n+git ls-tree $tree path >current &&\n+cat >expected <<\\EOF &&\n EOF\n test_output\ndiff --git a/t/chainlint/exclamation.expect b/t/chainlint/exclamation.expect\nindex 2d961a58c6..765a35bb4c 100644\n--- a/t/chainlint/exclamation.expect\n+++ b/t/chainlint/exclamation.expect\n@@ -1,4 +1,4 @@\n-if ! condition ; then echo nope ; else yep ; fi &&\n+if ! condition; then echo nope; else yep; fi &&\n test_prerequisite !MINGW &&\n mail uucp!address &&\n echo !whatever!\ndiff --git a/t/chainlint/for-loop-abbreviated.expect b/t/chainlint/for-loop-abbreviated.expect\nindex a21007a63f..02c0d15cca 100644\n--- a/t/chainlint/for-loop-abbreviated.expect\n+++ b/t/chainlint/for-loop-abbreviated.expect\n@@ -1,5 +1,5 @@\n for it\n do\n-\tpath=$(expr \"$it\" : ( [^:]*) ) &&\n+\tpath=$(expr \"$it\" : ([^:]*)) &&\n \tgit update-index --add \"$path\" || exit\n done\ndiff --git a/t/chainlint/function.expect b/t/chainlint/function.expect\nindex a14388e6b9..dd7c997a3c 100644\n--- a/t/chainlint/function.expect\n+++ b/t/chainlint/function.expect\n@@ -1,8 +1,8 @@\n-sha1_file ( ) {\n+sha1_file() {\n \techo \"$*\" | sed \"s#..#.git/objects/&/#\"\n } &&\n \n-remove_object ( ) {\n+remove_object() {\n \tfile=$(sha1_file \"$*\") &&\n \ttest -e \"$file\" ?!AMP?!\n \trm -f \"$file\"\ndiff --git a/t/chainlint/here-doc.expect b/t/chainlint/here-doc.expect\nindex 1df3f78282..91b961242a 100644\n--- a/t/chainlint/here-doc.expect\n+++ b/t/chainlint/here-doc.expect\n@@ -1,6 +1,6 @@\n boodle wobba \\\n-\tgorgo snoot \\\n-\twafta snurb <& 2 &&\n+(while test $i -le $blobcount\n+ do\n+\tprintf \"Generating blob $i/$blobcount\\r\" >&2 &&\n \tprintf \"blob\\nmark :$i\\ndata $blobsize\\n\" &&\n \t#test-tool genrandom $i $blobsize &&\n \tprintf \"%-${blobsize}s\" $i &&\n \techo \"M 100644 :$i $i\" >> commit &&\n \ti=$(($i+1)) ||\n \techo $? > exit-status\n-done &&\n-echo \"commit refs/heads/main\" &&\n-echo \"author A U Thor 123456789 +0000\" &&\n-echo \"committer C O Mitter 123456789 +0000\" &&\n-echo \"data 5\" &&\n-echo \">2gb\" &&\n-cat commit ) |\n+ done &&\n+ echo \"commit refs/heads/main\" &&\n+ echo \"author A U Thor 123456789 +0000\" &&\n+ echo \"committer C O Mitter 123456789 +0000\" &&\n+ echo \"data 5\" &&\n+ echo \">2gb\" &&\n+ cat commit) |\n git fast-import --big-file-threshold=2 &&\n test ! -f exit-status\ndiff --git a/t/chainlint/nested-loop-detect-failure.expect b/t/chainlint/nested-loop-detect-failure.expect\nindex 4793a0e8e1..3461df40e5 100644\n--- a/t/chainlint/nested-loop-detect-failure.expect\n+++ b/t/chainlint/nested-loop-detect-failure.expect\n@@ -1,31 +1,31 @@\n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" ?!LOOP?!\n+\t\techo \"$i$j\" >\"path$i$j\" ?!LOOP?!\n \tdone ?!LOOP?!\n done &&\n \n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" || return 1\n+\t\techo \"$i$j\" >\"path$i$j\" || return 1\n \tdone\n done &&\n \n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" ?!LOOP?!\n+\t\techo \"$i$j\" >\"path$i$j\" ?!LOOP?!\n \tdone || return 1\n done &&\n \n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" || return 1\n+\t\techo \"$i$j\" >\"path$i$j\" || return 1\n \tdone || return 1\n done\ndiff --git a/t/chainlint/subshell-here-doc.expect b/t/chainlint/subshell-here-doc.expect\nindex 52789278d1..75d6f607e2 100644\n--- a/t/chainlint/subshell-here-doc.expect\n+++ b/t/chainlint/subshell-here-doc.expect\n@@ -1,7 +1,7 @@\n (\n \techo wobba \\\n-\t\tgorgo snoot \\\n-\t\twafta snurb <<-EOF &&\n+\t gorgo snoot \\\n+\t wafta snurb <<-EOF &&\n \tquoth the raven,\n \tnevermore...\n \tEOF\ndiff --git a/t/chainlint/token-pasting.expect b/t/chainlint/token-pasting.expect\nindex 342360bcd0..6a387917a7 100644\n--- a/t/chainlint/token-pasting.expect\n+++ b/t/chainlint/token-pasting.expect\n@@ -4,22 +4,22 @@ git config filter.rot13.clean ./rot13.sh &&\n {\n echo \"*.t filter=rot13\" ?!AMP?!\n echo \"*.i ident\"\n-} > .gitattributes &&\n+} >.gitattributes &&\n \n {\n echo a b c d e f g h i j k l m ?!AMP?!\n echo n o p q r s t u v w x y z ?!AMP?!\n echo '$Id$'\n-} > test &&\n-cat test > test.t &&\n-cat test > test.o &&\n-cat test > test.i &&\n+} >test &&\n+cat test >test.t &&\n+cat test >test.o &&\n+cat test >test.i &&\n git add test test.t test.i &&\n rm -f test test.t test.i &&\n git checkout -- test test.t test.i &&\n \n-echo \"content-test2\" > test2.o &&\n-echo \"content-test3 - filename with special characters\" > \"test3 'sq',$x=.o\" ?!AMP?!\n+echo \"content-test2\" >test2.o &&\n+echo \"content-test3 - filename with special characters\" >\"test3 'sq',$x=.o\" ?!AMP?!\n \n downstream_url_for_sed=$(\n \tprintf \"%sn\" \"$downstream_url\" |\n-- \n2.43.GIT\n\n"},{"id":"485635","messageId":"CAPig+cQ2-PB24n0xfcoSy_1UT-VbEZUXXJ9QbA8FBA8Vfyd6Ng@mail.gmail.com","threadId":"60604","inReplyTo":"ZXq5GL723v4E3_IH@tanuki","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-14T08:39:17Z","receivedAt":"2023-12-14T08:39:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Dec 14, 2023 at 3:13 AM Patrick Steinhardt wrote:\n> On Wed, Dec 13, 2023 at 10:33:00PM -0500, Eric Sunshine wrote:\n> > On Wed, Dec 13, 2023 at 10:11 AM Junio C Hamano wrote:\n> > > Mostly because your \"differences in features supported by just-built\n> > > one and what happens to be on $PATH can cause problems\" cuts both\n> > > ways [...]\n> >\n> > I sent an alternative solution[1] which should sidestep this objection.\n>\n> Thanks, I've replied to the thread. I think by now there are three\n> different ideas:\n>\n> - Improve the logic to pick some kind of diff implementation, which is\n> my patch series. It would need to be improved so that we also probe\n> whether the respective Git executables actually understand the repo\n> format so that we can fall back from the just-built Git to system's\n> Git.\n>\n> - Munge the whitespace of the expected results with some regexes.\n> I like that idea better because we can avoid the git-diff(1)\n> problem, but find that the result is somewhat hard to read.\n>\n> - Fix the \".expect\" files so that we can avoid all of these games.\n>\n> I actually like the last option most. I'll have a go at it and send this\n> third version out in a bit.\n\nI sent a reply[1] in the other thread explaining why I'm still leaning\ntoward `sed` to smooth over these minor differences rather than\nchurning the \"expect\" files, especially since the minor differences\nare not significant to what is actually being tested. That said, I\nwon't stand in the way of such a patch to \"fix\" the \"expect\" files,\nbut it feels unnecessary.\n\n[1]: https://lore.kernel.org/git/CAPig+cSkuRfkR2D3JqYcbaJqj485nfD9Nq6pM=vXWB5DJenWpA@mail.gmail.com/\n"},{"id":"485636","messageId":"ZXq_jo9QfSmjPGmK@tanuki","threadId":"60604","inReplyTo":"CAPig+cQ2-PB24n0xfcoSy_1UT-VbEZUXXJ9QbA8FBA8Vfyd6Ng@mail.gmail.com","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-14T08:40:46Z","receivedAt":"2023-12-14T08:40:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 14, 2023 at 03:39:17AM -0500, Eric Sunshine wrote:\n> On Thu, Dec 14, 2023 at 3:13 AM Patrick Steinhardt wrote:\n> > On Wed, Dec 13, 2023 at 10:33:00PM -0500, Eric Sunshine wrote:\n> > > On Wed, Dec 13, 2023 at 10:11 AM Junio C Hamano wrote:\n> > > > Mostly because your \"differences in features supported by just-built\n> > > > one and what happens to be on $PATH can cause problems\" cuts both\n> > > > ways [...]\n> > >\n> > > I sent an alternative solution[1] which should sidestep this objection.\n> >\n> > Thanks, I've replied to the thread. I think by now there are three\n> > different ideas:\n> >\n> > - Improve the logic to pick some kind of diff implementation, which is\n> > my patch series. It would need to be improved so that we also probe\n> > whether the respective Git executables actually understand the repo\n> > format so that we can fall back from the just-built Git to system's\n> > Git.\n> >\n> > - Munge the whitespace of the expected results with some regexes.\n> > I like that idea better because we can avoid the git-diff(1)\n> > problem, but find that the result is somewhat hard to read.\n> >\n> > - Fix the \".expect\" files so that we can avoid all of these games.\n> >\n> > I actually like the last option most. I'll have a go at it and send this\n> > third version out in a bit.\n> \n> I sent a reply[1] in the other thread explaining why I'm still leaning\n> toward `sed` to smooth over these minor differences rather than\n> churning the \"expect\" files, especially since the minor differences\n> are not significant to what is actually being tested. That said, I\n> won't stand in the way of such a patch to \"fix\" the \"expect\" files,\n> but it feels unnecessary.\n> \n> [1]: https://lore.kernel.org/git/CAPig+cSkuRfkR2D3JqYcbaJqj485nfD9Nq6pM=vXWB5DJenWpA@mail.gmail.com/\n\nYeah, our mails indeed crossed. I personally do not mind much which of\nour patches land upstream and would be happy with either.\n\nThanks!\n\nPatrick\n"},{"id":"485637","messageId":"CAPig+cTZmiXdPZEVO-F2UzV9YaP6c7r2MfPTC3QWksJa+rM7VA@mail.gmail.com","threadId":"60604","inReplyTo":"aec86a15c69aa276eee4875fad208ee2fc57635a.1702542564.git.ps@pks.im","subject":"Re: [PATCH v2] tests: adjust whitespace in chainlint expectations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-14T08:44:43Z","receivedAt":"2023-12-14T08:44:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Dec 14, 2023 at 3:30 AM Patrick Steinhardt wrote:\n> [...]\n> Instead of improving the detection logic, fix our \".expect\" files so\n> that we do not need any post-processing anymore. This allows us to drop\n> the `-w` flag when diffing so that we can always use diff(1) now.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> diff --git a/t/Makefile b/t/Makefile\n> @@ -108,15 +108,7 @@ check-chainlint:\n> } >'$(CHAINLINTTMP_SQ)'/expect && \\\n> $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n> sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n> - if test -f ../GIT-BUILD-OPTIONS; then \\\n> - . ../GIT-BUILD-OPTIONS; \\\n> - fi && \\\n> - if test -x ../git$$X; then \\\n> - DIFFW=\"../git$$X --no-pager diff -w --no-index\"; \\\n> - else \\\n> - DIFFW=\"diff -w -u\"; \\\n> - fi && \\\n> - $$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n> + diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n\nThis change feels incomplete. Yes, it \"corrects\" the whitespace, but\ncheck-chainlint still applies sed to remove blank lines and strip the\nline numbers from the beginning of each line. Based upon the commit\nmessage, I had expected that all post-processing of the output would\nhave been eliminated and that the \"expect\" files would exactly mirror\nthe output of the linter.\n\nAs mentioned elsewhere[1], I'm not particularly in favor of this\napproach, though I won't stand in the way of it either.\n\n[1]: https://lore.kernel.org/git/CAPig+cSkuRfkR2D3JqYcbaJqj485nfD9Nq6pM=vXWB5DJenWpA@mail.gmail.com/\n"},{"id":"485656","messageId":"xmqqbkaspxn6.fsf@gitster.g","threadId":"60604","inReplyTo":"CAPig+cQ2-PB24n0xfcoSy_1UT-VbEZUXXJ9QbA8FBA8Vfyd6Ng@mail.gmail.com","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-14T16:16:45Z","receivedAt":"2023-12-14T16:16:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> - Fix the \".expect\" files so that we can avoid all of these games.\n>>\n>> I actually like the last option most. I'll have a go at it and send this\n>> third version out in a bit.\n>\n> I sent a reply[1] in the other thread explaining why I'm still leaning\n> toward `sed` to smooth over these minor differences rather than\n> churning the \"expect\" files, especially since the minor differences\n> are not significant to what is actually being tested.\n\nIf it is just one time bulk conversion under t/chainlint/ to match\nwhat the chainlint.pl script produces, with the possibility of\nsimilar bulk updates in the future when the script gets updated, I\ntend to agree with Patrick that getting rid of the fuzzy comparison\nwill be the best way forward.\n\nEspecially if the fuzzy comparison is done only to hide differences\nbetween what the old chainlint.sed used to produce and what the\ncurrent version produces, that is. If for some reason the script\nstarted to create subtly different output for other reasons (e.g.,\nit may produce different whitespaces on a particular platform, or\nwith a specific version of perl interpreter), we'd better be aware\nof it, instead of blindly ignoring the differences without\ninspecting them and verifying that they are benign.\n\nBy going through the single conversion pain, it will force us to\nthink twice before breaking its output stability while updating\nchainlint.pl, which would also be a good thing.\n\nTHanks.\n"},{"id":"485661","messageId":"CAPig+cQvcSeSKVE=0kDyNiSztNAgVwhfAzoL5K7uYHEKe=0f_A@mail.gmail.com","threadId":"60604","inReplyTo":"xmqqbkaspxn6.fsf@gitster.g","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-14T18:10:48Z","receivedAt":"2023-12-14T18:11:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Dec 14, 2023 at 11:16 AM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > I sent a reply[1] in the other thread explaining why I'm still leaning\n> > toward `sed` to smooth over these minor differences rather than\n> > churning the \"expect\" files, especially since the minor differences\n> > are not significant to what is actually being tested.\n>\n> If it is just one time bulk conversion under t/chainlint/ to match\n> what the chainlint.pl script produces, with the possibility of\n> similar bulk updates in the future when the script gets updated, I\n> tend to agree with Patrick that getting rid of the fuzzy comparison\n> will be the best way forward.\n\nOkay, that's fine. If we take this approach, though, then it would\nmake sense to eliminate _all_ gratuitous postprocessing of the\n\"expect\" files[1] so that we really are comparing the direct output of\nchainlint.pl with the \"expect\" files, rather than merely munging the\ninline whitespace of the \"expect\" files slightly as Patrick's proposed\npatch does[2].\n\n(The only postprocessing of \"expect\" files which needs to stay is the\nbit which removes the \"# LINT:\" comments which litter the \"expect\"\nfiles explaining to human readers why the linter should insert a\n\"???FOO???\" annotation at that particular point.)\n\n> Especially if the fuzzy comparison is done only to hide differences\n> between what the old chainlint.sed used to produce and what the\n> current version produces, that is. If for some reason the script\n> started to create subtly different output for other reasons (e.g.,\n> it may produce different whitespaces on a particular platform, or\n> with a specific version of perl interpreter), we'd better be aware\n> of it, instead of blindly ignoring the differences without\n> inspecting them and verifying that they are benign.\n>\n> By going through the single conversion pain, it will force us to\n> think twice before breaking its output stability while updating\n> chainlint.pl, which would also be a good thing.\n\nThe chainlint self-tests were never meant to be about its general\noutput stability. They were intended to ensure that the \"???FOO???\"\nannotations are: (1) indeed inserted for the set of linting problems\nthe tool detects, and (2) inserted at the correct spot in the emitted\noutput relative to the shell tokens to which the annotation applies.\nMinor differences in the tool's output (whether over time or between\nplatforms) should be immaterial in respect to those correctness goals.\n\n[1]: https://lore.kernel.org/git/CAPig+cTZmiXdPZEVO-F2UzV9YaP6c7r2MfPTC3QWksJa+rM7VA@mail.gmail.com/\n[2]: https://lore.kernel.org/git/aec86a15c69aa276eee4875fad208ee2fc57635a.1702542564.git.ps@pks.im/\n"},{"id":"485663","messageId":"xmqqfs04oawk.fsf@gitster.g","threadId":"60604","inReplyTo":"CAPig+cQvcSeSKVE=0kDyNiSztNAgVwhfAzoL5K7uYHEKe=0f_A@mail.gmail.com","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-14T19:13:15Z","receivedAt":"2023-12-14T19:15:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> (The only postprocessing of \"expect\" files which needs to stay is the\n> bit which removes the \"# LINT:\" comments which litter the \"expect\"\n> files explaining to human readers why the linter should insert a\n> \"???FOO???\" annotation at that particular point.)\n\nYup, that matches my understanding.\n\n> The chainlint self-tests were never meant to be about its general\n> output stability. They were intended to ensure that the \"???FOO???\"\n> annotations are: (1) indeed inserted for the set of linting problems\n> the tool detects, and (2) inserted at the correct spot in the emitted\n> output relative to the shell tokens to which the annotation applies.\n\nYup.\n\n> Minor differences in the tool's output (whether over time or between\n> platforms) should be immaterial in respect to those correctness goals.\n\nBut there is no reason to make such immaterial changes to the output\ngratuitously when we are updating the tool to improve it, no?\n"},{"id":"485703","messageId":"ZXvlExouF3a29wch@tanuki","threadId":"60604","inReplyTo":"CAPig+cQvcSeSKVE=0kDyNiSztNAgVwhfAzoL5K7uYHEKe=0f_A@mail.gmail.com","subject":"Re: [PATCH] tests: prefer host Git to verify chainlint self-checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-15T05:33:07Z","receivedAt":"2023-12-15T05:33:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 14, 2023 at 01:10:48PM -0500, Eric Sunshine wrote:\n> On Thu, Dec 14, 2023 at 11:16 AM Junio C Hamano wrote:\n> > Eric Sunshine writes:\n> > > I sent a reply[1] in the other thread explaining why I'm still leaning\n> > > toward `sed` to smooth over these minor differences rather than\n> > > churning the \"expect\" files, especially since the minor differences\n> > > are not significant to what is actually being tested.\n> >\n> > If it is just one time bulk conversion under t/chainlint/ to match\n> > what the chainlint.pl script produces, with the possibility of\n> > similar bulk updates in the future when the script gets updated, I\n> > tend to agree with Patrick that getting rid of the fuzzy comparison\n> > will be the best way forward.\n> \n> Okay, that's fine. If we take this approach, though, then it would\n> make sense to eliminate _all_ gratuitous postprocessing of the\n> \"expect\" files[1] so that we really are comparing the direct output of\n> chainlint.pl with the \"expect\" files, rather than merely munging the\n> inline whitespace of the \"expect\" files slightly as Patrick's proposed\n> patch does[2].\n> \n> (The only postprocessing of \"expect\" files which needs to stay is the\n> bit which removes the \"# LINT:\" comments which litter the \"expect\"\n> files explaining to human readers why the linter should insert a\n> \"???FOO???\" annotation at that particular point.)\n\nOkay. I'll send a v3 to also drop the other post-processing steps then.\n\nPatrick\n"},{"id":"485705","messageId":"8c3e1cb5eae13210070cc14f5f2a3f8c0dfc39c3.1702620230.git.ps@pks.im","threadId":"60604","inReplyTo":"4112adbe467c14a8f22a87ea41aa4705f8760cf6.1702380646.git.ps@pks.im","subject":"[PATCH v3] tests: adjust whitespace in chainlint expectations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-15T06:04:30Z","receivedAt":"2023-12-15T06:04:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"check-chainlint\" target runs automatically when running tests and\nperforms self-checks to verify that the chainlinter itself produces the\nexpected output. Originally, the chainlinter was implemented via sed,\nbut the infrastructure has been rewritten in fb41727b7e (t: retire\nunused chainlint.sed, 2022-09-01) to use a Perl script instead.\n\nThe rewrite caused some slight whitespace changes in the output that are\nultimately not of much importance. In order to be able to assert that\nthe actual chainlinter errors match our expectations we thus have to\nignore whitespace characters when diffing them. As the `-w` flag is not\nin POSIX we try to use `git diff -w --no-index` before we fall back to\n`diff -w -u`.\n\nTo accomodate for cases where the host system has no Git installation we\nuse the locally-compiled version of Git. This can result in problems\nthough when the Git project's repository is using extensions that the\nlocally-compiled version of Git doesn't understand. It will refuse to\nrun and thus cause the checks to fail.\n\nInstead of improving the detection logic, fix our \".expect\" files so\nthat we do not need any post-processing at all anymore. This allows us\nto drop the `-w` flag when diffing so that we can always use diff(1)\nnow.\n\nNote that we leave the post-processing of `chainlint.pl` output intact.\nAll we do here is to strip leading line numbers that it would otherwise\ngenerate. Having these would cause a rippling effect whenever we add a\nnew test that sorts into the middle of existing tests and would require\nus to renumerate all subsequent lines, which seems rather pointless.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/Makefile | 12 ++-------\n t/chainlint/blank-line-before-esac.expect | 12 +++------\n t/chainlint/block.expect | 6 ++---\n t/chainlint/chain-break-background.expect | 4 +--\n t/chainlint/chain-break-continue.expect | 1 -\n t/chainlint/chain-break-return-exit.expect | 15 +++++------\n t/chainlint/chain-break-status.expect | 5 ++--\n t/chainlint/chained-block.expect | 1 -\n t/chainlint/chained-subshell.expect | 5 ++--\n .../command-substitution-subsubshell.expect | 2 +-\n t/chainlint/cuddled.expect | 4 ---\n t/chainlint/dqstring-line-splice.expect | 4 +--\n t/chainlint/dqstring-no-interpolate.expect | 7 ++---\n t/chainlint/empty-here-doc.expect | 4 +--\n t/chainlint/exclamation.expect | 2 +-\n t/chainlint/for-loop-abbreviated.expect | 2 +-\n t/chainlint/function.expect | 6 ++---\n t/chainlint/here-doc-indent-operator.expect | 2 --\n t/chainlint/here-doc.expect | 7 ++---\n t/chainlint/inline-comment.expect | 1 -\n t/chainlint/loop-detect-failure.expect | 1 -\n t/chainlint/loop-detect-status.expect | 20 +++++++-------\n t/chainlint/negated-one-liner.expect | 1 -\n t/chainlint/nested-here-doc.expect | 3 ---\n t/chainlint/nested-loop-detect-failure.expect | 27 +++++++++----------\n t/chainlint/not-heredoc.expect | 1 -\n t/chainlint/one-liner.expect | 2 --\n t/chainlint/subshell-here-doc.expect | 6 ++---\n t/chainlint/token-pasting.expect | 18 +++++--------\n 29 files changed, 65 insertions(+), 116 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 225aaf78ed..ab620cf732 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -103,20 +103,12 @@ check-chainlint:\n \t\techo \"# chainlint: $(CHAINLINTTMP_SQ)/tests\" && \\\n \t\tfor i in $(CHAINLINTTESTS); do \\\n \t\t\techo \"# chainlint: $$i\" && \\\n-\t\t\tsed -e '/^[ \t]*$$/d' chainlint/$$i.expect; \\\n+\t\t\tcat chainlint/$$i.expect; \\\n \t\tdone \\\n \t} >'$(CHAINLINTTMP_SQ)'/expect && \\\n \t$(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n \t\tsed -e 's/^[1-9][0-9]* //;/^[ \t]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n-\tif test -f ../GIT-BUILD-OPTIONS; then \\\n-\t\t. ../GIT-BUILD-OPTIONS; \\\n-\tfi && \\\n-\tif test -x ../git$$X; then \\\n-\t\tDIFFW=\"../git$$X --no-pager diff -w --no-index\"; \\\n-\telse \\\n-\t\tDIFFW=\"diff -w -u\"; \\\n-\tfi && \\\n-\t$$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n+\tdiff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n \n test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \\\n \ttest-lint-filenames\ndiff --git a/t/chainlint/blank-line-before-esac.expect b/t/chainlint/blank-line-before-esac.expect\nindex 48ed4eb124..7400a1df7e 100644\n--- a/t/chainlint/blank-line-before-esac.expect\n+++ b/t/chainlint/blank-line-before-esac.expect\n@@ -1,18 +1,14 @@\n-test_done ( ) {\n+test_done () {\n \tcase \"$test_failure\" in\n-\t0 )\n+\t0)\n \t\ttest_at_end_hook_\n-\n \t\texit 0 ;;\n-\n-\t* )\n+\t*)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n \t\t\tsay_color error \"# failed $test_failure among $msg\"\n \t\t\tsay \"1..$test_count\"\n \t\tfi\n-\n \t\texit 1 ;;\n-\n-\t\tesac\n+\tesac\n }\ndiff --git a/t/chainlint/block.expect b/t/chainlint/block.expect\nindex a3bcea492a..569211de08 100644\n--- a/t/chainlint/block.expect\n+++ b/t/chainlint/block.expect\n@@ -10,12 +10,10 @@\n \t} ?!AMP?!\n \tbaz\n ) &&\n-\n {\n-\techo a ; ?!AMP?! echo b\n+\techo a; ?!AMP?! echo b\n } &&\n-{ echo a ; ?!AMP?! echo b ; } &&\n-\n+{ echo a; ?!AMP?! echo b; } &&\n {\n \techo \"${var}9\" &&\n \techo \"done\"\ndiff --git a/t/chainlint/chain-break-background.expect b/t/chainlint/chain-break-background.expect\nindex 28f9114f42..20d0bb5333 100644\n--- a/t/chainlint/chain-break-background.expect\n+++ b/t/chainlint/chain-break-background.expect\n@@ -1,9 +1,9 @@\n JGIT_DAEMON_PID= &&\n git init --bare empty.git &&\n-> empty.git/git-daemon-export-ok &&\n+>empty.git/git-daemon-export-ok &&\n mkfifo jgit_daemon_output &&\n {\n-\tjgit daemon --port=\"$JGIT_DAEMON_PORT\" . > jgit_daemon_output &\n+\tjgit daemon --port=\"$JGIT_DAEMON_PORT\" . >jgit_daemon_output &\n \tJGIT_DAEMON_PID=$!\n } &&\n test_expect_code 2 git ls-remote --exit-code git://localhost:$JGIT_DAEMON_PORT/empty.git\ndiff --git a/t/chainlint/chain-break-continue.expect b/t/chainlint/chain-break-continue.expect\nindex 47a3457710..fa00c9fdda 100644\n--- a/t/chainlint/chain-break-continue.expect\n+++ b/t/chainlint/chain-break-continue.expect\n@@ -4,7 +4,6 @@ do\n \ttest \"$path\" = \"foobar/non-note.txt\" && continue\n \ttest \"$path\" = \"deadbeef\" && continue\n \ttest \"$path\" = \"de/adbeef\" && continue\n-\n \tif test $(expr length \"$path\") -ne $hexsz\n \tthen\n \t\treturn 1\ndiff --git a/t/chainlint/chain-break-return-exit.expect b/t/chainlint/chain-break-return-exit.expect\nindex 1732d221c3..bedc0711e4 100644\n--- a/t/chainlint/chain-break-return-exit.expect\n+++ b/t/chainlint/chain-break-return-exit.expect\n@@ -1,18 +1,17 @@\n case \"$(git ls-files)\" in\n-one ) echo pass one ;;\n-* ) echo bad one ; return 1 ;;\n+one) echo pass one ;;\n+*) echo bad one; return 1 ;;\n esac &&\n (\n \tcase \"$(git ls-files)\" in\n-\ttwo ) echo pass two ;;\n-\t* ) echo bad two ; exit 1 ;;\n-esac\n+\ttwo) echo pass two ;;\n+\t*) echo bad two; exit 1 ;;\n+\tesac\n ) &&\n case \"$(git ls-files)\" in\n-dir/two\"$LF\"one ) echo pass both ;;\n-* ) echo bad ; return 1 ;;\n+dir/two\"$LF\"one) echo pass both ;;\n+*) echo bad; return 1 ;;\n esac &&\n-\n for i in 1 2 3 4 ; do\n \tgit checkout main -b $i || return $?\n \ttest_commit $i $i $i tag$i || return $?\ndiff --git a/t/chainlint/chain-break-status.expect b/t/chainlint/chain-break-status.expect\nindex f4bada9463..53b3f42e7a 100644\n--- a/t/chainlint/chain-break-status.expect\n+++ b/t/chainlint/chain-break-status.expect\n@@ -1,7 +1,6 @@\n-OUT=$(( ( large_git ; echo $? 1 >& 3 ) | : ) 3 >& 1) &&\n+OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n test_match_signal 13 \"$OUT\" &&\n-\n-{ test-tool sigchain > actual ; ret=$? ; } &&\n+{ test-tool sigchain >actual; ret=$?; } &&\n {\n \ttest_match_signal 15 \"$ret\" ||\n \ttest \"$ret\" = 3\ndiff --git a/t/chainlint/chained-block.expect b/t/chainlint/chained-block.expect\nindex 574cdceb07..2416e0fc80 100644\n--- a/t/chainlint/chained-block.expect\n+++ b/t/chainlint/chained-block.expect\n@@ -2,7 +2,6 @@ echo nobody home && {\n \ttest the doohicky ?!AMP?!\n \tright now\n } &&\n-\n GIT_EXTERNAL_DIFF=echo git diff | {\n \tread path oldfile oldhex oldmode newfile newhex newmode &&\n \ttest \"z$oh\" = \"z$oldhex\"\ndiff --git a/t/chainlint/chained-subshell.expect b/t/chainlint/chained-subshell.expect\nindex af0369d328..2a44845759 100644\n--- a/t/chainlint/chained-subshell.expect\n+++ b/t/chainlint/chained-subshell.expect\n@@ -3,8 +3,7 @@ mkdir sub && (\n \tfoo the bar ?!AMP?!\n \tnuff said\n ) &&\n-\n-cut \"-d \" -f actual | ( read s1 s2 s3 &&\n+cut \"-d \" -f actual | (read s1 s2 s3 &&\n test -f $s1 ?!AMP?!\n test $(cat $s2) = tree2path1 &&\n-test $(cat $s3) = tree3path1 )\n+test $(cat $s3) = tree3path1)\ndiff --git a/t/chainlint/command-substitution-subsubshell.expect b/t/chainlint/command-substitution-subsubshell.expect\nindex ab2f79e845..ec42f2c30c 100644\n--- a/t/chainlint/command-substitution-subsubshell.expect\n+++ b/t/chainlint/command-substitution-subsubshell.expect\n@@ -1,2 +1,2 @@\n-OUT=$(( ( large_git 1 >& 3 ) | : ) 3 >& 1) &&\n+OUT=$( ((large_git 1>&3) | :) 3>&1 ) &&\n test_match_signal 13 \"$OUT\"\ndiff --git a/t/chainlint/cuddled.expect b/t/chainlint/cuddled.expect\nindex c3e0be4047..9558362917 100644\n--- a/t/chainlint/cuddled.expect\n+++ b/t/chainlint/cuddled.expect\n@@ -1,17 +1,13 @@\n (cd foo &&\n \tbar\n ) &&\n-\n (cd foo ?!AMP?!\n \tbar\n ) &&\n-\n (\n \tcd foo &&\n \tbar) &&\n-\n (cd foo &&\n \tbar) &&\n-\n (cd foo ?!AMP?!\n \tbar)\ndiff --git a/t/chainlint/dqstring-line-splice.expect b/t/chainlint/dqstring-line-splice.expect\nindex bf9ced60d4..f8fa3bc969 100644\n--- a/t/chainlint/dqstring-line-splice.expect\n+++ b/t/chainlint/dqstring-line-splice.expect\n@@ -1,3 +1,3 @@\n-echo 'fatal: reword option of --fixup is mutually exclusive with' '--patch/--interactive/--all/--include/--only' > expect &&\n-test_must_fail git commit --fixup=reword:HEAD~ $1 2 > actual &&\n+echo 'fatal: reword option of --fixup is mutually exclusive with'\t'--patch/--interactive/--all/--include/--only' >expect &&\n+test_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&\n test_cmp expect actual\ndiff --git a/t/chainlint/dqstring-no-interpolate.expect b/t/chainlint/dqstring-no-interpolate.expect\nindex 10724987a5..bd60121b47 100644\n--- a/t/chainlint/dqstring-no-interpolate.expect\n+++ b/t/chainlint/dqstring-no-interpolate.expect\n@@ -1,11 +1,8 @@\n grep \"^ ! [rejected][ ]*$BRANCH -> $BRANCH (non-fast-forward)$\" out &&\n-\n grep \"^\\.git$\" output.txt &&\n-\n-\n (\n \tcd client$version &&\n \tGIT_TEST_PROTOCOL_VERSION=$version git fetch-pack --no-progress .. $(cat ../input)\n-) > output &&\n-\tcut -d ' ' -f 2 < output | sort > actual &&\n+) >output &&\n+\tcut -d ' ' -f 2 actual &&\n \ttest_cmp expect actual\ndiff --git a/t/chainlint/empty-here-doc.expect b/t/chainlint/empty-here-doc.expect\nindex e8733c97c6..8507721192 100644\n--- a/t/chainlint/empty-here-doc.expect\n+++ b/t/chainlint/empty-here-doc.expect\n@@ -1,4 +1,4 @@\n-git ls-tree $tree path > current &&\n-cat > expected <<\\EOF &&\n+git ls-tree $tree path >current &&\n+cat >expected <<\\EOF &&\n EOF\n test_output\ndiff --git a/t/chainlint/exclamation.expect b/t/chainlint/exclamation.expect\nindex 2d961a58c6..765a35bb4c 100644\n--- a/t/chainlint/exclamation.expect\n+++ b/t/chainlint/exclamation.expect\n@@ -1,4 +1,4 @@\n-if ! condition ; then echo nope ; else yep ; fi &&\n+if ! condition; then echo nope; else yep; fi &&\n test_prerequisite !MINGW &&\n mail uucp!address &&\n echo !whatever!\ndiff --git a/t/chainlint/for-loop-abbreviated.expect b/t/chainlint/for-loop-abbreviated.expect\nindex a21007a63f..02c0d15cca 100644\n--- a/t/chainlint/for-loop-abbreviated.expect\n+++ b/t/chainlint/for-loop-abbreviated.expect\n@@ -1,5 +1,5 @@\n for it\n do\n-\tpath=$(expr \"$it\" : ( [^:]*) ) &&\n+\tpath=$(expr \"$it\" : ([^:]*)) &&\n \tgit update-index --add \"$path\" || exit\n done\ndiff --git a/t/chainlint/function.expect b/t/chainlint/function.expect\nindex a14388e6b9..826b2ccc95 100644\n--- a/t/chainlint/function.expect\n+++ b/t/chainlint/function.expect\n@@ -1,11 +1,9 @@\n-sha1_file ( ) {\n+sha1_file() {\n \techo \"$*\" | sed \"s#..#.git/objects/&/#\"\n } &&\n-\n-remove_object ( ) {\n+remove_object() {\n \tfile=$(sha1_file \"$*\") &&\n \ttest -e \"$file\" ?!AMP?!\n \trm -f \"$file\"\n } ?!AMP?!\n-\n sha1_file arg && remove_object arg\ndiff --git a/t/chainlint/here-doc-indent-operator.expect b/t/chainlint/here-doc-indent-operator.expect\nindex f92a7ce999..bbaad024d2 100644\n--- a/t/chainlint/here-doc-indent-operator.expect\n+++ b/t/chainlint/here-doc-indent-operator.expect\n@@ -3,9 +3,7 @@ header: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n num_commits: $1\n chunks: oid_fanout oid_lookup commit_metadata generation_data bloom_indexes bloom_data\n EOF\n-\n cat >expect << -EOF ?!AMP?!\n this is not indented\n -EOF\n-\n cleanup\ndiff --git a/t/chainlint/here-doc.expect b/t/chainlint/here-doc.expect\nindex 1df3f78282..9e612ac8b1 100644\n--- a/t/chainlint/here-doc.expect\n+++ b/t/chainlint/here-doc.expect\n@@ -1,22 +1,19 @@\n boodle wobba \\\n-\tgorgo snoot \\\n-\twafta snurb <foo &&\n snoz\n boz\n woz\n Arbitrary_Tag_42\n-\n cat <<\"zump\" >boo &&\n snoz\n boz\n woz\n zump\n-\n horticulture <<\\EOF\n gomez\n morticia\ndiff --git a/t/chainlint/inline-comment.expect b/t/chainlint/inline-comment.expect\nindex 6bad218530..c7fc411f91 100644\n--- a/t/chainlint/inline-comment.expect\n+++ b/t/chainlint/inline-comment.expect\n@@ -3,6 +3,5 @@\n \tbarfoo ?!AMP?! # wrong position for &&\n \tflibble \"not a # comment\"\n ) &&\n-\n (cd foo &&\n \tflibble \"not a # comment\")\ndiff --git a/t/chainlint/loop-detect-failure.expect b/t/chainlint/loop-detect-failure.expect\nindex a66025c39d..7a1a1ff0bf 100644\n--- a/t/chainlint/loop-detect-failure.expect\n+++ b/t/chainlint/loop-detect-failure.expect\n@@ -5,7 +5,6 @@ do\n \tgit -C r1 add file.$n &&\n \tgit -C r1 commit -m \"$n\" || return 1\n done &&\n-\n git init r2 &&\n for n in 1000 10000\n do\ndiff --git a/t/chainlint/loop-detect-status.expect b/t/chainlint/loop-detect-status.expect\nindex 24da9e86d5..7ce3a34806 100644\n--- a/t/chainlint/loop-detect-status.expect\n+++ b/t/chainlint/loop-detect-status.expect\n@@ -1,18 +1,18 @@\n-( while test $i -le $blobcount\n-do\n-\tprintf \"Generating blob $i/$blobcount\\r\" >& 2 &&\n+(while test $i -le $blobcount\n+ do\n+\tprintf \"Generating blob $i/$blobcount\\r\" >&2 &&\n \tprintf \"blob\\nmark :$i\\ndata $blobsize\\n\" &&\n \t#test-tool genrandom $i $blobsize &&\n \tprintf \"%-${blobsize}s\" $i &&\n \techo \"M 100644 :$i $i\" >> commit &&\n \ti=$(($i+1)) ||\n \techo $? > exit-status\n-done &&\n-echo \"commit refs/heads/main\" &&\n-echo \"author A U Thor 123456789 +0000\" &&\n-echo \"committer C O Mitter 123456789 +0000\" &&\n-echo \"data 5\" &&\n-echo \">2gb\" &&\n-cat commit ) |\n+ done &&\n+ echo \"commit refs/heads/main\" &&\n+ echo \"author A U Thor 123456789 +0000\" &&\n+ echo \"committer C O Mitter 123456789 +0000\" &&\n+ echo \"data 5\" &&\n+ echo \">2gb\" &&\n+ cat commit) |\n git fast-import --big-file-threshold=2 &&\n test ! -f exit-status\ndiff --git a/t/chainlint/negated-one-liner.expect b/t/chainlint/negated-one-liner.expect\nindex ad4c2d949e..7ae60eb770 100644\n--- a/t/chainlint/negated-one-liner.expect\n+++ b/t/chainlint/negated-one-liner.expect\n@@ -1,5 +1,4 @@\n ! (foo && bar) &&\n ! (foo && bar) >baz &&\n-\n ! (foo; ?!AMP?! bar) &&\n ! (foo; ?!AMP?! bar) >baz\ndiff --git a/t/chainlint/nested-here-doc.expect b/t/chainlint/nested-here-doc.expect\nindex 29b3832a98..93eeded9f4 100644\n--- a/t/chainlint/nested-here-doc.expect\n+++ b/t/chainlint/nested-here-doc.expect\n@@ -6,7 +6,6 @@ fub < \"path$i$j\" ?!LOOP?!\n+\t\techo \"$i$j\" >\"path$i$j\" ?!LOOP?!\n \tdone ?!LOOP?!\n done &&\n-\n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" || return 1\n+\t\techo \"$i$j\" >\"path$i$j\" || return 1\n \tdone\n done &&\n-\n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" ?!LOOP?!\n+\t\techo \"$i$j\" >\"path$i$j\" ?!LOOP?!\n \tdone || return 1\n done &&\n-\n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" || return 1\n+\t\techo \"$i$j\" >\"path$i$j\" || return 1\n \tdone || return 1\n done\ndiff --git a/t/chainlint/not-heredoc.expect b/t/chainlint/not-heredoc.expect\nindex 2e9bb135fe..47311f4c2d 100644\n--- a/t/chainlint/not-heredoc.expect\n+++ b/t/chainlint/not-heredoc.expect\n@@ -3,7 +3,6 @@ echo ourside &&\n echo \"=======\" &&\n echo theirside &&\n echo \">>>>>>> theirs\" &&\n-\n (\n \techo \"<<<<<<< ours\" &&\n \techo ourside &&\ndiff --git a/t/chainlint/one-liner.expect b/t/chainlint/one-liner.expect\nindex 57a7a444c1..cdb45231f1 100644\n--- a/t/chainlint/one-liner.expect\n+++ b/t/chainlint/one-liner.expect\n@@ -1,9 +1,7 @@\n (foo && bar) &&\n (foo && bar) |\n (foo && bar) >baz &&\n-\n (foo; ?!AMP?! bar) &&\n (foo; ?!AMP?! bar) |\n (foo; ?!AMP?! bar) >baz &&\n-\n (foo \"bar; baz\")\ndiff --git a/t/chainlint/subshell-here-doc.expect b/t/chainlint/subshell-here-doc.expect\nindex 52789278d1..fa7cb0288a 100644\n--- a/t/chainlint/subshell-here-doc.expect\n+++ b/t/chainlint/subshell-here-doc.expect\n@@ -1,15 +1,13 @@\n (\n \techo wobba \\\n-\t\tgorgo snoot \\\n-\t\twafta snurb <<-EOF &&\n+\t gorgo snoot \\\n+\t wafta snurb <<-EOF &&\n \tquoth the raven,\n \tnevermore...\n \tEOF\n-\n \tcat <bip ?!AMP?!\n \tfish fly high\n EOF\n-\n \techo <<-\\EOF >bop\n \tgomez\n \tmorticia\ndiff --git a/t/chainlint/token-pasting.expect b/t/chainlint/token-pasting.expect\nindex 342360bcd0..cda7d54037 100644\n--- a/t/chainlint/token-pasting.expect\n+++ b/t/chainlint/token-pasting.expect\n@@ -1,26 +1,22 @@\n git config filter.rot13.smudge ./rot13.sh &&\n git config filter.rot13.clean ./rot13.sh &&\n-\n {\n echo \"*.t filter=rot13\" ?!AMP?!\n echo \"*.i ident\"\n-} > .gitattributes &&\n-\n+} >.gitattributes &&\n {\n echo a b c d e f g h i j k l m ?!AMP?!\n echo n o p q r s t u v w x y z ?!AMP?!\n echo '$Id$'\n-} > test &&\n-cat test > test.t &&\n-cat test > test.o &&\n-cat test > test.i &&\n+} >test &&\n+cat test >test.t &&\n+cat test >test.o &&\n+cat test >test.i &&\n git add test test.t test.i &&\n rm -f test test.t test.i &&\n git checkout -- test test.t test.i &&\n-\n-echo \"content-test2\" > test2.o &&\n-echo \"content-test3 - filename with special characters\" > \"test3 'sq',$x=.o\" ?!AMP?!\n-\n+echo \"content-test2\" >test2.o &&\n+echo \"content-test3 - filename with special characters\" >\"test3 'sq',$x=.o\" ?!AMP?!\n downstream_url_for_sed=$(\n \tprintf \"%sn\" \"$downstream_url\" |\n \tsed -e 's/\\/\\\\/g' -e 's/[[/.*^$]/\\&/g'\n-- \n2.43.GIT\n\n"},{"id":"485706","messageId":"CAPig+cQozi+aiTc5Bve4OHrfuSRGUCSkKmhoYtkGTmn64Ps-rw@mail.gmail.com","threadId":"60604","inReplyTo":"8c3e1cb5eae13210070cc14f5f2a3f8c0dfc39c3.1702620230.git.ps@pks.im","subject":"Re: [PATCH v3] tests: adjust whitespace in chainlint expectations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-15T06:24:20Z","receivedAt":"2023-12-15T06:24:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 15, 2023 at 1:04 AM Patrick Steinhardt wrote:\n> [...]\n> Instead of improving the detection logic, fix our \".expect\" files so\n> that we do not need any post-processing at all anymore. This allows us\n> to drop the `-w` flag when diffing so that we can always use diff(1)\n> now.\n>\n> Note that we leave the post-processing of `chainlint.pl` output intact.\n> All we do here is to strip leading line numbers that it would otherwise\n> generate.\n\nHmm, okay, but... (see below)\n\n> Having these would cause a rippling effect whenever we add a\n> new test that sorts into the middle of existing tests and would require\n> us to renumerate all subsequent lines, which seems rather pointless.\n\nJust an aside, not strictly relevant at this time: Ævar has proposed\nthat check-chainlint should not be creating conglomerate \"test\",\n\"expect\", and \"actual\" files, but should instead let `make` run\nchainlint.pl separately on each chainlint self-test file, thus\nbenefiting from `make`'s innate parallelism rather than baking\nparallelism into chainlint.pl itself. More importantly, `make`'s\ndependency tracking would ensure that a chainlint self-test file only\ngets rechecked if its timestamp changes. That differs from the current\nsituation in which _all_ of the chainlint self-test files are checked\non _every_ `make test` which is wasteful if none of them have changed.\nAnyhow, with his proposed approach, there wouldn't be cascading line\nnumber changes just because a new self-test file was added.\n\n> Signed-off-by: Patrick Steinhardt \n> ---\n> diff --git a/t/Makefile b/t/Makefile\n> @@ -103,20 +103,12 @@ check-chainlint:\n> $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n> sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n\nThe commit message claims that this is only stripping the line numbers\nwhich prefix each emitted line, but the `/^[ ]*$$/d` bit is also\ndeleting blank lines from the output of chainlint.pl. Thus, this ought\nto be:\n\n sed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \\\n"},{"id":"485707","messageId":"ZXvyL2wtoTIt6OVc@tanuki","threadId":"60604","inReplyTo":"CAPig+cQozi+aiTc5Bve4OHrfuSRGUCSkKmhoYtkGTmn64Ps-rw@mail.gmail.com","subject":"Re: [PATCH v3] tests: adjust whitespace in chainlint expectations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-15T06:29:03Z","receivedAt":"2023-12-15T06:29:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 15, 2023 at 01:24:20AM -0500, Eric Sunshine wrote:\n> On Fri, Dec 15, 2023 at 1:04 AM Patrick Steinhardt wrote:\n> > [...]\n> > Instead of improving the detection logic, fix our \".expect\" files so\n> > that we do not need any post-processing at all anymore. This allows us\n> > to drop the `-w` flag when diffing so that we can always use diff(1)\n> > now.\n> >\n> > Note that we leave the post-processing of `chainlint.pl` output intact.\n> > All we do here is to strip leading line numbers that it would otherwise\n> > generate.\n> \n> Hmm, okay, but... (see below)\n> \n> > Having these would cause a rippling effect whenever we add a\n> > new test that sorts into the middle of existing tests and would require\n> > us to renumerate all subsequent lines, which seems rather pointless.\n> \n> Just an aside, not strictly relevant at this time: Ævar has proposed\n> that check-chainlint should not be creating conglomerate \"test\",\n> \"expect\", and \"actual\" files, but should instead let `make` run\n> chainlint.pl separately on each chainlint self-test file, thus\n> benefiting from `make`'s innate parallelism rather than baking\n> parallelism into chainlint.pl itself. More importantly, `make`'s\n> dependency tracking would ensure that a chainlint self-test file only\n> gets rechecked if its timestamp changes. That differs from the current\n> situation in which _all_ of the chainlint self-test files are checked\n> on _every_ `make test` which is wasteful if none of them have changed.\n> Anyhow, with his proposed approach, there wouldn't be cascading line\n> number changes just because a new self-test file was added.\n\nI was indeed also thinking along this way and would tend to agree. I\npunted on it as I honestly only really care for fixing the immediate\nissue that the post-processing causes for me.\n\nAre you fine with deferring this bigger refactoring?\n\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > diff --git a/t/Makefile b/t/Makefile\n> > @@ -103,20 +103,12 @@ check-chainlint:\n> > $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n> > sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n> \n> The commit message claims that this is only stripping the line numbers\n> which prefix each emitted line, but the `/^[ ]*$$/d` bit is also\n> deleting blank lines from the output of chainlint.pl. Thus, this ought\n> to be:\n> \n> sed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \\\n\nGah, you're right, I missed the second part. Will fix in another round.\n\nPatrick\n"},{"id":"485708","messageId":"CAPig+cSBjsosRqoAafYN94Cco8+7SdUt0ND_jHS+jVPoM4K0JA@mail.gmail.com","threadId":"60604","inReplyTo":"ZXvyL2wtoTIt6OVc@tanuki","subject":"Re: [PATCH v3] tests: adjust whitespace in chainlint expectations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-15T06:40:33Z","receivedAt":"2023-12-15T06:40:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 15, 2023 at 1:29 AM Patrick Steinhardt wrote:\n> On Fri, Dec 15, 2023 at 01:24:20AM -0500, Eric Sunshine wrote:\n> > Just an aside, not strictly relevant at this time: Ævar has proposed\n> > that check-chainlint should not be creating conglomerate \"test\",\n> > \"expect\", and \"actual\" files, but should instead let `make` run\n> > chainlint.pl separately on each chainlint self-test file, thus\n> > benefiting from `make`'s innate parallelism rather than baking\n> > parallelism into chainlint.pl itself. More importantly, `make`'s\n> > dependency tracking would ensure that a chainlint self-test file only\n> > gets rechecked if its timestamp changes. That differs from the current\n> > situation in which _all_ of the chainlint self-test files are checked\n> > on _every_ `make test` which is wasteful if none of them have changed.\n> > Anyhow, with his proposed approach, there wouldn't be cascading line\n> > number changes just because a new self-test file was added.\n>\n> I was indeed also thinking along this way and would tend to agree. I\n> punted on it as I honestly only really care for fixing the immediate\n> issue that the post-processing causes for me.\n>\n> Are you fine with deferring this bigger refactoring?\n\nOh, yes, of course. Please don't read my aside-comment as a suggestion\nthat you should tackle such a change or that there is any urgent need\nto change how this all works. The currently proposed simple\nsolution(s) to allow you to get past this issue are perfectly\nacceptable.\n\n(As a further aside, just for completeness since I already mentioned\npart of his plan, Ævar also really intended that the test scripts\nthemselves, \"t/t????-*.sh\", should be run individually through\nchainlint.pl by the Makefile rather than sending all of them through\nit in one go, thus once again taking advantage of `make`'s innate\nparallelism, and only checking for &&-chain breakage if a test\nscript's timestamp has actually changed rather than checking all test\nscripts unconditionally on every `make test`[1,2,3].)\n\n[1]: https://lore.kernel.org/git/220901.86bkrzjm6e.gmgdl@evledraar.gmail.com/\n[2]: https://lore.kernel.org/git/221122.86cz9fbyln.gmgdl@evledraar.gmail.com/\n[3]: https://github.com/avar/git/commit/ff8b4a12b30ac5ca521a64e74b0fd968ab2d4585\n"},{"id":"485709","messageId":"fb312f559de7b99244e4c86a995250599cd9be06.1702622508.git.ps@pks.im","threadId":"60604","inReplyTo":"4112adbe467c14a8f22a87ea41aa4705f8760cf6.1702380646.git.ps@pks.im","subject":"[PATCH v4] tests: adjust whitespace in chainlint expectations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-15T06:42:47Z","receivedAt":"2023-12-15T06:42:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"check-chainlint\" target runs automatically when running tests and\nperforms self-checks to verify that the chainlinter itself produces the\nexpected output. Originally, the chainlinter was implemented via sed,\nbut the infrastructure has been rewritten in fb41727b7e (t: retire\nunused chainlint.sed, 2022-09-01) to use a Perl script instead.\n\nThe rewrite caused some slight whitespace changes in the output that are\nultimately not of much importance. In order to be able to assert that\nthe actual chainlinter errors match our expectations we thus have to\nignore whitespace characters when diffing them. As the `-w` flag is not\nin POSIX we try to use `git diff -w --no-index` before we fall back to\n`diff -w -u`.\n\nTo accomodate for cases where the host system has no Git installation we\nuse the locally-compiled version of Git. This can result in problems\nthough when the Git project's repository is using extensions that the\nlocally-compiled version of Git doesn't understand. It will refuse to\nrun and thus cause the checks to fail.\n\nInstead of improving the detection logic, fix our \".expect\" files so\nthat we do not need any post-processing at all anymore. This allows us\nto drop the `-w` flag when diffing so that we can always use diff(1)\nnow.\n\nNote that we keep some of the post-processing of `chainlint.pl` output\nintact to strip leading line numbers generated by the script. Having\nthese would cause a rippling effect whenever we add a new test that\nsorts into the middle of existing tests and would require us to\nrenumerate all subsequent lines, which seems rather pointless.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/Makefile | 14 +++--------\n t/chainlint/blank-line-before-esac.expect | 8 +++----\n t/chainlint/blank-line.expect | 4 ++++\n t/chainlint/block.expect | 4 ++--\n t/chainlint/chain-break-background.expect | 4 ++--\n t/chainlint/chain-break-return-exit.expect | 14 +++++------\n t/chainlint/chain-break-status.expect | 4 ++--\n t/chainlint/chained-subshell.expect | 4 ++--\n .../command-substitution-subsubshell.expect | 2 +-\n t/chainlint/dqstring-line-splice.expect | 6 +++--\n t/chainlint/dqstring-no-interpolate.expect | 5 ++--\n t/chainlint/empty-here-doc.expect | 4 ++--\n t/chainlint/exclamation.expect | 2 +-\n t/chainlint/for-loop-abbreviated.expect | 2 +-\n t/chainlint/for-loop.expect | 1 +\n t/chainlint/function.expect | 4 ++--\n t/chainlint/here-doc.expect | 4 ++--\n t/chainlint/loop-detect-status.expect | 20 ++++++++--------\n t/chainlint/nested-cuddled-subshell.expect | 6 +++++\n t/chainlint/nested-loop-detect-failure.expect | 24 +++++++++----------\n t/chainlint/nested-subshell.expect | 1 +\n t/chainlint/pipe.expect | 2 ++\n t/chainlint/subshell-here-doc.expect | 4 ++--\n t/chainlint/subshell-one-liner.expect | 5 ++++\n t/chainlint/t7900-subtree.expect | 1 +\n t/chainlint/token-pasting.expect | 14 +++++------\n t/chainlint/while-loop.expect | 1 +\n 27 files changed, 90 insertions(+), 74 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 225aaf78ed..b7a6fefe28 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -103,20 +103,12 @@ check-chainlint:\n \t\techo \"# chainlint: $(CHAINLINTTMP_SQ)/tests\" && \\\n \t\tfor i in $(CHAINLINTTESTS); do \\\n \t\t\techo \"# chainlint: $$i\" && \\\n-\t\t\tsed -e '/^[ \t]*$$/d' chainlint/$$i.expect; \\\n+\t\t\tcat chainlint/$$i.expect; \\\n \t\tdone \\\n \t} >'$(CHAINLINTTMP_SQ)'/expect && \\\n \t$(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n-\t\tsed -e 's/^[1-9][0-9]* //;/^[ \t]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n-\tif test -f ../GIT-BUILD-OPTIONS; then \\\n-\t\t. ../GIT-BUILD-OPTIONS; \\\n-\tfi && \\\n-\tif test -x ../git$$X; then \\\n-\t\tDIFFW=\"../git$$X --no-pager diff -w --no-index\"; \\\n-\telse \\\n-\t\tDIFFW=\"diff -w -u\"; \\\n-\tfi && \\\n-\t$$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n+\t\tsed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \\\n+\tdiff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n \n test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \\\n \ttest-lint-filenames\ndiff --git a/t/chainlint/blank-line-before-esac.expect b/t/chainlint/blank-line-before-esac.expect\nindex 48ed4eb124..056e03003d 100644\n--- a/t/chainlint/blank-line-before-esac.expect\n+++ b/t/chainlint/blank-line-before-esac.expect\n@@ -1,11 +1,11 @@\n-test_done ( ) {\n+test_done () {\n \tcase \"$test_failure\" in\n-\t0 )\n+\t0)\n \t\ttest_at_end_hook_\n \n \t\texit 0 ;;\n \n-\t* )\n+\t*)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n \t\t\tsay_color error \"# failed $test_failure among $msg\"\n@@ -14,5 +14,5 @@ test_done ( ) {\n \n \t\texit 1 ;;\n \n-\t\tesac\n+\tesac\n }\ndiff --git a/t/chainlint/blank-line.expect b/t/chainlint/blank-line.expect\nindex f76fde1ffb..b47827d749 100644\n--- a/t/chainlint/blank-line.expect\n+++ b/t/chainlint/blank-line.expect\n@@ -1,4 +1,8 @@\n (\n+\n \tnothing &&\n+\n \tsomething\n+\n+\n )\ndiff --git a/t/chainlint/block.expect b/t/chainlint/block.expect\nindex a3bcea492a..1c87326364 100644\n--- a/t/chainlint/block.expect\n+++ b/t/chainlint/block.expect\n@@ -12,9 +12,9 @@\n ) &&\n \n {\n-\techo a ; ?!AMP?! echo b\n+\techo a; ?!AMP?! echo b\n } &&\n-{ echo a ; ?!AMP?! echo b ; } &&\n+{ echo a; ?!AMP?! echo b; } &&\n \n {\n \techo \"${var}9\" &&\ndiff --git a/t/chainlint/chain-break-background.expect b/t/chainlint/chain-break-background.expect\nindex 28f9114f42..20d0bb5333 100644\n--- a/t/chainlint/chain-break-background.expect\n+++ b/t/chainlint/chain-break-background.expect\n@@ -1,9 +1,9 @@\n JGIT_DAEMON_PID= &&\n git init --bare empty.git &&\n-> empty.git/git-daemon-export-ok &&\n+>empty.git/git-daemon-export-ok &&\n mkfifo jgit_daemon_output &&\n {\n-\tjgit daemon --port=\"$JGIT_DAEMON_PORT\" . > jgit_daemon_output &\n+\tjgit daemon --port=\"$JGIT_DAEMON_PORT\" . >jgit_daemon_output &\n \tJGIT_DAEMON_PID=$!\n } &&\n test_expect_code 2 git ls-remote --exit-code git://localhost:$JGIT_DAEMON_PORT/empty.git\ndiff --git a/t/chainlint/chain-break-return-exit.expect b/t/chainlint/chain-break-return-exit.expect\nindex 1732d221c3..4cd18e2edf 100644\n--- a/t/chainlint/chain-break-return-exit.expect\n+++ b/t/chainlint/chain-break-return-exit.expect\n@@ -1,16 +1,16 @@\n case \"$(git ls-files)\" in\n-one ) echo pass one ;;\n-* ) echo bad one ; return 1 ;;\n+one) echo pass one ;;\n+*) echo bad one; return 1 ;;\n esac &&\n (\n \tcase \"$(git ls-files)\" in\n-\ttwo ) echo pass two ;;\n-\t* ) echo bad two ; exit 1 ;;\n-esac\n+\ttwo) echo pass two ;;\n+\t*) echo bad two; exit 1 ;;\n+\tesac\n ) &&\n case \"$(git ls-files)\" in\n-dir/two\"$LF\"one ) echo pass both ;;\n-* ) echo bad ; return 1 ;;\n+dir/two\"$LF\"one) echo pass both ;;\n+*) echo bad; return 1 ;;\n esac &&\n \n for i in 1 2 3 4 ; do\ndiff --git a/t/chainlint/chain-break-status.expect b/t/chainlint/chain-break-status.expect\nindex f4bada9463..e6b3b2193e 100644\n--- a/t/chainlint/chain-break-status.expect\n+++ b/t/chainlint/chain-break-status.expect\n@@ -1,7 +1,7 @@\n-OUT=$(( ( large_git ; echo $? 1 >& 3 ) | : ) 3 >& 1) &&\n+OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n test_match_signal 13 \"$OUT\" &&\n \n-{ test-tool sigchain > actual ; ret=$? ; } &&\n+{ test-tool sigchain >actual; ret=$?; } &&\n {\n \ttest_match_signal 15 \"$ret\" ||\n \ttest \"$ret\" = 3\ndiff --git a/t/chainlint/chained-subshell.expect b/t/chainlint/chained-subshell.expect\nindex af0369d328..83810ea7ec 100644\n--- a/t/chainlint/chained-subshell.expect\n+++ b/t/chainlint/chained-subshell.expect\n@@ -4,7 +4,7 @@ mkdir sub && (\n \tnuff said\n ) &&\n \n-cut \"-d \" -f actual | ( read s1 s2 s3 &&\n+cut \"-d \" -f actual | (read s1 s2 s3 &&\n test -f $s1 ?!AMP?!\n test $(cat $s2) = tree2path1 &&\n-test $(cat $s3) = tree3path1 )\n+test $(cat $s3) = tree3path1)\ndiff --git a/t/chainlint/command-substitution-subsubshell.expect b/t/chainlint/command-substitution-subsubshell.expect\nindex ab2f79e845..ec42f2c30c 100644\n--- a/t/chainlint/command-substitution-subsubshell.expect\n+++ b/t/chainlint/command-substitution-subsubshell.expect\n@@ -1,2 +1,2 @@\n-OUT=$(( ( large_git 1 >& 3 ) | : ) 3 >& 1) &&\n+OUT=$( ((large_git 1>&3) | :) 3>&1 ) &&\n test_match_signal 13 \"$OUT\"\ndiff --git a/t/chainlint/dqstring-line-splice.expect b/t/chainlint/dqstring-line-splice.expect\nindex bf9ced60d4..37eab80738 100644\n--- a/t/chainlint/dqstring-line-splice.expect\n+++ b/t/chainlint/dqstring-line-splice.expect\n@@ -1,3 +1,5 @@\n-echo 'fatal: reword option of --fixup is mutually exclusive with' '--patch/--interactive/--all/--include/--only' > expect &&\n-test_must_fail git commit --fixup=reword:HEAD~ $1 2 > actual &&\n+\n+echo 'fatal: reword option of --fixup is mutually exclusive with'\t'--patch/--interactive/--all/--include/--only' >expect &&\n+test_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&\n test_cmp expect actual\n+\ndiff --git a/t/chainlint/dqstring-no-interpolate.expect b/t/chainlint/dqstring-no-interpolate.expect\nindex 10724987a5..087eda15e4 100644\n--- a/t/chainlint/dqstring-no-interpolate.expect\n+++ b/t/chainlint/dqstring-no-interpolate.expect\n@@ -6,6 +6,7 @@ grep \"^\\.git$\" output.txt &&\n (\n \tcd client$version &&\n \tGIT_TEST_PROTOCOL_VERSION=$version git fetch-pack --no-progress .. $(cat ../input)\n-) > output &&\n-\tcut -d ' ' -f 2 < output | sort > actual &&\n+) >output &&\n+\tcut -d ' ' -f 2 actual &&\n \ttest_cmp expect actual\n+\ndiff --git a/t/chainlint/empty-here-doc.expect b/t/chainlint/empty-here-doc.expect\nindex e8733c97c6..8507721192 100644\n--- a/t/chainlint/empty-here-doc.expect\n+++ b/t/chainlint/empty-here-doc.expect\n@@ -1,4 +1,4 @@\n-git ls-tree $tree path > current &&\n-cat > expected <<\\EOF &&\n+git ls-tree $tree path >current &&\n+cat >expected <<\\EOF &&\n EOF\n test_output\ndiff --git a/t/chainlint/exclamation.expect b/t/chainlint/exclamation.expect\nindex 2d961a58c6..765a35bb4c 100644\n--- a/t/chainlint/exclamation.expect\n+++ b/t/chainlint/exclamation.expect\n@@ -1,4 +1,4 @@\n-if ! condition ; then echo nope ; else yep ; fi &&\n+if ! condition; then echo nope; else yep; fi &&\n test_prerequisite !MINGW &&\n mail uucp!address &&\n echo !whatever!\ndiff --git a/t/chainlint/for-loop-abbreviated.expect b/t/chainlint/for-loop-abbreviated.expect\nindex a21007a63f..02c0d15cca 100644\n--- a/t/chainlint/for-loop-abbreviated.expect\n+++ b/t/chainlint/for-loop-abbreviated.expect\n@@ -1,5 +1,5 @@\n for it\n do\n-\tpath=$(expr \"$it\" : ( [^:]*) ) &&\n+\tpath=$(expr \"$it\" : ([^:]*)) &&\n \tgit update-index --add \"$path\" || exit\n done\ndiff --git a/t/chainlint/for-loop.expect b/t/chainlint/for-loop.expect\nindex d65c82129a..d2237f1e38 100644\n--- a/t/chainlint/for-loop.expect\n+++ b/t/chainlint/for-loop.expect\n@@ -6,6 +6,7 @@\n \t\tbar\n \t\tEOF\n \tdone ?!AMP?!\n+\n \tfor i in a b c; do\n \t\techo $i &&\n \t\tcat $i ?!LOOP?!\ndiff --git a/t/chainlint/function.expect b/t/chainlint/function.expect\nindex a14388e6b9..dd7c997a3c 100644\n--- a/t/chainlint/function.expect\n+++ b/t/chainlint/function.expect\n@@ -1,8 +1,8 @@\n-sha1_file ( ) {\n+sha1_file() {\n \techo \"$*\" | sed \"s#..#.git/objects/&/#\"\n } &&\n \n-remove_object ( ) {\n+remove_object() {\n \tfile=$(sha1_file \"$*\") &&\n \ttest -e \"$file\" ?!AMP?!\n \trm -f \"$file\"\ndiff --git a/t/chainlint/here-doc.expect b/t/chainlint/here-doc.expect\nindex 1df3f78282..91b961242a 100644\n--- a/t/chainlint/here-doc.expect\n+++ b/t/chainlint/here-doc.expect\n@@ -1,6 +1,6 @@\n boodle wobba \\\n-\tgorgo snoot \\\n-\twafta snurb <& 2 &&\n+(while test $i -le $blobcount\n+ do\n+\tprintf \"Generating blob $i/$blobcount\\r\" >&2 &&\n \tprintf \"blob\\nmark :$i\\ndata $blobsize\\n\" &&\n \t#test-tool genrandom $i $blobsize &&\n \tprintf \"%-${blobsize}s\" $i &&\n \techo \"M 100644 :$i $i\" >> commit &&\n \ti=$(($i+1)) ||\n \techo $? > exit-status\n-done &&\n-echo \"commit refs/heads/main\" &&\n-echo \"author A U Thor 123456789 +0000\" &&\n-echo \"committer C O Mitter 123456789 +0000\" &&\n-echo \"data 5\" &&\n-echo \">2gb\" &&\n-cat commit ) |\n+ done &&\n+ echo \"commit refs/heads/main\" &&\n+ echo \"author A U Thor 123456789 +0000\" &&\n+ echo \"committer C O Mitter 123456789 +0000\" &&\n+ echo \"data 5\" &&\n+ echo \">2gb\" &&\n+ cat commit) |\n git fast-import --big-file-threshold=2 &&\n test ! -f exit-status\ndiff --git a/t/chainlint/nested-cuddled-subshell.expect b/t/chainlint/nested-cuddled-subshell.expect\nindex 2a86885ee6..3836049cc4 100644\n--- a/t/chainlint/nested-cuddled-subshell.expect\n+++ b/t/chainlint/nested-cuddled-subshell.expect\n@@ -2,18 +2,24 @@\n \t(cd foo &&\n \t\tbar\n \t) &&\n+\n \t(cd foo &&\n \t\tbar\n \t) ?!AMP?!\n+\n \t(\n \t\tcd foo &&\n \t\tbar) &&\n+\n \t(\n \t\tcd foo &&\n \t\tbar) ?!AMP?!\n+\n \t(cd foo &&\n \t\tbar) &&\n+\n \t(cd foo &&\n \t\tbar) ?!AMP?!\n+\n \tfoobar\n )\ndiff --git a/t/chainlint/nested-loop-detect-failure.expect b/t/chainlint/nested-loop-detect-failure.expect\nindex 4793a0e8e1..3461df40e5 100644\n--- a/t/chainlint/nested-loop-detect-failure.expect\n+++ b/t/chainlint/nested-loop-detect-failure.expect\n@@ -1,31 +1,31 @@\n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" ?!LOOP?!\n+\t\techo \"$i$j\" >\"path$i$j\" ?!LOOP?!\n \tdone ?!LOOP?!\n done &&\n \n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" || return 1\n+\t\techo \"$i$j\" >\"path$i$j\" || return 1\n \tdone\n done &&\n \n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" ?!LOOP?!\n+\t\techo \"$i$j\" >\"path$i$j\" ?!LOOP?!\n \tdone || return 1\n done &&\n \n-for i in 0 1 2 3 4 5 6 7 8 9 ;\n+for i in 0 1 2 3 4 5 6 7 8 9;\n do\n-\tfor j in 0 1 2 3 4 5 6 7 8 9 ;\n+\tfor j in 0 1 2 3 4 5 6 7 8 9;\n \tdo\n-\t\techo \"$i$j\" > \"path$i$j\" || return 1\n+\t\techo \"$i$j\" >\"path$i$j\" || return 1\n \tdone || return 1\n done\ndiff --git a/t/chainlint/nested-subshell.expect b/t/chainlint/nested-subshell.expect\nindex 02e0a9f1bb..73ff28546a 100644\n--- a/t/chainlint/nested-subshell.expect\n+++ b/t/chainlint/nested-subshell.expect\n@@ -4,6 +4,7 @@\n \t\techo a &&\n \t\techo b\n \t) >file &&\n+\n \tcd foo &&\n \t(\n \t\techo a ?!AMP?!\ndiff --git a/t/chainlint/pipe.expect b/t/chainlint/pipe.expect\nindex 2cfc028297..811971b1a3 100644\n--- a/t/chainlint/pipe.expect\n+++ b/t/chainlint/pipe.expect\n@@ -2,7 +2,9 @@\n \tfoo |\n \tbar |\n \tbaz &&\n+\n \tfish |\n \tcow ?!AMP?!\n+\n \tsunder\n )\ndiff --git a/t/chainlint/subshell-here-doc.expect b/t/chainlint/subshell-here-doc.expect\nindex 52789278d1..75d6f607e2 100644\n--- a/t/chainlint/subshell-here-doc.expect\n+++ b/t/chainlint/subshell-here-doc.expect\n@@ -1,7 +1,7 @@\n (\n \techo wobba \\\n-\t\tgorgo snoot \\\n-\t\twafta snurb <<-EOF &&\n+\t gorgo snoot \\\n+\t wafta snurb <<-EOF &&\n \tquoth the raven,\n \tnevermore...\n \tEOF\ndiff --git a/t/chainlint/subshell-one-liner.expect b/t/chainlint/subshell-one-liner.expect\nindex b7015361bf..8f694990e8 100644\n--- a/t/chainlint/subshell-one-liner.expect\n+++ b/t/chainlint/subshell-one-liner.expect\n@@ -2,13 +2,18 @@\n \t(foo && bar) &&\n \t(foo && bar) |\n \t(foo && bar) >baz &&\n+\n \t(foo; ?!AMP?! bar) &&\n \t(foo; ?!AMP?! bar) |\n \t(foo; ?!AMP?! bar) >baz &&\n+\n \t(foo || exit 1) &&\n \t(foo || exit 1) |\n \t(foo || exit 1) >baz &&\n+\n \t(foo && bar) ?!AMP?!\n+\n \t(foo && bar; ?!AMP?! baz) ?!AMP?!\n+\n \tfoobar\n )\ndiff --git a/t/chainlint/t7900-subtree.expect b/t/chainlint/t7900-subtree.expect\nindex 71b3b3bc20..02f3129232 100644\n--- a/t/chainlint/t7900-subtree.expect\n+++ b/t/chainlint/t7900-subtree.expect\n@@ -15,6 +15,7 @@ main-sub4\" &&\n $chkms\n TXT\n ) &&\n+\n \tsubfiles=$(git ls-files) &&\n \tcheck_equal \"$subfiles\" \"$chkms\n $chks\"\ndiff --git a/t/chainlint/token-pasting.expect b/t/chainlint/token-pasting.expect\nindex 342360bcd0..6a387917a7 100644\n--- a/t/chainlint/token-pasting.expect\n+++ b/t/chainlint/token-pasting.expect\n@@ -4,22 +4,22 @@ git config filter.rot13.clean ./rot13.sh &&\n {\n echo \"*.t filter=rot13\" ?!AMP?!\n echo \"*.i ident\"\n-} > .gitattributes &&\n+} >.gitattributes &&\n \n {\n echo a b c d e f g h i j k l m ?!AMP?!\n echo n o p q r s t u v w x y z ?!AMP?!\n echo '$Id$'\n-} > test &&\n-cat test > test.t &&\n-cat test > test.o &&\n-cat test > test.i &&\n+} >test &&\n+cat test >test.t &&\n+cat test >test.o &&\n+cat test >test.i &&\n git add test test.t test.i &&\n rm -f test test.t test.i &&\n git checkout -- test test.t test.i &&\n \n-echo \"content-test2\" > test2.o &&\n-echo \"content-test3 - filename with special characters\" > \"test3 'sq',$x=.o\" ?!AMP?!\n+echo \"content-test2\" >test2.o &&\n+echo \"content-test3 - filename with special characters\" >\"test3 'sq',$x=.o\" ?!AMP?!\n \n downstream_url_for_sed=$(\n \tprintf \"%sn\" \"$downstream_url\" |\ndiff --git a/t/chainlint/while-loop.expect b/t/chainlint/while-loop.expect\nindex 1f5eaea0fd..06c1567f48 100644\n--- a/t/chainlint/while-loop.expect\n+++ b/t/chainlint/while-loop.expect\n@@ -6,6 +6,7 @@\n \t\tbar\n \t\tEOF\n \tdone ?!AMP?!\n+\n \twhile true; do\n \t\techo foo &&\n \t\tcat bar ?!LOOP?!\n-- \n2.43.GIT\n\n"},{"id":"485710","messageId":"CAPig+cSNZ75WSjxfJFoVPBTkv7yQpxGz1c4ugCyQRLytpNLjig@mail.gmail.com","threadId":"60604","inReplyTo":"fb312f559de7b99244e4c86a995250599cd9be06.1702622508.git.ps@pks.im","subject":"Re: [PATCH v4] tests: adjust whitespace in chainlint expectations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-15T07:17:20Z","receivedAt":"2023-12-15T07:17:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 15, 2023 at 1:42 AM Patrick Steinhardt wrote:\n> [...]\n> Instead of improving the detection logic, fix our \".expect\" files so\n> that we do not need any post-processing at all anymore. This allows us\n> to drop the `-w` flag when diffing so that we can always use diff(1)\n> now.\n>\n> Note that we keep some of the post-processing of `chainlint.pl` output\n> intact to strip leading line numbers generated by the script. Having\n> these would cause a rippling effect whenever we add a new test that\n> sorts into the middle of existing tests and would require us to\n> renumerate all subsequent lines, which seems rather pointless.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> diff --git a/t/Makefile b/t/Makefile\n> @@ -103,20 +103,12 @@ check-chainlint:\n> for i in $(CHAINLINTTESTS); do \\\n> echo \"# chainlint: $$i\" && \\\n> - sed -e '/^[ ]*$$/d' chainlint/$$i.expect; \\\n> + cat chainlint/$$i.expect; \\\n> done \\\n> $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n> - sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n> + sed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \\\n> + diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual\n\nThanks, this version looks fine. FWIW, you may consider this:\n\n Reviewed-by: Eric Sunshine \n\nAside: I was rather surprised to see this output from git-am when applying v4:\n\n Applying: tests: adjust whitespace in chainlint expectations\n .git/rebase-apply/patch:205: new blank line at EOF.\n +\n .git/rebase-apply/patch:219: new blank line at EOF.\n +\n warning: 2 lines add whitespace errors.\n\nBut upon investigating the two \"test\" files in question,\ndqstring-line-splice.test and dqstring-no-interpolate.test, I recalled\nthat I had to play tricks to escape the single-quote context created\nby the Makefile when generating t/chainlinttmp/tests in order to allow\nchainlint.pl to see a double-quoted string. So, the abovementioned\nblank lines are indeed expected output from chainlint.pl given the\ntricks played.\n"},{"id":"485721","messageId":"xmqq5y0zmn44.fsf@gitster.g","threadId":"60604","inReplyTo":"CAPig+cSNZ75WSjxfJFoVPBTkv7yQpxGz1c4ugCyQRLytpNLjig@mail.gmail.com","subject":"Re: [PATCH v4] tests: adjust whitespace in chainlint expectations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-15T16:44:43Z","receivedAt":"2023-12-15T16:48:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> Aside: I was rather surprised to see this output from git-am when applying v4:\n>\n> Applying: tests: adjust whitespace in chainlint expectations\n> .git/rebase-apply/patch:205: new blank line at EOF.\n> +\n> .git/rebase-apply/patch:219: new blank line at EOF.\n> +\n> warning: 2 lines add whitespace errors.\n>\n> But upon investigating the two \"test\" files in question,\n> dqstring-line-splice.test and dqstring-no-interpolate.test, I recalled\n> that I had to play tricks to escape the single-quote context created\n> by the Makefile when generating t/chainlinttmp/tests in order to allow\n> chainlint.pl to see a double-quoted string. So, the abovementioned\n> blank lines are indeed expected output from chainlint.pl given the\n> tricks played.\n\nThanks for being extra careful while reviewing.\n\n"}]}