[PATCH] annotate-tests: quote variable expansions containing path names

[PATCH] annotate-tests: quote variable expansions containing path names

[Johannes Sixt]

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 <j6t@kdbg.org>
---
 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

Re: [PATCH] annotate-tests: quote variable expansions containing path names

[Philippe Blain]

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 <j6t@kdbg.org>
> ---
>   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 <levraiphilippeblain@gmail.com>

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.

Re: [PATCH] annotate-tests: quote variable expansions containing path names

[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.

-Peff

Re: [PATCH] annotate-tests: quote variable expansions containing path names

[Johannes Sixt]

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