# [PATCH] annotate-tests: quote variable expansions containing path names 4 messages from 2021-01-30 to 2021-02-03. Participants: Johannes Sixt, Philippe Blain, Jeff King. Thread: https://gitlist.dev/t/55077 ## Johannes Sixt, 2021-01-30 16:22 Subject: [PATCH] annotate-tests: quote variable expansions containing path names Message-ID: URL: https://gitlist.dev/e/cec4f442-03ea-e846-2613-f390b1e4d31b%40kdbg.org ``` The test case added by 9466e3809d ("blame: enable funcname blaming with userdiff driver", 2020-11-01) forgot to quote variable expansions. This causes failures when the current directory contains blanks. On variable that the test case introduces will not have IFS characters and could remain without quotes, but let's quote all expansions for consistency, not just the one that has the path name. Signed-off-by: Johannes Sixt --- t/annotate-tests.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index ee5d2d4cf8..29ce89090d 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -483,12 +483,12 @@ test_expect_success 'setup -L :funcname with userdiff driver' ' echo "fortran-* diff=fortran" >.gitattributes && fortran_file=fortran-external-function && orig_file="$TEST_DIRECTORY/t4018/$fortran_file" && - cp $orig_file . && - git add $fortran_file && + cp "$orig_file" . && + git add "$fortran_file" && GIT_AUTHOR_NAME="A" GIT_AUTHOR_EMAIL="A@test.git" \ git commit -m "add fortran file" && - sed -e "s/ChangeMe/IWasChanged/" <"$orig_file" >$fortran_file && - git add $fortran_file && + sed -e "s/ChangeMe/IWasChanged/" <"$orig_file" >"$fortran_file" && + git add "$fortran_file" && GIT_AUTHOR_NAME="B" GIT_AUTHOR_EMAIL="B@test.git" \ git commit -m "change fortran file" ' -- 2.30.0.119.g680bcb97f5 ``` ## Philippe Blain, 2021-01-30 17:14 Subject: Re: [PATCH] annotate-tests: quote variable expansions containing path names Message-ID: <762b6a24-15ef-7578-6064-eb82832699a4@gmail.com> URL: https://gitlist.dev/e/762b6a24-15ef-7578-6064-eb82832699a4%40gmail.com In-Reply-To: ``` Hi Johannes, Le 2021-01-30 à 11:22, Johannes Sixt a écrit : > The test case added by 9466e3809d ("blame: enable funcname blaming with > userdiff driver", 2020-11-01) forgot to quote variable expansions. This > causes failures when the current directory contains blanks. Woops, sorry for that. > > On variable s/On/One/ > that the test case introduces will not have IFS characters > and could remain without quotes, but let's quote all expansions for > consistency, not just the one that has the path name. > > Signed-off-by: Johannes Sixt > --- > t/annotate-tests.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh > index ee5d2d4cf8..29ce89090d 100644 > --- a/t/annotate-tests.sh > +++ b/t/annotate-tests.sh > @@ -483,12 +483,12 @@ test_expect_success 'setup -L :funcname with userdiff driver' ' > echo "fortran-* diff=fortran" >.gitattributes && > fortran_file=fortran-external-function && > orig_file="$TEST_DIRECTORY/t4018/$fortran_file" && > - cp $orig_file . && > - git add $fortran_file && > + cp "$orig_file" . && > + git add "$fortran_file" && > GIT_AUTHOR_NAME="A" GIT_AUTHOR_EMAIL="A@test.git" \ > git commit -m "add fortran file" && > - sed -e "s/ChangeMe/IWasChanged/" <"$orig_file" >$fortran_file && > - git add $fortran_file && > + sed -e "s/ChangeMe/IWasChanged/" <"$orig_file" >"$fortran_file" && > + git add "$fortran_file" && > GIT_AUTHOR_NAME="B" GIT_AUTHOR_EMAIL="B@test.git" \ > git commit -m "change fortran file" > ' > The patch obviously looks good: Acked-by: Philippe Blain Apart from that, maybe we could set up one of the CI jobs to clone git.git into a path with spaces, to try to avoid these kind of errors in the future ? Thanks, Philippe. ``` ## Jeff King, 2021-02-03 05:22 Subject: Re: [PATCH] annotate-tests: quote variable expansions containing path names Message-ID: URL: https://gitlist.dev/e/YBoy%2FF3tS3JtyhDX%40coredump.intra.peff.net In-Reply-To: <762b6a24-15ef-7578-6064-eb82832699a4@gmail.com> ``` On Sat, Jan 30, 2021 at 12:14:56PM -0500, Philippe Blain wrote: > Apart from that, maybe we could set up one of the CI jobs to > clone git.git into a path with spaces, to try to avoid these > kind of errors in the future ? We put a space in the temporary trash directory created by the test suite for exactly this purpose. So I was curious why it didn't detect this problem. It looks like it's because the issue is with $TEST_DIRECTORY, which is outside the trash directory. -Peff ``` ## Johannes Sixt, 2021-02-03 17:23 Subject: Re: [PATCH] annotate-tests: quote variable expansions containing path names Message-ID: URL: https://gitlist.dev/e/f667e887-0ba0-af0a-91a9-d84d833422b0%40kdbg.org In-Reply-To: ``` Am 03.02.21 um 06:22 schrieb Jeff King: > On Sat, Jan 30, 2021 at 12:14:56PM -0500, Philippe Blain wrote: > >> Apart from that, maybe we could set up one of the CI jobs to >> clone git.git into a path with spaces, to try to avoid these >> kind of errors in the future ? > > We put a space in the temporary trash directory created by the test > suite for exactly this purpose. So I was curious why it didn't detect > this problem. > > It looks like it's because the issue is with $TEST_DIRECTORY, which is > outside the trash directory. Yes, correct. I was also suprised at first that this was not caught. -- Hannes ```