{"thread":{"id":"55077","subject":"[PATCH] annotate-tests: quote variable expansions containing path names","startedAt":"2021-01-30T16:24:08Z","lastAt":"2021-02-03T17:24:17Z","messageCount":4,"participants":["Johannes Sixt","Philippe Blain","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"415656","messageId":"cec4f442-03ea-e846-2613-f390b1e4d31b@kdbg.org","threadId":"55077","inReplyTo":null,"subject":"[PATCH] annotate-tests: quote variable expansions containing path names","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-01-30T16:22:25Z","receivedAt":"2021-01-30T16:24:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The test case added by 9466e3809d (\"blame: enable funcname blaming with\nuserdiff driver\", 2020-11-01) forgot to quote variable expansions. This\ncauses failures when the current directory contains blanks.\n\nOn variable that the test case introduces will not have IFS characters\nand could remain without quotes, but let's quote all expansions for\nconsistency, not just the one that has the path name.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n t/annotate-tests.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex ee5d2d4cf8..29ce89090d 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -483,12 +483,12 @@ test_expect_success 'setup -L :funcname with userdiff driver' '\n \techo \"fortran-* diff=fortran\" >.gitattributes &&\n \tfortran_file=fortran-external-function &&\n \torig_file=\"$TEST_DIRECTORY/t4018/$fortran_file\" &&\n-\tcp $orig_file . &&\n-\tgit add $fortran_file &&\n+\tcp \"$orig_file\" . &&\n+\tgit add \"$fortran_file\" &&\n \tGIT_AUTHOR_NAME=\"A\" GIT_AUTHOR_EMAIL=\"A@test.git\" \\\n \tgit commit -m \"add fortran file\" &&\n-\tsed -e \"s/ChangeMe/IWasChanged/\" <\"$orig_file\" >$fortran_file &&\n-\tgit add $fortran_file &&\n+\tsed -e \"s/ChangeMe/IWasChanged/\" <\"$orig_file\" >\"$fortran_file\" &&\n+\tgit add \"$fortran_file\" &&\n \tGIT_AUTHOR_NAME=\"B\" GIT_AUTHOR_EMAIL=\"B@test.git\" \\\n \tgit commit -m \"change fortran file\"\n '\n-- \n2.30.0.119.g680bcb97f5\n"},{"id":"415657","messageId":"762b6a24-15ef-7578-6064-eb82832699a4@gmail.com","threadId":"55077","inReplyTo":"cec4f442-03ea-e846-2613-f390b1e4d31b@kdbg.org","subject":"Re: [PATCH] annotate-tests: quote variable expansions containing path names","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-01-30T17:14:56Z","receivedAt":"2021-01-30T17:15:41Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Johannes,\n\nLe 2021-01-30 à 11:22, Johannes Sixt a écrit :\n> The test case added by 9466e3809d (\"blame: enable funcname blaming with\n> userdiff driver\", 2020-11-01) forgot to quote variable expansions. This\n> causes failures when the current directory contains blanks.\n\nWoops, sorry for that.\n\n> \n> On variable \n\ns/On/One/\n\n> that the test case introduces will not have IFS characters\n> and could remain without quotes, but let's quote all expansions for\n> consistency, not just the one that has the path name.\n> \n> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> ---\n>   t/annotate-tests.sh | 8 ++++----\n>   1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n> index ee5d2d4cf8..29ce89090d 100644\n> --- a/t/annotate-tests.sh\n> +++ b/t/annotate-tests.sh\n> @@ -483,12 +483,12 @@ test_expect_success 'setup -L :funcname with userdiff driver' '\n>   \techo \"fortran-* diff=fortran\" >.gitattributes &&\n>   \tfortran_file=fortran-external-function &&\n>   \torig_file=\"$TEST_DIRECTORY/t4018/$fortran_file\" &&\n> -\tcp $orig_file . &&\n> -\tgit add $fortran_file &&\n> +\tcp \"$orig_file\" . &&\n> +\tgit add \"$fortran_file\" &&\n>   \tGIT_AUTHOR_NAME=\"A\" GIT_AUTHOR_EMAIL=\"A@test.git\" \\\n>   \tgit commit -m \"add fortran file\" &&\n> -\tsed -e \"s/ChangeMe/IWasChanged/\" <\"$orig_file\" >$fortran_file &&\n> -\tgit add $fortran_file &&\n> +\tsed -e \"s/ChangeMe/IWasChanged/\" <\"$orig_file\" >\"$fortran_file\" &&\n> +\tgit add \"$fortran_file\" &&\n>   \tGIT_AUTHOR_NAME=\"B\" GIT_AUTHOR_EMAIL=\"B@test.git\" \\\n>   \tgit commit -m \"change fortran file\"\n>   '\n> \n\nThe patch obviously looks good:\n\nAcked-by: Philippe Blain <levraiphilippeblain@gmail.com>\n\nApart from that, maybe we could set up one of the CI jobs to\nclone git.git into a path with spaces, to try to avoid these\nkind of errors in the future ?\n\nThanks,\n\nPhilippe.\n"},{"id":"416028","messageId":"YBoy/F3tS3JtyhDX@coredump.intra.peff.net","threadId":"55077","inReplyTo":"762b6a24-15ef-7578-6064-eb82832699a4@gmail.com","subject":"Re: [PATCH] annotate-tests: quote variable expansions containing path names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-03T05:22:04Z","receivedAt":"2021-02-03T05:23:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 30, 2021 at 12:14:56PM -0500, Philippe Blain wrote:\n\n> Apart from that, maybe we could set up one of the CI jobs to\n> clone git.git into a path with spaces, to try to avoid these\n> kind of errors in the future ?\n\nWe put a space in the temporary trash directory created by the test\nsuite for exactly this purpose. So I was curious why it didn't detect\nthis problem.\n\nIt looks like it's because the issue is with $TEST_DIRECTORY, which is\noutside the trash directory.\n\n-Peff\n"},{"id":"416068","messageId":"f667e887-0ba0-af0a-91a9-d84d833422b0@kdbg.org","threadId":"55077","inReplyTo":"YBoy/F3tS3JtyhDX@coredump.intra.peff.net","subject":"Re: [PATCH] annotate-tests: quote variable expansions containing path names","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-02-03T17:23:03Z","receivedAt":"2021-02-03T17:24:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.02.21 um 06:22 schrieb Jeff King:\n> On Sat, Jan 30, 2021 at 12:14:56PM -0500, Philippe Blain wrote:\n> \n>> Apart from that, maybe we could set up one of the CI jobs to\n>> clone git.git into a path with spaces, to try to avoid these\n>> kind of errors in the future ?\n> \n> We put a space in the temporary trash directory created by the test\n> suite for exactly this purpose. So I was curious why it didn't detect\n> this problem.\n> \n> It looks like it's because the issue is with $TEST_DIRECTORY, which is\n> outside the trash directory.\n\nYes, correct. I was also suprised at first that this was not caught.\n\n-- Hannes\n"}]}