| Message ID | patch-3.3-f787b19f8c2-20220621T221928Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tests: fix tests broken if a " " is in the checkout dir's path | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > test_expect_success 'override global and system config' ' > - test_when_finished rm -f "$HOME"/.config/git && > + test_when_finished "rm -rf \"$HOME\"/.config/git" && As this string is evaled, isn't it safer to defer dereferencing the environment variable at the runtime by quoting the dollar-sign, too? I.e. test_when_finished rm -rf \"\$HOME\"/.config/git && or something?
Junio C Hamano <gitster@pobox.com> writes: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> test_expect_success 'override global and system config' ' >> - test_when_finished rm -f "$HOME"/.config/git && >> + test_when_finished "rm -rf \"$HOME\"/.config/git" && > > As this string is evaled, isn't it safer to defer dereferencing the > environment variable at the runtime by quoting the dollar-sign, too? > > I.e. > > test_when_finished rm -rf \"\$HOME\"/.config/git && > > or something? It probably would matter if $HOME had a double-quote in it, I think
diff --git a/t/t1300-config.sh b/t/t1300-config.sh index d3d9adbb3db..da4d03813f1 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -2083,7 +2083,7 @@ test_expect_success '--show-scope with --show-origin' ' ' test_expect_success 'override global and system config' ' - test_when_finished rm -f "$HOME"/.config/git && + test_when_finished "rm -rf \"$HOME\"/.config/git" && cat >"$HOME"/.gitconfig <<-EOF && [home]
The "test_when_finished" cleanup phase added in 4179b4897f2 (config: allow overriding of global and system configuration, 2021-04-19) has never worked as intended, firstly the ".config/git" is a directory, so we'd need the "-r" flag, but more importantly the $HOME variable wasn't properly quoted. We'd thus end up trying to remove the "trash" part of "trash directory", which wouldn't fail with "-f", since "rm -f" won't fail on non-existing files. It's possible that this would have caused an actual failure if someone had a $HOME with a space character in it, such that our "rm -f" would fail to remove an existing directory, but in practice that probably never happened. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t1300-config.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)