{"thread":{"id":"47583","subject":"[PATCH] t3900: add some more quotes","startedAt":"2018-01-10T10:07:27Z","lastAt":"2018-01-11T12:11:51Z","messageCount":14,"participants":["Beat Bolli","Jeff King","Johannes Schindelin","Eric Sunshine","Johannes Sixt","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"336369","messageId":"20180110095832.22077-1-dev+git@drbeat.li","threadId":"47583","inReplyTo":null,"subject":"[PATCH] t3900: add some more quotes","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-01-10T09:58:32Z","receivedAt":"2018-01-10T10:07:27Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\nwere added to protect against spaces in $HOME. In the test_when_finished\nhander, two files are deleted which must be quoted individually.\n\nSigned-off-by: Beat Bolli \n---\n t/t3900-i18n-commit.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex 9e4e694d9..dc00db87b 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because of NULs' '\n '\n \n test_expect_success 'UTF-8 invalid characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \techo \"UTF-8 characters\" >F &&\n \tprintf \"Commit message\\n\\nInvalid surrogate:\\355\\240\\200\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n@@ -49,7 +49,7 @@ test_expect_success 'UTF-8 invalid characters refused' '\n '\n \n test_expect_success 'UTF-8 overlong sequences rejected' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \trm -f \"$HOME/stderr\" \"$HOME/invalid\" &&\n \techo \"UTF-8 overlong\" >F &&\n \tprintf \"\\340\\202\\251ommit message\\n\\nThis is not a space:\\300\\240\\n\" \\\n@@ -59,7 +59,7 @@ test_expect_success 'UTF-8 overlong sequences rejected' '\n '\n \n test_expect_success 'UTF-8 non-characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \techo \"UTF-8 non-character 1\" >F &&\n \tprintf \"Commit message\\n\\nNon-character:\\364\\217\\277\\276\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n@@ -68,7 +68,7 @@ test_expect_success 'UTF-8 non-characters refused' '\n '\n \n test_expect_success 'UTF-8 non-characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \techo \"UTF-8 non-character 2.\" >F &&\n \tprintf \"Commit message\\n\\nNon-character:\\357\\267\\220\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"336370","messageId":"20180110103357.GA18553@sigill.intra.peff.net","threadId":"47583","inReplyTo":"20180110095832.22077-1-dev+git@drbeat.li","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-10T10:33:57Z","receivedAt":"2018-01-10T10:34:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 10, 2018 at 10:58:32AM +0100, Beat Bolli wrote:\n\n> In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\n> were added to protect against spaces in $HOME. In the test_when_finished\n> hander, two files are deleted which must be quoted individually.\n\nDoh. I can't believe I missed that when reading the patch.\n\nThanks.\n\n-Peff\n"},{"id":"336395","messageId":"nycvar.QRO.7.76.6.1801101821010.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47583","inReplyTo":"20180110103357.GA18553@sigill.intra.peff.net","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-01-10T17:21:22Z","receivedAt":"2018-01-10T17:21:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 10 Jan 2018, Jeff King wrote:\n\n> On Wed, Jan 10, 2018 at 10:58:32AM +0100, Beat Bolli wrote:\n> \n> > In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\n> > were added to protect against spaces in $HOME. In the test_when_finished\n> > hander, two files are deleted which must be quoted individually.\n> \n> Doh. I can't believe I missed that when reading the patch.\n\nD'oh, and I can't believe that I missed this when writing the patch.\n\nThanks, Beat!\nDscho\n"},{"id":"336398","messageId":"CAPig+cTmgp3=jkG2Kb3x+3n4Dsq1V6GMGkfVE3=p6KGMBnNMkQ@mail.gmail.com","threadId":"47583","inReplyTo":"20180110095832.22077-1-dev+git@drbeat.li","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-10T17:44:38Z","receivedAt":"2018-01-10T17:44:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 10, 2018 at 4:58 AM, Beat Bolli wrote:\n> In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\n> were added to protect against spaces in $HOME. In the test_when_finished\n> hander, two files are deleted which must be quoted individually.\n\ns/hander/handler/\n\n> Signed-off-by: Beat Bolli \n"},{"id":"336406","messageId":"6b32292b-0f71-7cf8-2069-41766d440b58@kdbg.org","threadId":"47583","inReplyTo":"20180110095832.22077-1-dev+git@drbeat.li","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-01-10T19:02:09Z","receivedAt":"2018-01-10T19:02:16Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.01.2018 um 10:58 schrieb Beat Bolli:\n> In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\n> were added to protect against spaces in $HOME. In the test_when_finished\n> hander, two files are deleted which must be quoted individually.\n> \n> Signed-off-by: Beat Bolli \n> ---\n> t/t3900-i18n-commit.sh | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> index 9e4e694d9..dc00db87b 100755\n> --- a/t/t3900-i18n-commit.sh\n> +++ b/t/t3900-i18n-commit.sh\n> @@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because of NULs' '\n> '\n> \n> test_expect_success 'UTF-8 invalid characters refused' '\n> -\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n> +\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n\nShould that not better be\n\n\ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\"\n\ni.e., delay the expansion of $HOME to protect against double-quotes in \nthe path?\n\n-- Hannes\n"},{"id":"336412","messageId":"xmqqwp0p1ouf.fsf@gitster.mtv.corp.google.com","threadId":"47583","inReplyTo":"6b32292b-0f71-7cf8-2069-41766d440b58@kdbg.org","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T19:43:52Z","receivedAt":"2018-01-10T19:44:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> test_expect_success 'UTF-8 invalid characters refused' '\n>> -\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n>> +\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n>\n> Should that not better be\n>\n> \ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\"\n>\n> i.e., delay the expansion of $HOME to protect against double-quotes in\n> the path?\n\nYes ;-)\n"},{"id":"336414","messageId":"20180110195323.GA26186@sigill.intra.peff.net","threadId":"47583","inReplyTo":"6b32292b-0f71-7cf8-2069-41766d440b58@kdbg.org","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-10T19:53:23Z","receivedAt":"2018-01-10T19:53:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 10, 2018 at 08:02:09PM +0100, Johannes Sixt wrote:\n\n> > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> > index 9e4e694d9..dc00db87b 100755\n> > --- a/t/t3900-i18n-commit.sh\n> > +++ b/t/t3900-i18n-commit.sh\n> > @@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because of NULs' '\n> > '\n> > test_expect_success 'UTF-8 invalid characters refused' '\n> > -\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n> > +\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n> \n> Should that not better be\n> \n> \ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\"\n> \n> i.e., delay the expansion of $HOME to protect against double-quotes in the\n> path?\n\nYeah. One of the reasons for both of the errors in this thread is the\nnested double-quoting. Using single quotes is awkward because we're\nalready using them to delimit the whole snippet. I've often wondered if\nour tests would be more readable taking the snippet over stdin.\nSomething like:\n\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex 9e4e694d93..09ad4d8878 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused because of NULs' '\n \ttest_must_fail git commit -a -F \"$TEST_DIRECTORY\"/t3900/UTF-16.txt\n '\n \n-test_expect_success 'UTF-8 invalid characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+test_expect_success 'UTF-8 invalid characters refused' - <<\\EOT\n+\ttest_when_finished 'rm -f \"$HOME/stderr $HOME/invalid\"' &&\n \techo \"UTF-8 characters\" >F &&\n \tprintf \"Commit message\\n\\nInvalid surrogate:\\355\\240\\200\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n \tgit commit -a -F \"$HOME/invalid\" 2>\"$HOME\"/stderr &&\n \ttest_i18ngrep \"did not conform\" \"$HOME\"/stderr\n-'\n+EOT\n \n test_expect_success 'UTF-8 overlong sequences rejected' '\n \ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 1701fe2a06..be8a47d304 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -391,11 +391,32 @@ test_verify_prereq () {\n \terror \"bug in the test script: '$test_prereq' does not look like a prereq\"\n }\n \n+# Read from stdin into the variable given in $1.\n+test_read_to_eof () {\n+\t# Bash's \"read\" is sufficiently flexible that we can skip the extra\n+\t# process.\n+\tif test -n \"$BASH_VERSION\"\n+\tthen\n+\t\t# 64k should be enough for anyone...\n+\t\tread -N 65536 -r \"$1\"\n+\telse\n+\t\t# command substitution eats trailing whitespace, so we add\n+\t\t# and then remove a non-whitespace character.\n+\t\teval \"$1=\\$(cat; printf x)\"\n+\t\teval \"$1=\\${$1%x}\"\n+\tfi\n+}\n+\n test_expect_failure () {\n \ttest_start_\n \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-failure\"\n+\tif test \"$2\" = \"-\"\n+\tthen\n+\t\ttest_read_to_eof test_block\n+\t\tset -- \"$1\" \"$test_block\"\n+\tfi\n \ttest_verify_prereq\n \texport test_prereq\n \tif ! test_skip \"$@\"\n@@ -416,6 +437,11 @@ test_expect_success () {\n \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-success\"\n+\tif test \"$2\" = \"-\"\n+\tthen\n+\t\ttest_read_to_eof test_block\n+\t\tset -- \"$1\" \"$test_block\"\n+\tfi\n \ttest_verify_prereq\n \texport test_prereq\n \tif ! test_skip \"$@\"\n"},{"id":"336427","messageId":"xmqqy3l5z9hx.fsf@gitster.mtv.corp.google.com","threadId":"47583","inReplyTo":"20180110195323.GA26186@sigill.intra.peff.net","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T21:31:22Z","receivedAt":"2018-01-10T21:31:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah. One of the reasons for both of the errors in this thread is the\n> nested double-quoting. Using single quotes is awkward because we're\n> already using them to delimit the whole snippet. I've often wondered if\n> our tests would be more readable taking the snippet over stdin.\n> Something like:\n> +test_expect_success 'UTF-8 invalid characters refused' - <<\\EOT\n> +\ttest_when_finished 'rm -f \"$HOME/stderr $HOME/invalid\"' &&\n> ...\n> -'\n> +EOT\n> \n> +# Read from stdin into the variable given in $1.\n> +test_read_to_eof () {\n> +\t# Bash's \"read\" is sufficiently flexible that we can skip the extra\n> +\t# process.\n> +\tif test -n \"$BASH_VERSION\"\n> +\tthen\n> +\t\t# 64k should be enough for anyone...\n> +\t\tread -N 65536 -r \"$1\"\n> +\telse\n> +\t\t# command substitution eats trailing whitespace, so we add\n> +\t\t# and then remove a non-whitespace character.\n> +\t\teval \"$1=\\$(cat; printf x)\"\n> +\t\teval \"$1=\\${$1%x}\"\n> +\tfi\n> +}\n\nYuck. Hacky but nice.\n\nI think this will make it easier to read but I suspect here text\nwould use temporary files and may slow things down a bit? I do not\nknow if it is even measurable, though.\n\n> +\n> test_expect_failure () {\n> \ttest_start_\n> \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n> \ttest \"$#\" = 2 ||\n> \terror \"bug in the test script: not 2 or 3 parameters to test-expect-failure\"\n> +\tif test \"$2\" = \"-\"\n> +\tthen\n> +\t\ttest_read_to_eof test_block\n> +\t\tset -- \"$1\" \"$test_block\"\n> +\tfi\n> \ttest_verify_prereq\n> \texport test_prereq\n> \tif ! test_skip \"$@\"\n> @@ -416,6 +437,11 @@ test_expect_success () {\n> \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n> \ttest \"$#\" = 2 ||\n> \terror \"bug in the test script: not 2 or 3 parameters to test-expect-success\"\n> +\tif test \"$2\" = \"-\"\n> +\tthen\n> +\t\ttest_read_to_eof test_block\n> +\t\tset -- \"$1\" \"$test_block\"\n> +\tfi\n> \ttest_verify_prereq\n> \texport test_prereq\n> \tif ! test_skip \"$@\"\n"},{"id":"336432","messageId":"20180110223840.1481-1-dev+git@drbeat.li","threadId":"47583","inReplyTo":"CAPig+cTmgp3=jkG2Kb3x+3n4Dsq1V6GMGkfVE3=p6KGMBnNMkQ@mail.gmail.com","subject":"[PATCH v2] t3900: add some more quotes","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-01-10T22:38:40Z","receivedAt":"2018-01-10T22:39:10Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\nwere added to protect against spaces in $HOME. In the test_when_finished\nhandler, two files are deleted which must be quoted individually.\n\nSigned-off-by: Beat Bolli \n---\n\nDiff to v1:\n\ns/hander/handler/ in the message.\n\n t/t3900-i18n-commit.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex 9e4e694d9..dc00db87b 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because of NULs' '\n '\n \n test_expect_success 'UTF-8 invalid characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \techo \"UTF-8 characters\" >F &&\n \tprintf \"Commit message\\n\\nInvalid surrogate:\\355\\240\\200\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n@@ -49,7 +49,7 @@ test_expect_success 'UTF-8 invalid characters refused' '\n '\n \n test_expect_success 'UTF-8 overlong sequences rejected' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \trm -f \"$HOME/stderr\" \"$HOME/invalid\" &&\n \techo \"UTF-8 overlong\" >F &&\n \tprintf \"\\340\\202\\251ommit message\\n\\nThis is not a space:\\300\\240\\n\" \\\n@@ -59,7 +59,7 @@ test_expect_success 'UTF-8 overlong sequences rejected' '\n '\n \n test_expect_success 'UTF-8 non-characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \techo \"UTF-8 non-character 1\" >F &&\n \tprintf \"Commit message\\n\\nNon-character:\\364\\217\\277\\276\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n@@ -68,7 +68,7 @@ test_expect_success 'UTF-8 non-characters refused' '\n '\n \n test_expect_success 'UTF-8 non-characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n \techo \"UTF-8 non-character 2.\" >F &&\n \tprintf \"Commit message\\n\\nNon-character:\\357\\267\\220\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"336434","messageId":"nycvar.QRO.7.76.6.1801110008030.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47583","inReplyTo":"20180110223840.1481-1-dev+git@drbeat.li","subject":"Re: [PATCH v2] t3900: add some more quotes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-01-10T23:09:19Z","receivedAt":"2018-01-10T23:09:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Beat,\n\nOn Wed, 10 Jan 2018, Beat Bolli wrote:\n\n> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> index 9e4e694d9..dc00db87b 100755\n> --- a/t/t3900-i18n-commit.sh\n> +++ b/t/t3900-i18n-commit.sh\n> @@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because of NULs' '\n> '\n> \n> test_expect_success 'UTF-8 invalid characters refused' '\n> -\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n> +\ttest_when_finished \"rm -f \\\"$HOME/stderr\\\" \\\"$HOME/invalid\\\"\" &&\n\nIt is probably worth quoting the dollar characters, too, as pointed out by\nHannes Sixt: we want to interpolate the value only when needed, just in\ncase that the HOME variable contains double quotes.\n\nCiao,\nDscho\n"},{"id":"336435","messageId":"xmqqfu7dz4xr.fsf@gitster.mtv.corp.google.com","threadId":"47583","inReplyTo":"20180110223840.1481-1-dev+git@drbeat.li","subject":"Re: [PATCH v2] t3900: add some more quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T23:09:52Z","receivedAt":"2018-01-10T23:10:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli writes:\n\n> In 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\n> were added to protect against spaces in $HOME. In the test_when_finished\n> handler, two files are deleted which must be quoted individually.\n>\n> Signed-off-by: Beat Bolli \n> ---\n>\n> Diff to v1:\n>\n> s/hander/handler/ in the message.\n> ...\n\nOK, but that forgets to fix a more important issue raised in the\ndiscussion, no?\n\nHere is what I ended up queuing in the meantime. Thanks.\n\n-- >8 --\nFrom: Beat Bolli \nDate: Wed, 10 Jan 2018 10:58:32 +0100\nSubject: [PATCH] t3900: add some more quotes\n\nIn 89a70b80 (\"t0302 & t3900: add forgotten quotes\", 2018-01-03), quotes\nwere added to protect against spaces in $HOME. In the test_when_finished\ncommand, two files are deleted which must be quoted individually.\n\n[jc: with \\$HOME in the test_when_finished command quoted, as\npointed out by j6t].\n\nSigned-off-by: Beat Bolli \nHelped-by: Johannes Sixt \nSigned-off-by: Junio C Hamano \n---\n t/t3900-i18n-commit.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex 9e4e694d93..b92ff95977 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because of NULs' '\n '\n \n test_expect_success 'UTF-8 invalid characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\" &&\n \techo \"UTF-8 characters\" >F &&\n \tprintf \"Commit message\\n\\nInvalid surrogate:\\355\\240\\200\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n@@ -49,7 +49,7 @@ test_expect_success 'UTF-8 invalid characters refused' '\n '\n \n test_expect_success 'UTF-8 overlong sequences rejected' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\" &&\n \trm -f \"$HOME/stderr\" \"$HOME/invalid\" &&\n \techo \"UTF-8 overlong\" >F &&\n \tprintf \"\\340\\202\\251ommit message\\n\\nThis is not a space:\\300\\240\\n\" \\\n@@ -59,7 +59,7 @@ test_expect_success 'UTF-8 overlong sequences rejected' '\n '\n \n test_expect_success 'UTF-8 non-characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\" &&\n \techo \"UTF-8 non-character 1\" >F &&\n \tprintf \"Commit message\\n\\nNon-character:\\364\\217\\277\\276\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n@@ -68,7 +68,7 @@ test_expect_success 'UTF-8 non-characters refused' '\n '\n \n test_expect_success 'UTF-8 non-characters refused' '\n-\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"\\$HOME/stderr\\\" \\\"\\$HOME/invalid\\\"\" &&\n \techo \"UTF-8 non-character 2.\" >F &&\n \tprintf \"Commit message\\n\\nNon-character:\\357\\267\\220\\n\" \\\n \t\t>\"$HOME/invalid\" &&\n-- \n2.16.0-rc1-187-g8dee184084\n\n"},{"id":"336447","messageId":"20180111093819.GB9190@sigill.intra.peff.net","threadId":"47583","inReplyTo":"xmqqy3l5z9hx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t3900: add some more quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-11T09:38:20Z","receivedAt":"2018-01-11T09:38:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 10, 2018 at 01:31:22PM -0800, Junio C Hamano wrote:\n\n> > +# Read from stdin into the variable given in $1.\n> > +test_read_to_eof () {\n> > +\t# Bash's \"read\" is sufficiently flexible that we can skip the extra\n> > +\t# process.\n> > +\tif test -n \"$BASH_VERSION\"\n> > +\tthen\n> > +\t\t# 64k should be enough for anyone...\n> > +\t\tread -N 65536 -r \"$1\"\n> > +\telse\n> > +\t\t# command substitution eats trailing whitespace, so we add\n> > +\t\t# and then remove a non-whitespace character.\n> > +\t\teval \"$1=\\$(cat; printf x)\"\n> > +\t\teval \"$1=\\${$1%x}\"\n> > +\tfi\n> > +}\n> \n> Yuck. Hacky but nice.\n> \n> I think this will make it easier to read but I suspect here text\n> would use temporary files and may slow things down a bit? I do not\n> know if it is even measurable, though.\n\nYeah, since I was able to contain the horrible-ness in this function, I\nthink the overall readability/portability is probably OK. My main\nconcern was that it would be slower (hence the bash hackery). I applied\nthe patch below on top to see what kind of impact we could measure\nacross the whole suite:\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex be8a47d304..6f2e6e7560 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -441,6 +441,15 @@ test_expect_success () {\n \tthen\n \t\ttest_read_to_eof test_block\n \t\tset -- \"$1\" \"$test_block\"\n+\telse\n+\t\t# this is obviously a dumb thing to do, but it's just\n+\t\t# a performance-testing hack.\n+\t\ttest_read_to_eof test_block < I've often wondered if\n> our tests would be more readable taking the snippet over stdin.\n> Something like:\n> \n> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> index 9e4e694d93..09ad4d8878 100755\n> --- a/t/t3900-i18n-commit.sh\n> +++ b/t/t3900-i18n-commit.sh\n> @@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused because of NULs' '\n> \ttest_must_fail git commit -a -F \"$TEST_DIRECTORY\"/t3900/UTF-16.txt\n> '\n> \n> -test_expect_success 'UTF-8 invalid characters refused' '\n\nNote that the test snippet started right after that last single quote,\ni.e. it started with a newline.\n\n> -\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n> +test_expect_success 'UTF-8 invalid characters refused' - <<\\EOT\n> +\ttest_when_finished 'rm -f \"$HOME/stderr $HOME/invalid\"' &&\n\nAnd now it starts at the beginning of this line, i.e. without that\nleading neline. This change leads to the following when run with '-v':\n\n expecting success: \ttest_when_finished 'rm -f \"$HOME/stderr $HOME/invalid\"' &&\n\techo \"UTF-8 characters\" >F &&\n\tprintf \"Commit message\\n\\nInvalid surrogate:\\355\\240\\200\\n\" \\\n\t\t>\"$HOME/invalid\" &&\n\tgit commit -a -F \"$HOME/invalid\" 2>\"$HOME\"/stderr &&\n\ttest_i18ngrep \"did not conform\" \"$HOME\"/stderr\n\nNotice how the \"expecting success\" and the first line of the test ended\nup in the same line. I find this more annoying than the lack of empty\nline between the colored and indented test code and the uncolored and\nunindented test output.\n\n> \techo \"UTF-8 characters\" >F &&\n> \tprintf \"Commit message\\n\\nInvalid surrogate:\\355\\240\\200\\n\" \\\n> \t\t>\"$HOME/invalid\" &&\n> \tgit commit -a -F \"$HOME/invalid\" 2>\"$HOME\"/stderr &&\n> \ttest_i18ngrep \"did not conform\" \"$HOME\"/stderr\n> -'\n> +EOT\n> \n> test_expect_success 'UTF-8 overlong sequences rejected' '\n> \ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 1701fe2a06..be8a47d304 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -391,11 +391,32 @@ test_verify_prereq () {\n> \terror \"bug in the test script: '$test_prereq' does not look like a prereq\"\n> }\n> \n> +# Read from stdin into the variable given in $1.\n> +test_read_to_eof () {\n> +\t# Bash's \"read\" is sufficiently flexible that we can skip the extra\n> +\t# process.\n> +\tif test -n \"$BASH_VERSION\"\n> +\tthen\n> +\t\t# 64k should be enough for anyone...\n> +\t\tread -N 65536 -r \"$1\"\n> +\telse\n> +\t\t# command substitution eats trailing whitespace, so we add\n> +\t\t# and then remove a non-whitespace character.\n> +\t\teval \"$1=\\$(cat; printf x)\"\n> +\t\teval \"$1=\\${$1%x}\"\n> +\tfi\n> +}\n\nCommand substitutions don't eat trailing whitespaces in general, only\ntrailing newlines. POSIX:\n\n The shell shall expand the command substitution by executing command\n in a subshell environment (see Shell Execution Environment) and\n replacing the command substitution (the text of command plus the\n enclosing \"$()\" or backquotes) with the standard output of the\n command, removing sequences of one or more s at the end of\n the substitution.\n\nBash and dash conform to this.\n\nHow about this alternative (also adding the missing leading newline\nmentioned above):\n\n+\t\teval \"$1='\n+'\\$(cat)'\n+'\"\n\nThe indentation is yuck, but overall perhaps a bit less hacky...\n\n"},{"id":"336457","messageId":"20180111121143.GA6357@sigill.intra.peff.net","threadId":"47583","inReplyTo":"20180111113928.6412-1-szeder.dev@gmail.com","subject":"Re: read test snippet from stdin [was: [PATCH] t3900: add some more quotes]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-11T12:11:44Z","receivedAt":"2018-01-11T12:11:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 11, 2018 at 12:39:28PM +0100, SZEDER Gábor wrote:\n\n> > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> > index 9e4e694d93..09ad4d8878 100755\n> > --- a/t/t3900-i18n-commit.sh\n> > +++ b/t/t3900-i18n-commit.sh\n> > @@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused because of NULs' '\n> > \ttest_must_fail git commit -a -F \"$TEST_DIRECTORY\"/t3900/UTF-16.txt\n> > '\n> > \n> > -test_expect_success 'UTF-8 invalid characters refused' '\n> \n> Note that the test snippet started right after that last single quote,\n> i.e. it started with a newline.\n> \n> > -\ttest_when_finished \"rm -f \\\"$HOME/stderr $HOME/invalid\\\"\" &&\n> > +test_expect_success 'UTF-8 invalid characters refused' - <<\\EOT\n> > +\ttest_when_finished 'rm -f \"$HOME/stderr $HOME/invalid\"' &&\n> \n> And now it starts at the beginning of this line, i.e. without that\n> leading neline. This change leads to the following when run with '-v':\n\nYeah, I noticed that, too. It would be easy enough to add the extra\nnewline ourselves when printing the verbose output (quite a few of our\nolder tests don't start with a newline already, so it would be improving\nthem, too).\n\nAlternatively, I considered adding a whole new helper function that\nwould skip the need to say \"-\" as the test body. And then it would\nalways know we were reading from the here-doc.\n\n> > +\t\t# command substitution eats trailing whitespace, so we add\n> > +\t\t# and then remove a non-whitespace character.\n> > +\t\teval \"$1=\\$(cat; printf x)\"\n> > +\t\teval \"$1=\\${$1%x}\"\n> > +\tfi\n> > +}\n> \n> Command substitutions don't eat trailing whitespaces in general, only\n> trailing newlines. POSIX:\n\nYeah, I wondered about that, but didn't bother digging since the\nsolution is the same either way.\n\n> How about this alternative (also adding the missing leading newline\n> mentioned above):\n> \n> +\t\teval \"$1='\n> +'\\$(cat)'\n> +'\"\n> \n> The indentation is yuck, but overall perhaps a bit less hacky...\n\nI wrote something very similar at first, before finding the printf trick\non Stack Overflow. I thought the indentation on what I wrote was too\nugly. :)\n\nI don't have a strong preference, and certainly if one is more portable\nthan the other, we should choose that.\n\nThe main point of my email was just to say \"do people even like the\nconcept/direction?\"\n\n-Peff\n"}]}