patch, 3 partstests: fix tests broken if a " " is in the checkout dir's path

The 1/3 here fixes regressions in v2.37.0-rc0, and with 2-3/3 the test suite passes with the "git" directory named e.g. "git ~ @ checkout" when running:

	make NO_PERL=Y test

The "NO_PERL=Y" being because several git-svn tests still fail, for reasons I haven't looked into.

Ævar Arnfjörð Bjarmason (3):
  tests: add missing double quotes to included library paths
  test-lib.sh: fix prepend_var() quoting issue
  config tests: fix harmless but broken "rm -r" cleanup

 t/t1300-config.sh         | 2 +-
 t/t3700-add.sh            | 2 +-
 t/t3903-stash.sh          | 2 +-
 t/t7609-mergetool--lib.sh | 2 +-
 t/test-lib.sh             | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

Fix two inclusions added in d42bab442d7 (core.fsyncmethod: tests for batch mode, 2022-04-04) that needed to be quoted, and the same sort of issue in a242c150ebb (vimdiff: integrate layout tests in the unit tests framework ('t' folder), 2022-03-30). Both were first released with v2.37.0-rc0.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3700-add.sh            | 2 +-
 t/t3903-stash.sh          | 2 +-
 t/t7609-mergetool--lib.sh | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Describe what symptom you may see if it is left unfixed. IOW, if we consider that having a whitespace in the path to the build directory is just as crazy as having a single-quote, it would make this patch unnecessary, Make it easier for readers to decide, by saying something like

	As TEST_DIRECTORY is (typically) path to t/ in the extracted
	source tree, it can have funny characters like $IFS whitespace
	in it.

at least.

Thanks.

Fix a quoting issue in the function introduced in b9638d7286f (test-lib: make $GIT_BUILD_DIR an absolute path, 2022-02-27), running the test suite where the git checkout was on a path with e.g. a space in it would fail.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

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:

It probably would matter if $HOME had a double-quote in it, I think

A re-roll of https://lore.kernel.org/git/cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com/; Changes since v1:

 - Clarified IFS splitting issues in 1/3, as requested by Junio
 - Fixed $HOME quoting issue in 3/3, and also added the missing cleanup
   for $HOME/.gitconfig to that test.

Ævar Arnfjörð Bjarmason (3):
  tests: add missing double quotes to included library paths
  test-lib.sh: fix prepend_var() quoting issue
  config tests: fix harmless but broken "rm -r" cleanup

 t/t1300-config.sh         | 5 +++--
 t/t3700-add.sh            | 2 +-
 t/t3903-stash.sh          | 2 +-
 t/t7609-mergetool--lib.sh | 2 +-
 t/test-lib.sh             | 4 ++--
 5 files changed, 8 insertions(+), 7 deletions(-)

Range-diff against v1:
1:  05ba5e7f837 ! 1:  f4ef137d076 tests: add missing double quotes to included library paths
    @@ Metadata
      ## Commit message ##
         tests: add missing double quotes to included library paths
     
    -    Fix two inclusions added in d42bab442d7 (core.fsyncmethod: tests for
    -    batch mode, 2022-04-04) that needed to be quoted, and the same sort of
    -    issue in a242c150ebb (vimdiff: integrate layout tests in the unit
    -    tests framework ('t' folder), 2022-03-30). Both were first released
    -    with v2.37.0-rc0.
    +    Fix inclusion errors which would occur if the $TEST_DIRECTORY had $IFS
    +    whitespace in it.
    +
    +    See d42bab442d7 (core.fsyncmethod: tests for batch mode, 2022-04-04)
    +    and a242c150ebb (vimdiff: integrate layout tests in the unit tests
    +    framework ('t' folder), 2022-03-30) for the two relevant commits. Both
    +    were first released with v2.37.0-rc0 (and were also part of v2.37.0).
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
2:  e06bf0cdfbe = 2:  b56ededf1b8 test-lib.sh: fix prepend_var() quoting issue
3:  f787b19f8c2 ! 3:  d3f65326701 config tests: fix harmless but broken "rm -r" cleanup
    @@ Commit message
         fail to remove an existing directory, but in practice that probably
         never happened.
     
    +    Let's fix both the quoting issue, and the other issue cleanup issue in
    +    4179b4897f2, which is that we were attempting to clean up
    +    ~/.config/git, but weren't cleaing up ~/.gitconfig.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t1300-config.sh ##
    @@ t/t1300-config.sh: 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" &&
    - 
    +-
    ++	test_when_finished rm -f \"\$HOME\"/.gitconfig &&
      	cat >"$HOME"/.gitconfig <<-EOF &&
      	[home]
    + 		config = true
    + 	EOF
    ++
    ++	test_when_finished rm -rf \"\$HOME\"/.config/git &&
    + 	mkdir -p "$HOME"/.config/git &&
    + 	cat >"$HOME"/.config/git/config <<-EOF &&
    + 	[xdg]

Fix inclusion errors which would occur if the $TEST_DIRECTORY had $IFS whitespace in it.

See d42bab442d7 (core.fsyncmethod: tests for batch mode, 2022-04-04) and a242c150ebb (vimdiff: integrate layout tests in the unit tests framework ('t' folder), 2022-03-30) for the two relevant commits. Both were first released with v2.37.0-rc0 (and were also part of v2.37.0).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3700-add.sh            | 2 +-
 t/t3903-stash.sh          | 2 +-
 t/t7609-mergetool--lib.sh | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Fix a quoting issue in the function introduced in b9638d7286f (test-lib: make $GIT_BUILD_DIR an absolute path, 2022-02-27), running the test suite where the git checkout was on a path with e.g. a space in it would fail.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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.

Let's fix both the quoting issue, and the other issue cleanup issue in 4179b4897f2, which is that we were attempting to clean up ~/.config/git, but weren't cleaing up ~/.gitconfig.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t1300-config.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

This does look more correct than the original.

Nicely done.  Will queue.

Thanks.