{"thread":{"id":"58045","subject":"[PATCH 0/3] tests: fix tests broken if a \" \" is in the checkout dir's path","startedAt":"2022-06-21T22:21:47Z","lastAt":"2022-06-30T20:52:34Z","messageCount":12,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"457646","messageId":"cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com","threadId":"58045","inReplyTo":null,"subject":"[PATCH 0/3] tests: fix tests broken if a \" \" is in the checkout dir's path","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:21:37Z","receivedAt":"2022-06-21T22:21:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The 1/3 here fixes regressions in v2.37.0-rc0, and with 2-3/3 the test\nsuite passes with the \"git\" directory named e.g. \"git ~ @ checkout\"\nwhen running:\n\n\tmake NO_PERL=Y test\n\nThe \"NO_PERL=Y\" being because several git-svn tests still fail, for\nreasons I haven't looked into.\n\nÆvar Arnfjörð Bjarmason (3):\n  tests: add missing double quotes to included library paths\n  test-lib.sh: fix prepend_var() quoting issue\n  config tests: fix harmless but broken \"rm -r\" cleanup\n\n t/t1300-config.sh         | 2 +-\n t/t3700-add.sh            | 2 +-\n t/t3903-stash.sh          | 2 +-\n t/t7609-mergetool--lib.sh | 2 +-\n t/test-lib.sh             | 4 ++--\n 5 files changed, 6 insertions(+), 6 deletions(-)\n\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457647","messageId":"patch-1.3-05ba5e7f837-20220621T221928Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com","subject":"[PATCH 1/3] tests: add missing double quotes to included library paths","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:21:38Z","receivedAt":"2022-06-21T22:21:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix two inclusions added in d42bab442d7 (core.fsyncmethod: tests for\nbatch mode, 2022-04-04) that needed to be quoted, and the same sort of\nissue in a242c150ebb (vimdiff: integrate layout tests in the unit\ntests framework ('t' folder), 2022-03-30). Both were first released\nwith v2.37.0-rc0.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t3700-add.sh            | 2 +-\n t/t3903-stash.sh          | 2 +-\n t/t7609-mergetool--lib.sh | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 8979c8a5f03..8689b48589c 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -8,7 +8,7 @@ test_description='Test of git add, including the -- option.'\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-. $TEST_DIRECTORY/lib-unique-files.sh\n+. \"$TEST_DIRECTORY\"/lib-unique-files.sh\n \n # Test the file mode \"$1\" of the file \"$2\" in the index.\n test_mode_in_index () {\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 20e94881964..2a4c3fd61c0 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n-. $TEST_DIRECTORY/lib-unique-files.sh\n+. \"$TEST_DIRECTORY\"/lib-unique-files.sh\n \n test_expect_success 'usage on cmd and subcommand invalid option' '\n \ttest_expect_code 129 git stash --invalid-option 2>usage &&\ndiff --git a/t/t7609-mergetool--lib.sh b/t/t7609-mergetool--lib.sh\nindex d848fe6442b..330d6d603d7 100755\n--- a/t/t7609-mergetool--lib.sh\n+++ b/t/t7609-mergetool--lib.sh\n@@ -7,7 +7,7 @@ Testing basic merge tools options'\n . ./test-lib.sh\n \n test_expect_success 'mergetool --tool=vimdiff creates the expected layout' '\n-\t. $GIT_BUILD_DIR/mergetools/vimdiff &&\n+\t. \"$GIT_BUILD_DIR\"/mergetools/vimdiff &&\n \trun_unit_tests\n '\n \n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457648","messageId":"patch-2.3-e06bf0cdfbe-20220621T221928Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com","subject":"[PATCH 2/3] test-lib.sh: fix prepend_var() quoting issue","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:21:39Z","receivedAt":"2022-06-21T22:21:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a quoting issue in the function introduced in\nb9638d7286f (test-lib: make $GIT_BUILD_DIR an absolute path,\n2022-02-27), running the test suite where the git checkout was on a\npath with e.g. a space in it would fail.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 55857af601b..8cabb4d10f9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -57,14 +57,14 @@ fi\n #\n #\tprepend_var VAR : VALUE\n prepend_var () {\n-\teval \"$1=$3\\${$1:+${3:+$2}\\$$1}\"\n+\teval \"$1=\\\"$3\\${$1:+${3:+$2}\\$$1}\\\"\"\n }\n \n # If [AL]SAN is in effect we want to abort so that we notice\n # problems. The GIT_SAN_OPTIONS variable can be used to set common\n # defaults shared between [AL]SAN_OPTIONS.\n prepend_var GIT_SAN_OPTIONS : abort_on_error=1\n-prepend_var GIT_SAN_OPTIONS : strip_path_prefix=\\\"$GIT_BUILD_DIR/\\\"\n+prepend_var GIT_SAN_OPTIONS : strip_path_prefix=\"$GIT_BUILD_DIR/\"\n \n # If we were built with ASAN, it may complain about leaks\n # of program-lifetime variables. Disable it by default to lower\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457649","messageId":"patch-3.3-f787b19f8c2-20220621T221928Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com","subject":"[PATCH 3/3] config tests: fix harmless but broken \"rm -r\" cleanup","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:21:40Z","receivedAt":"2022-06-21T22:21:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"test_when_finished\" cleanup phase added in 4179b4897f2 (config:\nallow overriding of global and system configuration, 2021-04-19) has\nnever worked as intended, firstly the \".config/git\" is a directory, so\nwe'd need the \"-r\" flag, but more importantly the $HOME variable\nwasn't properly quoted.\n\nWe'd thus end up trying to remove the \"trash\" part of \"trash\ndirectory\", which wouldn't fail with \"-f\", since \"rm -f\" won't fail on\nnon-existing files.\n\nIt's possible that this would have caused an actual failure if someone\nhad a $HOME with a space character in it, such that our \"rm -f\" would\nfail to remove an existing directory, but in practice that probably\nnever happened.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t1300-config.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex d3d9adbb3db..da4d03813f1 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2083,7 +2083,7 @@ test_expect_success '--show-scope with --show-origin' '\n '\n \n test_expect_success 'override global and system config' '\n-\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\ttest_when_finished \"rm -rf \\\"$HOME\\\"/.config/git\" &&\n \n \tcat >\"$HOME\"/.gitconfig <<-EOF &&\n \t[home]\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457656","messageId":"xmqq4k0dfxxm.fsf@gitster.g","threadId":"58045","inReplyTo":"patch-3.3-f787b19f8c2-20220621T221928Z-avarab@gmail.com","subject":"Re: [PATCH 3/3] config tests: fix harmless but broken \"rm -r\" cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-21T22:34:29Z","receivedAt":"2022-06-21T22:34:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n>  test_expect_success 'override global and system config' '\n> -\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n> +\ttest_when_finished \"rm -rf \\\"$HOME\\\"/.config/git\" &&\n\nAs this string is evaled, isn't it safer to defer dereferencing the\nenvironment variable at the runtime by quoting the dollar-sign, too?\n\nI.e.\n\n\ttest_when_finished rm -rf \\\"\\$HOME\\\"/.config/git &&\n\nor something?\n"},{"id":"457663","messageId":"xmqqzgi5ej0d.fsf@gitster.g","threadId":"58045","inReplyTo":"patch-1.3-05ba5e7f837-20220621T221928Z-avarab@gmail.com","subject":"Re: [PATCH 1/3] tests: add missing double quotes to included library paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-21T22:42:10Z","receivedAt":"2022-06-21T22:42:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Fix two inclusions added in d42bab442d7 (core.fsyncmethod: tests for\n> batch mode, 2022-04-04) that needed to be quoted, and the same sort of\n> issue in a242c150ebb (vimdiff: integrate layout tests in the unit\n> tests framework ('t' folder), 2022-03-30). Both were first released\n> with v2.37.0-rc0.\n\nDescribe what symptom you may see if it is left unfixed.  IOW, if we\nconsider that having a whitespace in the path to the build directory\nis just as crazy as having a single-quote, it would make this patch\nunnecessary,  Make it easier for readers to decide, by saying\nsomething like\n\n\tAs TEST_DIRECTORY is (typically) path to t/ in the extracted\n\tsource tree, it can have funny characters like $IFS whitespace\n\tin it.\n\nat least.\n\nThanks.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  t/t3700-add.sh            | 2 +-\n>  t/t3903-stash.sh          | 2 +-\n>  t/t7609-mergetool--lib.sh | 2 +-\n>  3 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> index 8979c8a5f03..8689b48589c 100755\n> --- a/t/t3700-add.sh\n> +++ b/t/t3700-add.sh\n> @@ -8,7 +8,7 @@ test_description='Test of git add, including the -- option.'\n>  TEST_PASSES_SANITIZE_LEAK=true\n>  . ./test-lib.sh\n>  \n> -. $TEST_DIRECTORY/lib-unique-files.sh\n> +. \"$TEST_DIRECTORY\"/lib-unique-files.sh\n>  \n>  # Test the file mode \"$1\" of the file \"$2\" in the index.\n>  test_mode_in_index () {\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index 20e94881964..2a4c3fd61c0 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n>  \n>  . ./test-lib.sh\n> -. $TEST_DIRECTORY/lib-unique-files.sh\n> +. \"$TEST_DIRECTORY\"/lib-unique-files.sh\n>  \n>  test_expect_success 'usage on cmd and subcommand invalid option' '\n>  \ttest_expect_code 129 git stash --invalid-option 2>usage &&\n> diff --git a/t/t7609-mergetool--lib.sh b/t/t7609-mergetool--lib.sh\n> index d848fe6442b..330d6d603d7 100755\n> --- a/t/t7609-mergetool--lib.sh\n> +++ b/t/t7609-mergetool--lib.sh\n> @@ -7,7 +7,7 @@ Testing basic merge tools options'\n>  . ./test-lib.sh\n>  \n>  test_expect_success 'mergetool --tool=vimdiff creates the expected layout' '\n> -\t. $GIT_BUILD_DIR/mergetools/vimdiff &&\n> +\t. \"$GIT_BUILD_DIR\"/mergetools/vimdiff &&\n>  \trun_unit_tests\n>  '\n"},{"id":"457680","messageId":"xmqq8rpp8div.fsf@gitster.g","threadId":"58045","inReplyTo":"xmqq4k0dfxxm.fsf@gitster.g","subject":"Re: [PATCH 3/3] config tests: fix harmless but broken \"rm -r\" cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-22T05:37:12Z","receivedAt":"2022-06-22T05:37:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>>  test_expect_success 'override global and system config' '\n>> -\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n>> +\ttest_when_finished \"rm -rf \\\"$HOME\\\"/.config/git\" &&\n>\n> As this string is evaled, isn't it safer to defer dereferencing the\n> environment variable at the runtime by quoting the dollar-sign, too?\n>\n> I.e.\n>\n> \ttest_when_finished rm -rf \\\"\\$HOME\\\"/.config/git &&\n>\n> or something?\n\nIt probably would matter if $HOME had a double-quote in it, I think\n"},{"id":"458174","messageId":"patch-v2-1.3-f4ef137d076-20220630T101646Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-v2-0.3-00000000000-20220630T101646Z-avarab@gmail.com","subject":"[PATCH v2 1/3] tests: add missing double quotes to included library paths","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T10:18:34Z","receivedAt":"2022-06-30T10:19:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix inclusion errors which would occur if the $TEST_DIRECTORY had $IFS\nwhitespace in it.\n\nSee d42bab442d7 (core.fsyncmethod: tests for batch mode, 2022-04-04)\nand a242c150ebb (vimdiff: integrate layout tests in the unit tests\nframework ('t' folder), 2022-03-30) for the two relevant commits. Both\nwere first released with v2.37.0-rc0 (and were also part of v2.37.0).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t3700-add.sh            | 2 +-\n t/t3903-stash.sh          | 2 +-\n t/t7609-mergetool--lib.sh | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 8979c8a5f03..8689b48589c 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -8,7 +8,7 @@ test_description='Test of git add, including the -- option.'\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-. $TEST_DIRECTORY/lib-unique-files.sh\n+. \"$TEST_DIRECTORY\"/lib-unique-files.sh\n \n # Test the file mode \"$1\" of the file \"$2\" in the index.\n test_mode_in_index () {\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 20e94881964..2a4c3fd61c0 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n-. $TEST_DIRECTORY/lib-unique-files.sh\n+. \"$TEST_DIRECTORY\"/lib-unique-files.sh\n \n test_expect_success 'usage on cmd and subcommand invalid option' '\n \ttest_expect_code 129 git stash --invalid-option 2>usage &&\ndiff --git a/t/t7609-mergetool--lib.sh b/t/t7609-mergetool--lib.sh\nindex d848fe6442b..330d6d603d7 100755\n--- a/t/t7609-mergetool--lib.sh\n+++ b/t/t7609-mergetool--lib.sh\n@@ -7,7 +7,7 @@ Testing basic merge tools options'\n . ./test-lib.sh\n \n test_expect_success 'mergetool --tool=vimdiff creates the expected layout' '\n-\t. $GIT_BUILD_DIR/mergetools/vimdiff &&\n+\t. \"$GIT_BUILD_DIR\"/mergetools/vimdiff &&\n \trun_unit_tests\n '\n \n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458175","messageId":"cover-v2-0.3-00000000000-20220630T101646Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com","subject":"[PATCH v2 0/3] tests: fix tests broken if a \" \" is in the checkout dir's path","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T10:18:33Z","receivedAt":"2022-06-30T10:19:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A re-roll of\nhttps://lore.kernel.org/git/cover-0.3-00000000000-20220621T221928Z-avarab@gmail.com/;\nChanges since v1:\n\n - Clarified IFS splitting issues in 1/3, as requested by Junio\n - Fixed $HOME quoting issue in 3/3, and also added the missing cleanup\n   for $HOME/.gitconfig to that test.\n\nÆvar Arnfjörð Bjarmason (3):\n  tests: add missing double quotes to included library paths\n  test-lib.sh: fix prepend_var() quoting issue\n  config tests: fix harmless but broken \"rm -r\" cleanup\n\n t/t1300-config.sh         | 5 +++--\n t/t3700-add.sh            | 2 +-\n t/t3903-stash.sh          | 2 +-\n t/t7609-mergetool--lib.sh | 2 +-\n t/test-lib.sh             | 4 ++--\n 5 files changed, 8 insertions(+), 7 deletions(-)\n\nRange-diff against v1:\n1:  05ba5e7f837 ! 1:  f4ef137d076 tests: add missing double quotes to included library paths\n    @@ Metadata\n      ## Commit message ##\n         tests: add missing double quotes to included library paths\n     \n    -    Fix two inclusions added in d42bab442d7 (core.fsyncmethod: tests for\n    -    batch mode, 2022-04-04) that needed to be quoted, and the same sort of\n    -    issue in a242c150ebb (vimdiff: integrate layout tests in the unit\n    -    tests framework ('t' folder), 2022-03-30). Both were first released\n    -    with v2.37.0-rc0.\n    +    Fix inclusion errors which would occur if the $TEST_DIRECTORY had $IFS\n    +    whitespace in it.\n    +\n    +    See d42bab442d7 (core.fsyncmethod: tests for batch mode, 2022-04-04)\n    +    and a242c150ebb (vimdiff: integrate layout tests in the unit tests\n    +    framework ('t' folder), 2022-03-30) for the two relevant commits. Both\n    +    were first released with v2.37.0-rc0 (and were also part of v2.37.0).\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n2:  e06bf0cdfbe = 2:  b56ededf1b8 test-lib.sh: fix prepend_var() quoting issue\n3:  f787b19f8c2 ! 3:  d3f65326701 config tests: fix harmless but broken \"rm -r\" cleanup\n    @@ Commit message\n         fail to remove an existing directory, but in practice that probably\n         never happened.\n     \n    +    Let's fix both the quoting issue, and the other issue cleanup issue in\n    +    4179b4897f2, which is that we were attempting to clean up\n    +    ~/.config/git, but weren't cleaing up ~/.gitconfig.\n    +\n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## t/t1300-config.sh ##\n    @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n      \n      test_expect_success 'override global and system config' '\n     -\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n    -+\ttest_when_finished \"rm -rf \\\"$HOME\\\"/.config/git\" &&\n    - \n    +-\n    ++\ttest_when_finished rm -f \\\"\\$HOME\\\"/.gitconfig &&\n      \tcat >\"$HOME\"/.gitconfig <<-EOF &&\n      \t[home]\n    + \t\tconfig = true\n    + \tEOF\n    ++\n    ++\ttest_when_finished rm -rf \\\"\\$HOME\\\"/.config/git &&\n    + \tmkdir -p \"$HOME\"/.config/git &&\n    + \tcat >\"$HOME\"/.config/git/config <<-EOF &&\n    + \t[xdg]\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458176","messageId":"patch-v2-2.3-b56ededf1b8-20220630T101646Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-v2-0.3-00000000000-20220630T101646Z-avarab@gmail.com","subject":"[PATCH v2 2/3] test-lib.sh: fix prepend_var() quoting issue","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T10:18:35Z","receivedAt":"2022-06-30T10:19:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a quoting issue in the function introduced in\nb9638d7286f (test-lib: make $GIT_BUILD_DIR an absolute path,\n2022-02-27), running the test suite where the git checkout was on a\npath with e.g. a space in it would fail.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 55857af601b..8cabb4d10f9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -57,14 +57,14 @@ fi\n #\n #\tprepend_var VAR : VALUE\n prepend_var () {\n-\teval \"$1=$3\\${$1:+${3:+$2}\\$$1}\"\n+\teval \"$1=\\\"$3\\${$1:+${3:+$2}\\$$1}\\\"\"\n }\n \n # If [AL]SAN is in effect we want to abort so that we notice\n # problems. The GIT_SAN_OPTIONS variable can be used to set common\n # defaults shared between [AL]SAN_OPTIONS.\n prepend_var GIT_SAN_OPTIONS : abort_on_error=1\n-prepend_var GIT_SAN_OPTIONS : strip_path_prefix=\\\"$GIT_BUILD_DIR/\\\"\n+prepend_var GIT_SAN_OPTIONS : strip_path_prefix=\"$GIT_BUILD_DIR/\"\n \n # If we were built with ASAN, it may complain about leaks\n # of program-lifetime variables. Disable it by default to lower\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458177","messageId":"patch-v2-3.3-d3f65326701-20220630T101646Z-avarab@gmail.com","threadId":"58045","inReplyTo":"cover-v2-0.3-00000000000-20220630T101646Z-avarab@gmail.com","subject":"[PATCH v2 3/3] config tests: fix harmless but broken \"rm -r\" cleanup","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T10:18:36Z","receivedAt":"2022-06-30T10:19:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"test_when_finished\" cleanup phase added in 4179b4897f2 (config:\nallow overriding of global and system configuration, 2021-04-19) has\nnever worked as intended, firstly the \".config/git\" is a directory, so\nwe'd need the \"-r\" flag, but more importantly the $HOME variable\nwasn't properly quoted.\n\nWe'd thus end up trying to remove the \"trash\" part of \"trash\ndirectory\", which wouldn't fail with \"-f\", since \"rm -f\" won't fail on\nnon-existing files.\n\nIt's possible that this would have caused an actual failure if someone\nhad a $HOME with a space character in it, such that our \"rm -f\" would\nfail to remove an existing directory, but in practice that probably\nnever happened.\n\nLet's fix both the quoting issue, and the other issue cleanup issue in\n4179b4897f2, which is that we were attempting to clean up\n~/.config/git, but weren't cleaing up ~/.gitconfig.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t1300-config.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex d3d9adbb3db..c6661e61af5 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2083,12 +2083,13 @@ test_expect_success '--show-scope with --show-origin' '\n '\n \n test_expect_success 'override global and system config' '\n-\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n-\n+\ttest_when_finished rm -f \\\"\\$HOME\\\"/.gitconfig &&\n \tcat >\"$HOME\"/.gitconfig <<-EOF &&\n \t[home]\n \t\tconfig = true\n \tEOF\n+\n+\ttest_when_finished rm -rf \\\"\\$HOME\\\"/.config/git &&\n \tmkdir -p \"$HOME\"/.config/git &&\n \tcat >\"$HOME\"/.config/git/config <<-EOF &&\n \t[xdg]\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458248","messageId":"xmqq8rpdeuwq.fsf@gitster.g","threadId":"58045","inReplyTo":"patch-v2-3.3-d3f65326701-20220630T101646Z-avarab@gmail.com","subject":"Re: [PATCH v2 3/3] config tests: fix harmless but broken \"rm -r\" cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-30T20:52:21Z","receivedAt":"2022-06-30T20:52:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> diff --git a/t/t1300-config.sh b/t/t1300-config.sh\n> index d3d9adbb3db..c6661e61af5 100755\n> --- a/t/t1300-config.sh\n> +++ b/t/t1300-config.sh\n> @@ -2083,12 +2083,13 @@ test_expect_success '--show-scope with --show-origin' '\n>  '\n>  \n>  test_expect_success 'override global and system config' '\n> -\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n> -\n> +\ttest_when_finished rm -f \\\"\\$HOME\\\"/.gitconfig &&\n\nThis does look more correct than the original.\n\n>  \tcat >\"$HOME\"/.gitconfig <<-EOF &&\n>  \t[home]\n>  \t\tconfig = true\n>  \tEOF\n> +\n> +\ttest_when_finished rm -rf \\\"\\$HOME\\\"/.config/git &&\n>  \tmkdir -p \"$HOME\"/.config/git &&\n>  \tcat >\"$HOME\"/.config/git/config <<-EOF &&\n>  \t[xdg]\n\nNicely done.  Will queue.\n\nThanks.\n"}]}