threads / patch / 55077

patchannotate-tests: quote variable expansions containing path names

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

## tl;dr

4 messages between Jan 30, 2021 and Feb 3, 2021. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Johannes Sixt· Jan 30, 2021, 16:22 UTC · lore

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(-)

Show changes to t/annotate-tests.sh +4 −4

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· Jan 30, 2021, 17:14 UTC · re: Johannes Sixt · lore

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

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/

Show 31 quoted lines

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

Jeff King· Feb 3, 2021, 05:22 UTC · re: Philippe Blain · lore

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

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· Feb 3, 2021, 17:23 UTC · re: Jeff King · lore

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

Am 03.02.21 um 06:22 schrieb Jeff King:

Show 12 quoted lines

> 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