| Message ID | cec4f442-03ea-e846-2613-f390b1e4d31b@kdbg.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 107d10fe4f05c6d13b25a2196689a451175a38df |
| Headers | show |
| Series | annotate-tests: quote variable expansions containing path names | expand |
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.
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
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
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 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(-)