{"thread":{"id":"60294","subject":"[PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","startedAt":"2023-10-03T08:24:11Z","lastAt":"2023-10-04T18:24:16Z","messageCount":9,"participants":["Štěpán Němec","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"482556","messageId":"20231003082323.3002663-1-stepnem@smrk.net","threadId":"60294","inReplyTo":null,"subject":"[PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Štěpán Němec","fromEmail":"stepnem@smrk.net","sentAt":"2023-10-03T08:23:23Z","receivedAt":"2023-10-03T08:24:11Z","isPatch":true,"sender":{"key":"stepnem@smrk.net","avatar":"https://avatars.githubusercontent.com/u/106838?v=4"},"body":"Turns out having `pwd` (which TEST_DIRECTORY defaults to) print $PWD\nwith a trailing slash isn't very difficult, in my case it went something\nlike\n\n  ; tmux new-window -c ~/src/git/t/\n  [in the new window]\n  ; sh ./t0000-basic.sh\n  PANIC: Running in a /home/stepnem/src/git/t/ that doesn't end in '/t'?\n  ; pwd\n  /home/stepnem/src/git/t/\n\n(tmux(1) apparently sets PWD in the environment in addition to calling\nchdir(2), which seems enough to make at least some shells preserve the\ntrailing slash in `pwd` output.)\n\nStrip the trailing slash, if present, to prevent bailing out with the\nPANIC message in such cases.\n\nSigned-off-by: Štěpán Němec <stepnem@smrk.net>\n---\n t/test-lib.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1656c9eed006..3b6f1a17e349 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -35,6 +35,7 @@ else\n \t# needing to exist.\n \tTEST_DIRECTORY=$(cd \"$TEST_DIRECTORY\" && pwd) || exit 1\n fi\n+TEST_DIRECTORY=\"${TEST_DIRECTORY%/}\"\n if test -z \"$TEST_OUTPUT_DIRECTORY\"\n then\n \t# Similarly, override this to store the test-results subdir\n\nbase-commit: d0e8084c65cbf949038ae4cc344ac2c2efd77415\n-- \n2.42.0\n\n"},{"id":"482621","messageId":"xmqqwmw3wgeo.fsf@gitster.g","threadId":"60294","inReplyTo":"20231003082323.3002663-1-stepnem@smrk.net","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-03T21:21:51Z","receivedAt":"2023-10-03T21:21:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Štěpán Němec <stepnem@smrk.net> writes:\n\n> Turns out having `pwd` (which TEST_DIRECTORY defaults to) print $PWD\n> with a trailing slash isn't very difficult, in my case it went something\n> like\n>\n>   ; tmux new-window -c ~/src/git/t/\n>   [in the new window]\n>   ; sh ./t0000-basic.sh\n>   PANIC: Running in a /home/stepnem/src/git/t/ that doesn't end in '/t'?\n>   ; pwd\n>   /home/stepnem/src/git/t/\n>\n> (tmux(1) apparently sets PWD in the environment in addition to calling\n> chdir(2), which seems enough to make at least some shells preserve the\n> trailing slash in `pwd` output.)\n>\n> Strip the trailing slash, if present, to prevent bailing out with the\n> PANIC message in such cases.\n>\n> Signed-off-by: Štěpán Němec <stepnem@smrk.net>\n> ---\n>  t/test-lib.sh | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1656c9eed006..3b6f1a17e349 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -35,6 +35,7 @@ else\n>  \t# needing to exist.\n>  \tTEST_DIRECTORY=$(cd \"$TEST_DIRECTORY\" && pwd) || exit 1\n>  fi\n> +TEST_DIRECTORY=\"${TEST_DIRECTORY%/}\"\n>  if test -z \"$TEST_OUTPUT_DIRECTORY\"\n>  then\n>  \t# Similarly, override this to store the test-results subdir\n>\n> base-commit: d0e8084c65cbf949038ae4cc344ac2c2efd77415\n\nWhile this would certainly squelch the particular test on TEST_DIRECTORY\nI am not sure if a safer fix would be to fix your PWD when it has a\ntrailing slash.  Your tests with this patch may be taking unintended\nbranch when they do something like\n\n\tif test \"$TRASH_DIRECTORY\" = \"$(pwd)\"\n\tthen\n\t\t...\n\tfi\n\nas TRASH_DIRECTORY, whose default is derived from TEST_DIRECTORY and\nthanks to your fix it now does not have an extra slash in it, does\nnot have a trailing slash but your $(pwd) or $PWD would have the\ntrailing slash, simply because the patch only fixes TEST_DIRECTORY\nwithout fixing PWD.\n\nI wonder if this would be a safer alternative, or is it doing too\nmuch more than what is necessary?\n\n t/test-lib.sh | 31 +++++++++++++++++++------------\n 1 file changed, 19 insertions(+), 12 deletions(-)\n\ndiff --git c/t/test-lib.sh w/t/test-lib.sh\nindex 1656c9eed0..6ec42eab0d 100644\n--- c/t/test-lib.sh\n+++ w/t/test-lib.sh\n@@ -22,19 +22,26 @@ then\n \t# ensure that TEST_DIRECTORY is an absolute path so that it\n \t# is valid even if the current working directory is changed\n \tTEST_DIRECTORY=$(pwd)\n-else\n-\t# The TEST_DIRECTORY will always be the path to the \"t\"\n-\t# directory in the git.git checkout. This is overridden by\n-\t# e.g. t/lib-subtest.sh, but only because its $(pwd) is\n-\t# different. Those tests still set \"$TEST_DIRECTORY\" to the\n-\t# same path.\n-\t#\n-\t# See use of \"$GIT_BUILD_DIR\" and \"$TEST_DIRECTORY\" below for\n-\t# hard assumptions about \"$GIT_BUILD_DIR/t\" existing and being\n-\t# the \"$TEST_DIRECTORY\", and e.g. \"$TEST_DIRECTORY/helper\"\n-\t# needing to exist.\n-\tTEST_DIRECTORY=$(cd \"$TEST_DIRECTORY\" && pwd) || exit 1\n fi\n+\n+# The TEST_DIRECTORY will always be the path to the \"t\"\n+# directory in the git.git checkout. This is overridden by\n+# e.g. t/lib-subtest.sh, but only because its $(pwd) is\n+# different. Those tests still set \"$TEST_DIRECTORY\" to the\n+# same path.\n+#\n+# See use of \"$GIT_BUILD_DIR\" and \"$TEST_DIRECTORY\" below for\n+# hard assumptions about \"$GIT_BUILD_DIR/t\" existing and being\n+# the \"$TEST_DIRECTORY\", and e.g. \"$TEST_DIRECTORY/helper\"\n+# needing to exist.\n+#\n+# Also, apparently a shell spawned in some tricky way can be\n+# talked into keeping a slash at the end of its $PWD.\n+#\n+# All of the above can be worked around by doing an extra chdir\n+# and asking where we ended up to be.\n+TEST_DIRECTORY=$(cd \"$TEST_DIRECTORY/.\" && pwd) || exit 1\n+\n if test -z \"$TEST_OUTPUT_DIRECTORY\"\n then\n \t# Similarly, override this to store the test-results subdir\n"},{"id":"482626","messageId":"xmqqjzs3wer0.fsf@gitster.g","threadId":"60294","inReplyTo":"xmqqwmw3wgeo.fsf@gitster.g","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-03T21:57:39Z","receivedAt":"2023-10-03T21:57:45Z","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> I wonder if this would be a safer alternative, or is it doing too\n> much more than what is necessary?\n\nAn alternative with much smaller blast radius would be to do this.\n\nHopefully, by going \"$(pwd)/.\" before asking the value returned by\nthe `pwd` command, we can make sure that the trailing slash is\nremoved (or at least $(pwd) and $TEST_DIRECTORY should be identical\nafter this is done).\n\n\n\n t/test-lib.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git c/t/test-lib.sh w/t/test-lib.sh\nindex 1656c9eed0..d159358b21 100644\n--- c/t/test-lib.sh\n+++ w/t/test-lib.sh\n@@ -19,9 +19,13 @@\n # t/ subdirectory and are run in 'trash directory' subdirectory.\n if test -z \"$TEST_DIRECTORY\"\n then\n-\t# ensure that TEST_DIRECTORY is an absolute path so that it\n+\t# Ensure that TEST_DIRECTORY is an absolute path so that it\n \t# is valid even if the current working directory is changed\n-\tTEST_DIRECTORY=$(pwd)\n+\t# Some environments can talk the shell into keeping trailing\n+\t# slash in $PWD---go there and ask where we are to work it\n+\t# around, as we expect TEST_DIRECTORY and PWD are both\n+\t# canonical and can textually be compared for equality\n+\tTEST_DIRECTORY=$(cd \"$(pwd)/.\" && pwd)\n else\n \t# The TEST_DIRECTORY will always be the path to the \"t\"\n \t# directory in the git.git checkout. This is overridden by\n"},{"id":"482637","messageId":"20231004113413+0200.161419-stepnem@smrk.net","threadId":"60294","inReplyTo":"xmqqjzs3wer0.fsf@gitster.g","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Štěpán Němec","fromEmail":"stepnem@smrk.net","sentAt":"2023-10-04T09:34:13Z","receivedAt":"2023-10-04T09:34:27Z","isPatch":true,"sender":{"key":"stepnem@smrk.net","avatar":"https://avatars.githubusercontent.com/u/106838?v=4"},"body":"On Tue, 03 Oct 2023 14:57:39 -0700\nJunio C. Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> I wonder if this would be a safer alternative, or is it doing too\n>> much more than what is necessary?\n>\n> An alternative with much smaller blast radius would be to do this.\n>\n> Hopefully, by going \"$(pwd)/.\" before asking the value returned by\n> the `pwd` command, we can make sure that the trailing slash is\n> removed (or at least $(pwd) and $TEST_DIRECTORY should be identical\n> after this is done).\n>\n>\n>\n>  t/test-lib.sh | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git c/t/test-lib.sh w/t/test-lib.sh\n> index 1656c9eed0..d159358b21 100644\n> --- c/t/test-lib.sh\n> +++ w/t/test-lib.sh\n> @@ -19,9 +19,13 @@\n>  # t/ subdirectory and are run in 'trash directory' subdirectory.\n>  if test -z \"$TEST_DIRECTORY\"\n>  then\n> -\t# ensure that TEST_DIRECTORY is an absolute path so that it\n> +\t# Ensure that TEST_DIRECTORY is an absolute path so that it\n>  \t# is valid even if the current working directory is changed\n> -\tTEST_DIRECTORY=$(pwd)\n> +\t# Some environments can talk the shell into keeping trailing\n> +\t# slash in $PWD---go there and ask where we are to work it\n> +\t# around, as we expect TEST_DIRECTORY and PWD are both\n> +\t# canonical and can textually be compared for equality\n> +\tTEST_DIRECTORY=$(cd \"$(pwd)/.\" && pwd)\n>  else\n>  \t# The TEST_DIRECTORY will always be the path to the \"t\"\n>  \t# directory in the git.git checkout. This is overridden by\n\nYes, actually, AFAICT just $(cd . && pwd) fixes things (and saves a few\nsyscalls), and I agree this is a much better approach than my naive fix.\n\nAnd functionally it should be equivalent to your other solution that did\nthis unconditionally, because the else branch (TEST_DIRECTORY set) was\nalready cd-ing anyway.\n\nThanks,\n\n  Štěpán\n"},{"id":"482657","messageId":"xmqqbkdes6lf.fsf@gitster.g","threadId":"60294","inReplyTo":"20231004113413+0200.161419-stepnem@smrk.net","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-04T16:19:40Z","receivedAt":"2023-10-04T16:19:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Štěpán Němec <stepnem@smrk.net> writes:\n\n> Yes, actually, AFAICT just $(cd . && pwd) fixes things (and saves a few\n> syscalls), and I agree this is a much better approach than my naive fix.\n\nActually I was still being silly.  We sometimes do\n\n\tval=$(cd there && do something there)\n\nso that we can get the output from a command in a different\ndirectory _without_ having to move our current directory.  But the\npoint of this current topic is that we _need_ to convince the shell\nthat the path to our current directory is a canonical one without\ntrailing slash, so my silly 'cd \"$(pwd)/.\"' (or your \"cd .\") should\nbe done outside the command expansion, or the canonicalized $PWD will\nonly appear inside the $() and the next reference to $(pwd) or $PWD\nin the test script will still give the path with the trailing slash,\nthat is textually different from $TEST_DIRECTORY.\n\nI wonder if this works better for you.  We would be sure that $PWD\nand $TEST_DIRECTORY (when the latter is not imported from the\nenvironment) are the same, so \"your cwd that does not end with /t\nand has a trailing slash after it\" would be gone.  Any $PWD or $(pwd)\nthe tests refer to later in the step will also lack the unwanted\ntrailing slash.   As long as \"cd .\" is sufficient to cause the shell\nreexamine and canonicalize the $PWD, that is.\n\nThanks.\n\n t/test-lib.sh | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git c/t/test-lib.sh w/t/test-lib.sh\nindex 1656c9eed0..a7045e028c 100644\n--- c/t/test-lib.sh\n+++ w/t/test-lib.sh\n@@ -19,6 +19,11 @@\n # t/ subdirectory and are run in 'trash directory' subdirectory.\n if test -z \"$TEST_DIRECTORY\"\n then\n+\t# It is reported that some shells spawned in tricky ways can\n+\t# give $PWD with a trailing slash. An explicit chdir hopefully\n+\t# would wake them out of their hallucination.\n+\tcd .\n+\n \t# ensure that TEST_DIRECTORY is an absolute path so that it\n \t# is valid even if the current working directory is changed\n \tTEST_DIRECTORY=$(pwd)\n@@ -626,7 +631,6 @@ fi\n # Protect ourselves from common misconfiguration to export\n # CDPATH into the environment\n unset CDPATH\n-\n unset GREP_OPTIONS\n unset UNZIP\n \n"},{"id":"482660","messageId":"20231004190140+0200.740775-stepnem@smrk.net","threadId":"60294","inReplyTo":"xmqqbkdes6lf.fsf@gitster.g","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Štěpán Němec","fromEmail":"stepnem@smrk.net","sentAt":"2023-10-04T17:01:40Z","receivedAt":"2023-10-04T17:01:50Z","isPatch":true,"sender":{"key":"stepnem@smrk.net","avatar":"https://avatars.githubusercontent.com/u/106838?v=4"},"body":"On Wed, 04 Oct 2023 09:19:40 -0700\nJunio C. Hamano wrote:\n\n> Štěpán Němec <stepnem@smrk.net> writes:\n>\n>> Yes, actually, AFAICT just $(cd . && pwd) fixes things (and saves a few\n>> syscalls), and I agree this is a much better approach than my naive fix.\n>\n> Actually I was still being silly.  We sometimes do\n>\n> \tval=$(cd there && do something there)\n>\n> so that we can get the output from a command in a different\n> directory _without_ having to move our current directory.  But the\n> point of this current topic is that we _need_ to convince the shell\n> that the path to our current directory is a canonical one without\n> trailing slash, so my silly 'cd \"$(pwd)/.\"' (or your \"cd .\") should\n> be done outside the command expansion, or the canonicalized $PWD will\n> only appear inside the $() and the next reference to $(pwd) or $PWD\n> in the test script will still give the path with the trailing slash,\n> that is textually different from $TEST_DIRECTORY.\n>\n> I wonder if this works better for you.  We would be sure that $PWD\n> and $TEST_DIRECTORY (when the latter is not imported from the\n> environment) are the same, so \"your cwd that does not end with /t\n> and has a trailing slash after it\" would be gone.  Any $PWD or $(pwd)\n> the tests refer to later in the step will also lack the unwanted\n> trailing slash.   As long as \"cd .\" is sufficient to cause the shell\n> reexamine and canonicalize the $PWD, that is.\n>\n> Thanks.\n>\n>  t/test-lib.sh | 6 +++++-\n>  1 file changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git c/t/test-lib.sh w/t/test-lib.sh\n> index 1656c9eed0..a7045e028c 100644\n> --- c/t/test-lib.sh\n> +++ w/t/test-lib.sh\n> @@ -19,6 +19,11 @@\n>  # t/ subdirectory and are run in 'trash directory' subdirectory.\n>  if test -z \"$TEST_DIRECTORY\"\n>  then\n> +\t# It is reported that some shells spawned in tricky ways can\n> +\t# give $PWD with a trailing slash. An explicit chdir hopefully\n> +\t# would wake them out of their hallucination.\n> +\tcd .\n> +\n>  \t# ensure that TEST_DIRECTORY is an absolute path so that it\n>  \t# is valid even if the current working directory is changed\n>  \tTEST_DIRECTORY=$(pwd)\n> @@ -626,7 +631,6 @@ fi\n>  # Protect ourselves from common misconfiguration to export\n>  # CDPATH into the environment\n>  unset CDPATH\n> -\n>  unset GREP_OPTIONS\n>  unset UNZIP\n\nYes, this is even simpler and more obvious, although for some reason,\nthe subshell version works just as well, i.e., with just\nTEST_DIRECTORY=$(cd . && pwd) (no cd in the parent) in test-lib.sh and\n\ncat <<EOF >./tslash.sh\n#!/bin/sh\ntest_description='yada yada'\n\n. ./test-lib.sh\n\ntest_expect_success 'check TEST_DIRECTORY for trailing slash' '\n        echo \"$TEST_DIRECTORY\" &&\n        test \"$TEST_DIRECTORY\" = \"${TEST_DIRECTORY%/}\"\n'\n\ntest_done\nEOF\n\nI get\n\n; echo ${TEST_DIRECTORY-unset}\nunset\n; pwd\n/home/stepnem/src/git/t/\n; echo $PWD\n/home/stepnem/src/git/t/\n; sh ./tslash.sh -v\nInitialized empty Git repository in /home/stepnem/src/git/t/trash directory.tslash/.git/\nexpecting success of slash.1 'check TEST_DIRECTORY for trailing slash':\n        echo \"$TEST_DIRECTORY\" &&\n        test \"$TEST_DIRECTORY\" = \"${TEST_DIRECTORY%/}\"\n\n/home/stepnem/src/git/t\nok 1 - check TEST_DIRECTORY for trailing slash\n\n# passed all 1 test(s)\n1..1\n; pwd\n/home/stepnem/src/git/t/\n; echo $PWD\n/home/stepnem/src/git/t/\n\n[and, needless to say, reverting the changes still triggers the panic:\n; sh ./tslash.sh\nPANIC: Running in a /home/stepnem/src/git/t/ that doesn't end in '/t'?\n]\n\nGo figure...\n\nIn any case, for what it's worth, any of your versions is\nTested-by: Štěpán Němec <stepnem@smrk.net>\n\nThanks,\n\n  Štěpán\n"},{"id":"482661","messageId":"xmqqttr6pavh.fsf@gitster.g","threadId":"60294","inReplyTo":"20231004190140+0200.740775-stepnem@smrk.net","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-04T17:15:30Z","receivedAt":"2023-10-04T17:15:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Štěpán Němec <stepnem@smrk.net> writes:\n\n> Yes, this is even simpler and more obvious, although for some reason,\n> the subshell version works just as well, i.e., with just\n> TEST_DIRECTORY=$(cd . && pwd) (no cd in the parent) in test-lib.sh and\n>\n> cat <<EOF >./tslash.sh\n> #!/bin/sh\n> test_description='yada yada'\n>\n> . ./test-lib.sh\n>\n> test_expect_success 'check TEST_DIRECTORY for trailing slash' '\n>         echo \"$TEST_DIRECTORY\" &&\n>         test \"$TEST_DIRECTORY\" = \"${TEST_DIRECTORY%/}\"\n> '\n>\n> test_done\n> EOF\n\nIf the only thing you checked was if TEST_DIRECTORY has or does not\nhave a trailing slash, then it is totally understandable how and why\nthe chdir inside subshell works.  The value $(pwd) in the subshell\nreturns, that is assigned to TEST_DIRECTORY, is canonicalized.  But\nthe outer shell still thinks $(pwd) is with trailing slash, and the\nabove does not test it.\n\n    test_expect_success 'check $PWD for trailing slash' '\n            echo \"$PWD\" &&\n            test \"$PWD\" = \"${PWD%/}\"\n    '\n\nThe primary reason why I said I was silly doing it in a subshell was\nbecause I wanted to make sure that any test that refers to $PWD or\n$(pwd) later in the code would not get upset the same way with\ntrailing slash after $PWD or $(pwd).  As long as it is not overdone,\nit is a good practice to consider possibilities of similar problems\ntriggered by the same root cause, which we may not have got bitten\nby yet, and preventing it from happening with the same fix.\n\nThanks.\n"},{"id":"482662","messageId":"20231004194046+0200.353534-stepnem@smrk.net","threadId":"60294","inReplyTo":"xmqqttr6pavh.fsf@gitster.g","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Štěpán Němec","fromEmail":"stepnem@smrk.net","sentAt":"2023-10-04T17:40:46Z","receivedAt":"2023-10-04T17:40:52Z","isPatch":true,"sender":{"key":"stepnem@smrk.net","avatar":"https://avatars.githubusercontent.com/u/106838?v=4"},"body":"On Wed, 04 Oct 2023 10:15:30 -0700\nJunio C. Hamano wrote:\n\n> Štěpán Němec <stepnem@smrk.net> writes:\n>\n>> Yes, this is even simpler and more obvious, although for some reason,\n>> the subshell version works just as well, i.e., with just\n>> TEST_DIRECTORY=$(cd . && pwd) (no cd in the parent) in test-lib.sh and\n>>\n>> cat <<EOF >./tslash.sh\n>> #!/bin/sh\n>> test_description='yada yada'\n>>\n>> . ./test-lib.sh\n>>\n>> test_expect_success 'check TEST_DIRECTORY for trailing slash' '\n>>         echo \"$TEST_DIRECTORY\" &&\n>>         test \"$TEST_DIRECTORY\" = \"${TEST_DIRECTORY%/}\"\n>> '\n>>\n>> test_done\n>> EOF\n>\n> If the only thing you checked was if TEST_DIRECTORY has or does not\n> have a trailing slash, then it is totally understandable how and why\n> the chdir inside subshell works.  The value $(pwd) in the subshell\n> returns, that is assigned to TEST_DIRECTORY, is canonicalized.  But\n> the outer shell still thinks $(pwd) is with trailing slash, and the\n> above does not test it.\n\nOh my, of course... thanks.\n\n>\n>     test_expect_success 'check $PWD for trailing slash' '\n>             echo \"$PWD\" &&\n>             test \"$PWD\" = \"${PWD%/}\"\n>     '\n\n> The primary reason why I said I was silly doing it in a subshell was\n> because I wanted to make sure that any test that refers to $PWD or\n> $(pwd) later in the code would not get upset the same way with\n> trailing slash after $PWD or $(pwd).\n\nThe $PWD inside the test is the trash directory, though, so that's not\nthe problematic $PWD with a trailing slash any more, so I guess this\nproblem can't really happen in a test that follows the '. ./test-lib.sh'\n(which does cd -P \"$TRASH_DIRECTORY\") convention?\n\n-- \nŠtěpán\n"},{"id":"482666","messageId":"xmqqpm1up7p1.fsf@gitster.g","threadId":"60294","inReplyTo":"20231004194046+0200.353534-stepnem@smrk.net","subject":"Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-04T18:24:10Z","receivedAt":"2023-10-04T18:24:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Štěpán Němec <stepnem@smrk.net> writes:\n\n> The $PWD inside the test is the trash directory, though, so that's not\n> the problematic $PWD with a trailing slash any more, so I guess this\n> problem can't really happen in a test that follows the '. ./test-lib.sh'\n> (which does cd -P \"$TRASH_DIRECTORY\") convention?\n\nYeah, after the dot-include of test-lib.sh returns, everything\nshould be safe.  What happens inside test-lib.sh (and worse yet,\ninside future versions of that file) before that \"cd -P\" is what\nthe extra-carefulness protects against.\n"}]}