{"thread":{"id":"46522","subject":"[PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","startedAt":"2017-08-06T23:39:05Z","lastAt":"2021-07-30T17:16:54Z","messageCount":17,"participants":["Ævar Arnfjörð Bjarmason","brian m. carlson","Junio C Hamano","René Scharfe","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"325662","messageId":"20170806233850.14711-1-avarab@gmail.com","threadId":"46522","inReplyTo":null,"subject":"[PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-08-06T23:38:50Z","receivedAt":"2017-08-06T23:39:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change an argument to test_line_count (which'll ultimately be turned\ninto a \"test\" expression) to use \"-gt\" instead of \">\" for an\narithmetic test.\n\nThis broken on e.g. OpenBSD as of v2.13.0 with my commit\nac3f5a3468 (\"ref-filter: add --no-contains option to\ntag/branch/for-each-ref\", 2017-03-24).\n\nUpstream just worked around it by patching git and didn't tell us\nabout it, I discovered this when reading various Git packaging\nimplementations: https://github.com/openbsd/ports/commit/7e48bf88a20\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nDavid, it would be great to get a quick bug report to\ngit@vger.kernel.org if you end up having to monkeypatch something\nwe've done. We won't bite, promise :)\n\nAs shown in that linked Github commit OpenBSD has another recent\nworkaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a\nrelated test, maybe René can make more sense of that?\n\nThere's more patches in their ports which indicate possible bugs of\nours: https://github.com/openbsd/ports/tree/master/devel/git/patches/\n\n t/t7004-tag.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 0ef7b94394..0e2e57aa3d 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1887,7 +1887,7 @@ EOF\"\n \trun_with_limited_stack git tag --contains HEAD >actual &&\n \ttest_cmp expect actual &&\n \trun_with_limited_stack git tag --no-contains HEAD >actual &&\n-\ttest_line_count \">\" 10 actual\n+\ttest_line_count \"-gt\" 10 actual\n '\n \n test_expect_success '--format should list tags as per format given' '\n-- \n2.14.0.rc1.383.gd1ce394fe2\n\n"},{"id":"325663","messageId":"20170807011832.oy6lxf2ewkmzg65s@genre.crustytoothpaste.net","threadId":"46522","inReplyTo":"20170806233850.14711-1-avarab@gmail.com","subject":"Re: [PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-07T01:18:32Z","receivedAt":"2017-08-07T01:18:45Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Aug 06, 2017 at 11:38:50PM +0000, Ævar Arnfjörð Bjarmason wrote:\n> Change an argument to test_line_count (which'll ultimately be turned\n> into a \"test\" expression) to use \"-gt\" instead of \">\" for an\n> arithmetic test.\n> \n> This broken on e.g. OpenBSD as of v2.13.0 with my commit\n> ac3f5a3468 (\"ref-filter: add --no-contains option to\n> tag/branch/for-each-ref\", 2017-03-24).\n> \n> Upstream just worked around it by patching git and didn't tell us\n> about it, I discovered this when reading various Git packaging\n> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n> \n> David, it would be great to get a quick bug report to\n> git@vger.kernel.org if you end up having to monkeypatch something\n> we've done. We won't bite, promise :)\n> \n> As shown in that linked Github commit OpenBSD has another recent\n> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a\n> related test, maybe René can make more sense of that?\n\nI've confirmed using the NetBSD 7.1 man pages that NetBSD will also want\nDIR_HAS_BSD_GROUP_SEMANTICS.  MirBSD will also, according to its man\npages.\n\nAs I understand it, the only consequence of not setting this flag on BSD\nsystems is that some directories will be setgid, which, while ugly and\nuseless, should have no negative effect.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"325665","messageId":"xmqq4ltkrwkk.fsf@gitster.mtv.corp.google.com","threadId":"46522","inReplyTo":"20170806233850.14711-1-avarab@gmail.com","subject":"Re: [PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T01:33:15Z","receivedAt":"2017-08-07T01:33:28Z","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> Change an argument to test_line_count (which'll ultimately be turned\n> into a \"test\" expression) to use \"-gt\" instead of \">\" for an\n> arithmetic test.\n>\n> This broken on e.g. OpenBSD as of v2.13.0 with my commit\n> ac3f5a3468 (\"ref-filter: add --no-contains option to\n> tag/branch/for-each-ref\", 2017-03-24).\n>\n> Upstream just worked around it by patching git and didn't tell us\n> about it, I discovered this when reading various Git packaging\n> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20\n\nThanks for finding and relaying this.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>\n> David, it would be great to get a quick bug report to\n> git@vger.kernel.org if you end up having to monkeypatch something\n> we've done. We won't bite, promise :)\n\nYeah.  I was hoping that your new list for platform porters would\nhelp communicate these issues easier for those who are a bit shy to\nbe on this general development list ;-)\n\n> As shown in that linked Github commit OpenBSD has another recent\n> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a\n> related test, maybe René can make more sense of that?\n>\n> There's more patches in their ports which indicate possible bugs of\n> ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/\n>\n>  t/t7004-tag.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 0ef7b94394..0e2e57aa3d 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1887,7 +1887,7 @@ EOF\"\n>  \trun_with_limited_stack git tag --contains HEAD >actual &&\n>  \ttest_cmp expect actual &&\n>  \trun_with_limited_stack git tag --no-contains HEAD >actual &&\n> -\ttest_line_count \">\" 10 actual\n> +\ttest_line_count \"-gt\" 10 actual\n>  '\n>  \n>  test_expect_success '--format should list tags as per format given' '\n"},{"id":"325673","messageId":"56fd58d6-acc7-042a-1b68-9854ed54361b@web.de","threadId":"46522","inReplyTo":"20170807011832.oy6lxf2ewkmzg65s@genre.crustytoothpaste.net","subject":"Re: [PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-07T10:17:48Z","receivedAt":"2017-08-07T10:18:22Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.08.2017 um 03:18 schrieb brian m. carlson:\n> On Sun, Aug 06, 2017 at 11:38:50PM +0000, Ævar Arnfjörð Bjarmason wrote:\n>> Change an argument to test_line_count (which'll ultimately be turned\n>> into a \"test\" expression) to use \"-gt\" instead of \">\" for an\n>> arithmetic test.\n>>\n>> This broken on e.g. OpenBSD as of v2.13.0 with my commit\n>> ac3f5a3468 (\"ref-filter: add --no-contains option to\n>> tag/branch/for-each-ref\", 2017-03-24).\n>>\n>> Upstream just worked around it by patching git and didn't tell us\n>> about it, I discovered this when reading various Git packaging\n>> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>> ---\n>>\n>> David, it would be great to get a quick bug report to\n>> git@vger.kernel.org if you end up having to monkeypatch something\n>> we've done. We won't bite, promise :)\n>>\n>> As shown in that linked Github commit OpenBSD has another recent\n>> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a\n>> related test, maybe René can make more sense of that?\n> \n> I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want\n> DIR_HAS_BSD_GROUP_SEMANTICS.  MirBSD will also, according to its man\n> pages.\n> \n> As I understand it, the only consequence of not setting this flag on BSD\n> systems is that some directories will be setgid, which, while ugly and\n> useless, should have no negative effect.\n\nRight; specifically it's for newly created subdirectories of shared\ndirectories, which we want to be owned by the same group as their\nparent. That's the default on BSDs, and we have to set the setgid bit to\nturn on that semantic only on other systems, e.g. on Linux.\n\n81a24b52c1 (Do not use GUID on dir in git init --shared=all on FreeBSD)\nintroduced DIR_HAS_BSD_GROUP_SEMANTICS with the rationale that setting\nsetgid on directories may not even be allowed for normal users.  I can't\nreproduce this (t1301 succeeds for me on FreeBSD 10.3 even without that\nbuild flag), but apparently at least in some configurations it's not just\na cosmetic issue.\n\nThe skipped test 'init in long base path' in t0001 is a different kettle\nof fish.  getcwd(3) on OpenBSD respects permissions on the parent\ndirectories up to root.  E.g. after \"chmod 711 /home\" normal users would\nget EACCES when they'd call getcwd(3) in their homes there, while e.g.\non Linux and FreeBSD they'd successfully get their current working dir.\n\nRené\n"},{"id":"325675","messageId":"873793zls6.fsf@gmail.com","threadId":"46522","inReplyTo":"56fd58d6-acc7-042a-1b68-9854ed54361b@web.de","subject":"Re: [PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-08-07T10:59:05Z","receivedAt":"2017-08-07T10:59:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Aug 07 2017, René Scharfe jotted:\n\n> Am 07.08.2017 um 03:18 schrieb brian m. carlson:\n>> On Sun, Aug 06, 2017 at 11:38:50PM +0000, Ævar Arnfjörð Bjarmason wrote:\n>>> Change an argument to test_line_count (which'll ultimately be turned\n>>> into a \"test\" expression) to use \"-gt\" instead of \">\" for an\n>>> arithmetic test.\n>>>\n>>> This broken on e.g. OpenBSD as of v2.13.0 with my commit\n>>> ac3f5a3468 (\"ref-filter: add --no-contains option to\n>>> tag/branch/for-each-ref\", 2017-03-24).\n>>>\n>>> Upstream just worked around it by patching git and didn't tell us\n>>> about it, I discovered this when reading various Git packaging\n>>> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20\n>>>\n>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>>> ---\n>>>\n>>> David, it would be great to get a quick bug report to\n>>> git@vger.kernel.org if you end up having to monkeypatch something\n>>> we've done. We won't bite, promise :)\n>>>\n>>> As shown in that linked Github commit OpenBSD has another recent\n>>> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a\n>>> related test, maybe René can make more sense of that?\n>>\n>> I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want\n>> DIR_HAS_BSD_GROUP_SEMANTICS.  MirBSD will also, according to its man\n>> pages.\n>>\n>> As I understand it, the only consequence of not setting this flag on BSD\n>> systems is that some directories will be setgid, which, while ugly and\n>> useless, should have no negative effect.\n>\n> Right; specifically it's for newly created subdirectories of shared\n> directories, which we want to be owned by the same group as their\n> parent. That's the default on BSDs, and we have to set the setgid bit to\n> turn on that semantic only on other systems, e.g. on Linux.\n>\n> 81a24b52c1 (Do not use GUID on dir in git init --shared=all on FreeBSD)\n> introduced DIR_HAS_BSD_GROUP_SEMANTICS with the rationale that setting\n> setgid on directories may not even be allowed for normal users.  I can't\n> reproduce this (t1301 succeeds for me on FreeBSD 10.3 even without that\n> build flag), but apparently at least in some configurations it's not just\n> a cosmetic issue.\n>\n> The skipped test 'init in long base path' in t0001 is a different kettle\n> of fish.  getcwd(3) on OpenBSD respects permissions on the parent\n> directories up to root.  E.g. after \"chmod 711 /home\" normal users would\n> get EACCES when they'd call getcwd(3) in their homes there, while e.g.\n> on Linux and FreeBSD they'd successfully get their current working dir.\n\nGood summary. As openbsd-tech points out it seems the bug is ours, if\nI'm understanding this correctly:\nhttp://marc.info/?l=openbsd-tech&m=149625768825173&w=2\n\n    [EACCES]\n        Search permission was denied for the current directory, or read or\n        search permission was denied for a directory above the current\n        directory in the file hierarchy.\n"},{"id":"325676","messageId":"cde6c1a5-3dfc-9710-bab9-b847a5dd960b@web.de","threadId":"46522","inReplyTo":"56fd58d6-acc7-042a-1b68-9854ed54361b@web.de","subject":"[PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-07T11:04:18Z","receivedAt":"2017-08-07T11:04:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The sub-test \"init in long base path\" in t0001 checks the ability to\nhandle long base paths with restrictive permissions (--x).  On OpenBSD\ngetcwd(3) fails in that case even for short paths.  Check the two\naspects separately by trying to use a long base path both with and\nwithout execute-only permissions.  Only attempt the former if we know\nthat getcwd(3) doesn't care.\n\nOriginal-patch-by: David Coppa <dcoppa@openbsd.org>\nReported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n t/t0001-init.sh | 30 ++++++++++++++++++++++++++++--\n 1 file changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex c4814d248f..86c1a51654 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -315,18 +315,44 @@ test_expect_success 'init with separate gitdir' '\n \ttest_path_is_dir realgitdir/refs\n '\n \n-test_expect_success 'init in long base path' '\n+test_lazy_prereq GETCWD_IGNORES_PERMS '\n+\tbase=GETCWD_TEST_BASE_DIR &&\n+\tmkdir -p $base/dir &&\n+\tchmod 100 $base ||\n+\terror \"bug in test script: cannot prepare $base\"\n+\n+\t(cd $base/dir && /bin/pwd -P)\n+\tstatus=$?\n+\n+\tchmod 700 $base &&\n+\trm -rf $base ||\n+\terror \"bug in test script: cannot clean $base\"\n+\treturn $status\n+'\n+\n+check_long_base_path () {\n \t# exceed initial buffer size of strbuf_getcwd()\n \tcomponent=123456789abcdef &&\n \ttest_when_finished \"chmod 0700 $component; rm -rf $component\" &&\n \tp31=$component/$component &&\n \tp127=$p31/$p31/$p31/$p31 &&\n \tmkdir -p $p127 &&\n-\tchmod 0111 $component &&\n+\tif test $# = 1\n+\tthen\n+\t\tchmod $1 $component\n+\tfi &&\n \t(\n \t\tcd $p127 &&\n \t\tgit init newdir\n \t)\n+}\n+\n+test_expect_success 'init in long base path' '\n+\tcheck_long_base_path\n+'\n+\n+test_expect_success GETCWD_IGNORES_PERMS 'init in long restricted base path' '\n+\tcheck_long_base_path 0111\n '\n \n test_expect_success 're-init on .git file' '\n-- \n2.14.0\n"},{"id":"325704","messageId":"xmqqk22fqo6g.fsf@gitster.mtv.corp.google.com","threadId":"46522","inReplyTo":"20170806233850.14711-1-avarab@gmail.com","subject":"Re: [PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T17:32:07Z","receivedAt":"2017-08-07T17:32:22Z","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> Change an argument to test_line_count (which'll ultimately be turned\n> into a \"test\" expression) to use \"-gt\" instead of \">\" for an\n> arithmetic test.\n>\n> This broken on e.g. OpenBSD as of v2.13.0 with my commit\n> ac3f5a3468 (\"ref-filter: add --no-contains option to\n> tag/branch/for-each-ref\", 2017-03-24).\n>\n> Upstream just worked around it by patching git and didn't tell us\n\nI'm tempted to do s/Upstream/Downstream/ while queuing, but please\ntell me I am confused.\n\n> about it, I discovered this when reading various Git packaging\n> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>\n> David, it would be great to get a quick bug report to\n> git@vger.kernel.org if you end up having to monkeypatch something\n> we've done. We won't bite, promise :)\n>\n> As shown in that linked Github commit OpenBSD has another recent\n> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a\n> related test, maybe René can make more sense of that?\n>\n> There's more patches in their ports which indicate possible bugs of\n> ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/\n>\n>  t/t7004-tag.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 0ef7b94394..0e2e57aa3d 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1887,7 +1887,7 @@ EOF\"\n>  \trun_with_limited_stack git tag --contains HEAD >actual &&\n>  \ttest_cmp expect actual &&\n>  \trun_with_limited_stack git tag --no-contains HEAD >actual &&\n> -\ttest_line_count \">\" 10 actual\n> +\ttest_line_count \"-gt\" 10 actual\n>  '\n>  \n>  test_expect_success '--format should list tags as per format given' '\n"},{"id":"325709","messageId":"87h8xjfe1n.fsf@linux-m68k.org","threadId":"46522","inReplyTo":"20170806233850.14711-1-avarab@gmail.com","subject":"Re: [PATCH] tests: don't give unportable \">\" to \"test\" built-in, use -gt","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-08-07T18:06:28Z","receivedAt":"2017-08-07T18:06:43Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Aug 06 2017, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 0ef7b94394..0e2e57aa3d 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1887,7 +1887,7 @@ EOF\"\n>  \trun_with_limited_stack git tag --contains HEAD >actual &&\n>  \ttest_cmp expect actual &&\n>  \trun_with_limited_stack git tag --no-contains HEAD >actual &&\n> -\ttest_line_count \">\" 10 actual\n> +\ttest_line_count \"-gt\" 10 actual\n\nMaybe also remove the quotes, they are no longer needed.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"426088","messageId":"patch-1.1-c70791bbd3-20210601T003743Z-avarab@gmail.com","threadId":"46522","inReplyTo":"cde6c1a5-3dfc-9710-bab9-b847a5dd960b@web.de","subject":"[PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-01T00:38:48Z","receivedAt":"2021-06-01T00:38:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"With a54e938e5b (strbuf: support long paths w/o read rights in\nstrbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems\nlike OpenBSD and AIX whose getcwd(3) has standard (but not like glibc\net al) behavior.\n\nThis was partially fixed in bed67874e2 (t0001: skip test with\nrestrictive permissions if getpwd(3) respects them, 2017-08-07).\n\nThe problem with that fix is that while its analysis of the problem is\ncorrect, it doesn't actually call getcwd(3), instead it invokes \"pwd\n-P\". There is no guarantee that \"pwd -P\" is actually going to call\ngetcwd(3), as opposed to e.g. being a shell built-in.\n\nOn AIX under both bash and ksh this test breaks because \"pwd -P\" will\nhappily display the current working directory, but getcwd(3) called by\nthe \"git init\" we're testing here will fail to get it.\n\nI checked whether clobbering the $PWD environment variable would\naffect it, and it didn't. Presumably these shells keep track of their\nworking directory internally.\n\nLet's change the test to a new \"test-tool getcwd\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nNot an issue new in v2.32.0, but an easy enough fix, so I thought I'd\nsend it now anyway in case we'd like these various AIX fixes in one\nbatch...\n\n Makefile               |  1 +\n t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++\n t/helper/test-tool.c   |  1 +\n t/helper/test-tool.h   |  1 +\n t/t0001-init.sh        |  5 ++++-\n 5 files changed, 33 insertions(+), 1 deletion(-)\n create mode 100644 t/helper/test-getcwd.c\n\ndiff --git a/Makefile b/Makefile\nindex c3565fc0f8..8c000ba48b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o\n TEST_BUILTINS_OBJS += test-fast-rebase.o\n TEST_BUILTINS_OBJS += test-genrandom.o\n TEST_BUILTINS_OBJS += test-genzeros.o\n+TEST_BUILTINS_OBJS += test-getcwd.o\n TEST_BUILTINS_OBJS += test-hash-speed.o\n TEST_BUILTINS_OBJS += test-hash.o\n TEST_BUILTINS_OBJS += test-hashmap.o\ndiff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c\nnew file mode 100644\nindex 0000000000..d680038a78\n--- /dev/null\n+++ b/t/helper/test-getcwd.c\n@@ -0,0 +1,26 @@\n+#include \"test-tool.h\"\n+#include \"git-compat-util.h\"\n+#include \"parse-options.h\"\n+\n+static const char *getcwd_usage[] = {\n+\t\"test-tool getcwd\",\n+\tNULL\n+};\n+\n+int cmd__getcwd(int argc, const char **argv)\n+{\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tchar *cwd;\n+\n+\targc = parse_options(argc, argv, \"test-tools\", options, getcwd_usage, 0);\n+\tif (argc > 0)\n+\t\tusage_with_options(getcwd_usage, options);\n+\n+\tcwd = xgetcwd();\n+\tputs(cwd);\n+\tfree(cwd);\n+\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex c5bd0c6d4c..3c13cb19b5 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {\n \t{ \"fast-rebase\", cmd__fast_rebase },\n \t{ \"genrandom\", cmd__genrandom },\n \t{ \"genzeros\", cmd__genzeros },\n+\t{ \"getcwd\", cmd__getcwd },\n \t{ \"hashmap\", cmd__hashmap },\n \t{ \"hash-speed\", cmd__hash_speed },\n \t{ \"index-version\", cmd__index_version },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex e8069a3b22..e691a4d9e9 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);\n int cmd__fast_rebase(int argc, const char **argv);\n int cmd__genrandom(int argc, const char **argv);\n int cmd__genzeros(int argc, const char **argv);\n+int cmd__getcwd(int argc, const char **argv);\n int cmd__hashmap(int argc, const char **argv);\n int cmd__hash_speed(int argc, const char **argv);\n int cmd__index_version(int argc, const char **argv);\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex acd662e403..df544bb321 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n \tchmod 100 $base ||\n \tBUG \"cannot prepare $base\"\n \n-\t(cd $base/dir && /bin/pwd -P)\n+\t(\n+\t\tcd $base/dir &&\n+\t\ttest-tool getcwd\n+\t)\n \tstatus=$?\n \n \tchmod 700 $base &&\n-- \n2.32.0.rc1.460.g26a014da44c\n\n"},{"id":"426147","messageId":"b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de","threadId":"46522","inReplyTo":"patch-1.1-c70791bbd3-20210601T003743Z-avarab@gmail.com","subject":"Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-06-01T16:15:39Z","receivedAt":"2021-06-01T16:15:56Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:\n> With a54e938e5b (strbuf: support long paths w/o read rights in\n> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems\n> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc\n> et al) behavior.\n>\n> This was partially fixed in bed67874e2 (t0001: skip test with\n> restrictive permissions if getpwd(3) respects them, 2017-08-07).\n>\n> The problem with that fix is that while its analysis of the problem is\n> correct, it doesn't actually call getcwd(3), instead it invokes \"pwd\n> -P\". There is no guarantee that \"pwd -P\" is actually going to call\n> getcwd(3), as opposed to e.g. being a shell built-in.\n>\n> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n> happily display the current working directory, but getcwd(3) called by\n> the \"git init\" we're testing here will fail to get it.\n>\n> I checked whether clobbering the $PWD environment variable would\n> affect it, and it didn't. Presumably these shells keep track of their\n> working directory internally.\n>\n> Let's change the test to a new \"test-tool getcwd\".\n\nMakes sense.\n\nIf /bin/pwd can figure out the path to the current working directory\nwithout read permissions to parent directories then it might be possible\nto teach strbuf_getcwd() the same trick, though.  How does it do it?\n\nPerhaps it falls back to $PWD; POSIX says the behavior of pwd is\nunspecified if that variable would be changed, so a compliant\nimplementation would be allowed to do that.  I think that way is not\ninteresting for strbuf_getcwd(), though, because if we trust that\nvariable then we can read it directly instead.  It gets stale if any\nparent directory is renamed.  E.g. the following commands would print a\nstring ending in \"stale\":\n\n\tmkdir stale\n\tcd stale\n\tmv ../stale ../fresh\n\tchmod 111 ../fresh\n\t/bin/pwd -P\n\nPerhaps it asks the kernel, like getcwd() does on FreeBSD.  It would\nbe a bit weird to expose this functionality in a command line tool, but\nnot in the library function, so this is unlikely.  You seem to say that\n/bin/pwd is a shell builtin on your system, which is also weird, though.\nThe commands above would print a string ending in \"fresh\" with the\nsyscall method.\n\nAn evil way would be to temporarily add read permission to all parent\ndirectories.  It would also print a string ending in \"fresh\".  You'd\nprobably see chmod calls when running /bin/pwd using truss in that\ncase, and it would fail if chmod is not allowed.\n\nThat's all I can think of.\n\nIf strbuf_getcwd() were to learn any of these tricks, then so would\n\"test-tool getcwd\", via its xgetcwd() call.  At that point we'd better\nrename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.\n\nBut I guess we need none of that because we never got a request from\nan AIX user to support a /home directory without read permissions,\nright?\n\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>\n> Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd\n> send it now anyway in case we'd like these various AIX fixes in one\n> batch...\n>\n>  Makefile               |  1 +\n>  t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++\n>  t/helper/test-tool.c   |  1 +\n>  t/helper/test-tool.h   |  1 +\n>  t/t0001-init.sh        |  5 ++++-\n>  5 files changed, 33 insertions(+), 1 deletion(-)\n>  create mode 100644 t/helper/test-getcwd.c\n>\n> diff --git a/Makefile b/Makefile\n> index c3565fc0f8..8c000ba48b 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o\n>  TEST_BUILTINS_OBJS += test-fast-rebase.o\n>  TEST_BUILTINS_OBJS += test-genrandom.o\n>  TEST_BUILTINS_OBJS += test-genzeros.o\n> +TEST_BUILTINS_OBJS += test-getcwd.o\n>  TEST_BUILTINS_OBJS += test-hash-speed.o\n>  TEST_BUILTINS_OBJS += test-hash.o\n>  TEST_BUILTINS_OBJS += test-hashmap.o\n> diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c\n> new file mode 100644\n> index 0000000000..d680038a78\n> --- /dev/null\n> +++ b/t/helper/test-getcwd.c\n> @@ -0,0 +1,26 @@\n> +#include \"test-tool.h\"\n> +#include \"git-compat-util.h\"\n> +#include \"parse-options.h\"\n> +\n> +static const char *getcwd_usage[] = {\n> +\t\"test-tool getcwd\",\n> +\tNULL\n> +};\n> +\n> +int cmd__getcwd(int argc, const char **argv)\n> +{\n> +\tstruct option options[] = {\n> +\t\tOPT_END()\n> +\t};\n> +\tchar *cwd;\n> +\n> +\targc = parse_options(argc, argv, \"test-tools\", options, getcwd_usage, 0);\n> +\tif (argc > 0)\n> +\t\tusage_with_options(getcwd_usage, options);\n> +\n> +\tcwd = xgetcwd();\n> +\tputs(cwd);\n> +\tfree(cwd);\n> +\n> +\treturn 0;\n> +}\n> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\n> index c5bd0c6d4c..3c13cb19b5 100644\n> --- a/t/helper/test-tool.c\n> +++ b/t/helper/test-tool.c\n> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {\n>  \t{ \"fast-rebase\", cmd__fast_rebase },\n>  \t{ \"genrandom\", cmd__genrandom },\n>  \t{ \"genzeros\", cmd__genzeros },\n> +\t{ \"getcwd\", cmd__getcwd },\n>  \t{ \"hashmap\", cmd__hashmap },\n>  \t{ \"hash-speed\", cmd__hash_speed },\n>  \t{ \"index-version\", cmd__index_version },\n> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n> index e8069a3b22..e691a4d9e9 100644\n> --- a/t/helper/test-tool.h\n> +++ b/t/helper/test-tool.h\n> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);\n>  int cmd__fast_rebase(int argc, const char **argv);\n>  int cmd__genrandom(int argc, const char **argv);\n>  int cmd__genzeros(int argc, const char **argv);\n> +int cmd__getcwd(int argc, const char **argv);\n>  int cmd__hashmap(int argc, const char **argv);\n>  int cmd__hash_speed(int argc, const char **argv);\n>  int cmd__index_version(int argc, const char **argv);\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index acd662e403..df544bb321 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n>  \tchmod 100 $base ||\n>  \tBUG \"cannot prepare $base\"\n>\n> -\t(cd $base/dir && /bin/pwd -P)\n> +\t(\n> +\t\tcd $base/dir &&\n> +\t\ttest-tool getcwd\n> +\t)\n>  \tstatus=$?\n>\n>  \tchmod 700 $base &&\n>\n"},{"id":"426163","messageId":"xmqq4kehs2nt.fsf@gitster.g","threadId":"46522","inReplyTo":"b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de","subject":"Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-01T21:20:06Z","receivedAt":"2021-06-01T21:20:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n>> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n>> happily display the current working directory, but getcwd(3) called by\n>> the \"git init\" we're testing here will fail to get it.\n>>\n>> I checked whether clobbering the $PWD environment variable would\n>> affect it, and it didn't. Presumably these shells keep track of their\n>> working directory internally.\n>>\n>> Let's change the test to a new \"test-tool getcwd\".\n>\n> Makes sense.\n>\n> If /bin/pwd can figure out the path to the current working directory\n> without read permissions to parent directories then it might be possible\n> to teach strbuf_getcwd() the same trick, though.  How does it do it?\n> ...\n> If strbuf_getcwd() were to learn any of these tricks, then so would\n> \"test-tool getcwd\", via its xgetcwd() call.  At that point we'd better\n> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.\n>\n> But I guess we need none of that because we never got a request from\n> an AIX user to support a /home directory without read permissions,\n> right?\n\nNice \"thinking aloud\".\n"},{"id":"426574","messageId":"871r9d6hhy.fsf@evledraar.gmail.com","threadId":"46522","inReplyTo":"b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de","subject":"Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-07T11:24:23Z","receivedAt":"2021-06-07T11:28:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jun 01 2021, René Scharfe wrote:\n\n> Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:\n>> With a54e938e5b (strbuf: support long paths w/o read rights in\n>> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems\n>> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc\n>> et al) behavior.\n>>\n>> This was partially fixed in bed67874e2 (t0001: skip test with\n>> restrictive permissions if getpwd(3) respects them, 2017-08-07).\n>>\n>> The problem with that fix is that while its analysis of the problem is\n>> correct, it doesn't actually call getcwd(3), instead it invokes \"pwd\n>> -P\". There is no guarantee that \"pwd -P\" is actually going to call\n>> getcwd(3), as opposed to e.g. being a shell built-in.\n>>\n>> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n>> happily display the current working directory, but getcwd(3) called by\n>> the \"git init\" we're testing here will fail to get it.\n>>\n>> I checked whether clobbering the $PWD environment variable would\n>> affect it, and it didn't. Presumably these shells keep track of their\n>> working directory internally.\n>>\n>> Let's change the test to a new \"test-tool getcwd\".\n>\n> Makes sense.\n>\n> If /bin/pwd can figure out the path to the current working directory\n> without read permissions to parent directories then it might be possible\n> to teach strbuf_getcwd() the same trick, though.  How does it do it?\n>\n> Perhaps it falls back to $PWD; POSIX says the behavior of pwd is\n> unspecified if that variable would be changed, so a compliant\n> implementation would be allowed to do that.  I think that way is not\n> interesting for strbuf_getcwd(), though, because if we trust that\n> variable then we can read it directly instead.  It gets stale if any\n> parent directory is renamed.  E.g. the following commands would print a\n> string ending in \"stale\":\n>\n> \tmkdir stale\n> \tcd stale\n> \tmv ../stale ../fresh\n> \tchmod 111 ../fresh\n> \t/bin/pwd -P\n\nYes, AIX prints \"stale\" here, but e.g. my Linux box prints \"fresh\".\n\n> Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would\n> be a bit weird to expose this functionality in a command line tool, but\n> not in the library function, so this is unlikely.  You seem to say that\n> /bin/pwd is a shell builtin on your system, which is also weird, though.\n> The commands above would print a string ending in \"fresh\" with the\n> syscall method.\n>\n> An evil way would be to temporarily add read permission to all parent\n> directories.  It would also print a string ending in \"fresh\".  You'd\n> probably see chmod calls when running /bin/pwd using truss in that\n> case, and it would fail if chmod is not allowed.\n>\n> That's all I can think of.\n>\n> If strbuf_getcwd() were to learn any of these tricks, then so would\n> \"test-tool getcwd\", via its xgetcwd() call.  At that point we'd better\n> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.\n>\n> But I guess we need none of that because we never got a request from\n> an AIX user to support a /home directory without read permissions,\n> right?\n\nI don't really see the point of trying that hard. Yes, we could make\nsome forward progress if we bent over backwards and got the current\nworking directory, but what would we be left with? A git repository the\nuser can't \"ls\" inside of.\n\nSo any number of other thing after that now-working xgetcwd() call would\nfail, we couldn't list any files in the working tree or .git directory.\n\nWhy not just fix the bug in there being disconnect between pwd and\ngetcwd() here and move on?\n\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>> ---\n>>\n>> Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd\n>> send it now anyway in case we'd like these various AIX fixes in one\n>> batch...\n>>\n>>  Makefile               |  1 +\n>>  t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++\n>>  t/helper/test-tool.c   |  1 +\n>>  t/helper/test-tool.h   |  1 +\n>>  t/t0001-init.sh        |  5 ++++-\n>>  5 files changed, 33 insertions(+), 1 deletion(-)\n>>  create mode 100644 t/helper/test-getcwd.c\n>>\n>> diff --git a/Makefile b/Makefile\n>> index c3565fc0f8..8c000ba48b 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o\n>>  TEST_BUILTINS_OBJS += test-fast-rebase.o\n>>  TEST_BUILTINS_OBJS += test-genrandom.o\n>>  TEST_BUILTINS_OBJS += test-genzeros.o\n>> +TEST_BUILTINS_OBJS += test-getcwd.o\n>>  TEST_BUILTINS_OBJS += test-hash-speed.o\n>>  TEST_BUILTINS_OBJS += test-hash.o\n>>  TEST_BUILTINS_OBJS += test-hashmap.o\n>> diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c\n>> new file mode 100644\n>> index 0000000000..d680038a78\n>> --- /dev/null\n>> +++ b/t/helper/test-getcwd.c\n>> @@ -0,0 +1,26 @@\n>> +#include \"test-tool.h\"\n>> +#include \"git-compat-util.h\"\n>> +#include \"parse-options.h\"\n>> +\n>> +static const char *getcwd_usage[] = {\n>> +\t\"test-tool getcwd\",\n>> +\tNULL\n>> +};\n>> +\n>> +int cmd__getcwd(int argc, const char **argv)\n>> +{\n>> +\tstruct option options[] = {\n>> +\t\tOPT_END()\n>> +\t};\n>> +\tchar *cwd;\n>> +\n>> +\targc = parse_options(argc, argv, \"test-tools\", options, getcwd_usage, 0);\n>> +\tif (argc > 0)\n>> +\t\tusage_with_options(getcwd_usage, options);\n>> +\n>> +\tcwd = xgetcwd();\n>> +\tputs(cwd);\n>> +\tfree(cwd);\n>> +\n>> +\treturn 0;\n>> +}\n>> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\n>> index c5bd0c6d4c..3c13cb19b5 100644\n>> --- a/t/helper/test-tool.c\n>> +++ b/t/helper/test-tool.c\n>> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {\n>>  \t{ \"fast-rebase\", cmd__fast_rebase },\n>>  \t{ \"genrandom\", cmd__genrandom },\n>>  \t{ \"genzeros\", cmd__genzeros },\n>> +\t{ \"getcwd\", cmd__getcwd },\n>>  \t{ \"hashmap\", cmd__hashmap },\n>>  \t{ \"hash-speed\", cmd__hash_speed },\n>>  \t{ \"index-version\", cmd__index_version },\n>> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n>> index e8069a3b22..e691a4d9e9 100644\n>> --- a/t/helper/test-tool.h\n>> +++ b/t/helper/test-tool.h\n>> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);\n>>  int cmd__fast_rebase(int argc, const char **argv);\n>>  int cmd__genrandom(int argc, const char **argv);\n>>  int cmd__genzeros(int argc, const char **argv);\n>> +int cmd__getcwd(int argc, const char **argv);\n>>  int cmd__hashmap(int argc, const char **argv);\n>>  int cmd__hash_speed(int argc, const char **argv);\n>>  int cmd__index_version(int argc, const char **argv);\n>> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n>> index acd662e403..df544bb321 100755\n>> --- a/t/t0001-init.sh\n>> +++ b/t/t0001-init.sh\n>> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n>>  \tchmod 100 $base ||\n>>  \tBUG \"cannot prepare $base\"\n>>\n>> -\t(cd $base/dir && /bin/pwd -P)\n>> +\t(\n>> +\t\tcd $base/dir &&\n>> +\t\ttest-tool getcwd\n>> +\t)\n>>  \tstatus=$?\n>>\n>>  \tchmod 700 $base &&\n>>\n\n"},{"id":"426575","messageId":"87y2bl52v4.fsf@evledraar.gmail.com","threadId":"46522","inReplyTo":"xmqq4kehs2nt.fsf@gitster.g","subject":"Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-07T11:29:18Z","receivedAt":"2021-06-07T11:31:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jun 02 2021, Junio C Hamano wrote:\n\n> René Scharfe <l.s.r@web.de> writes:\n>\n>>> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n>>> happily display the current working directory, but getcwd(3) called by\n>>> the \"git init\" we're testing here will fail to get it.\n>>>\n>>> I checked whether clobbering the $PWD environment variable would\n>>> affect it, and it didn't. Presumably these shells keep track of their\n>>> working directory internally.\n>>>\n>>> Let's change the test to a new \"test-tool getcwd\".\n>>\n>> Makes sense.\n>>\n>> If /bin/pwd can figure out the path to the current working directory\n>> without read permissions to parent directories then it might be possible\n>> to teach strbuf_getcwd() the same trick, though.  How does it do it?\n>> ...\n>> If strbuf_getcwd() were to learn any of these tricks, then so would\n>> \"test-tool getcwd\", via its xgetcwd() call.  At that point we'd better\n>> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.\n>>\n>> But I guess we need none of that because we never got a request from\n>> an AIX user to support a /home directory without read permissions,\n>> right?\n>\n> Nice \"thinking aloud\".\n\nI see (well, rc/release period and all) that you didn't pick this up. I\nthink per my just-sent\nhttp://lore.kernel.org/git/871r9d6hhy.fsf@evledraar.gmail.com that it\nmakes sense to just leave this at s/pwd(1)/getcwd(3)/ as this patch\ndoes.\n"},{"id":"426613","messageId":"57c2f4dc-06b9-b330-a01c-d2ae0e86df7c@web.de","threadId":"46522","inReplyTo":"871r9d6hhy.fsf@evledraar.gmail.com","subject":"Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-06-07T15:26:05Z","receivedAt":"2021-06-07T15:26:19Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.06.21 um 13:24 schrieb Ævar Arnfjörð Bjarmason:\n>\n> On Tue, Jun 01 2021, René Scharfe wrote:\n>\n>> Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:\n>>> With a54e938e5b (strbuf: support long paths w/o read rights in\n>>> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems\n>>> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc\n>>> et al) behavior.\n>>>\n>>> This was partially fixed in bed67874e2 (t0001: skip test with\n>>> restrictive permissions if getpwd(3) respects them, 2017-08-07).\n>>>\n>>> The problem with that fix is that while its analysis of the problem is\n>>> correct, it doesn't actually call getcwd(3), instead it invokes \"pwd\n>>> -P\". There is no guarantee that \"pwd -P\" is actually going to call\n>>> getcwd(3), as opposed to e.g. being a shell built-in.\n>>>\n>>> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n>>> happily display the current working directory, but getcwd(3) called by\n>>> the \"git init\" we're testing here will fail to get it.\n>>>\n>>> I checked whether clobbering the $PWD environment variable would\n>>> affect it, and it didn't. Presumably these shells keep track of their\n>>> working directory internally.\n>>>\n>>> Let's change the test to a new \"test-tool getcwd\".\n>>\n>> Makes sense.\n>>\n>> If /bin/pwd can figure out the path to the current working directory\n>> without read permissions to parent directories then it might be possible\n>> to teach strbuf_getcwd() the same trick, though.  How does it do it?\n>>\n>> Perhaps it falls back to $PWD; POSIX says the behavior of pwd is\n>> unspecified if that variable would be changed, so a compliant\n>> implementation would be allowed to do that.  I think that way is not\n>> interesting for strbuf_getcwd(), though, because if we trust that\n>> variable then we can read it directly instead.  It gets stale if any\n>> parent directory is renamed.  E.g. the following commands would print a\n>> string ending in \"stale\":\n>>\n>> \tmkdir stale\n>> \tcd stale\n>> \tmv ../stale ../fresh\n>> \tchmod 111 ../fresh\n>> \t/bin/pwd -P\n>\n> Yes, AIX prints \"stale\" here, but e.g. my Linux box prints \"fresh\".\n\nOK, thanks for checking.  I find it weird: Why would they add a command\nthat basically prints $PWD when callers can easily access this variable\ndirectly?  Anyway, it is what it is.\n\n>> Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would\n>> be a bit weird to expose this functionality in a command line tool, but\n>> not in the library function, so this is unlikely.  You seem to say that\n>> /bin/pwd is a shell builtin on your system, which is also weird, though.\n>> The commands above would print a string ending in \"fresh\" with the\n>> syscall method.\n>>\n>> An evil way would be to temporarily add read permission to all parent\n>> directories.  It would also print a string ending in \"fresh\".  You'd\n>> probably see chmod calls when running /bin/pwd using truss in that\n>> case, and it would fail if chmod is not allowed.\n>>\n>> That's all I can think of.\n>>\n>> If strbuf_getcwd() were to learn any of these tricks, then so would\n>> \"test-tool getcwd\", via its xgetcwd() call.  At that point we'd better\n>> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.\n>>\n>> But I guess we need none of that because we never got a request from\n>> an AIX user to support a /home directory without read permissions,\n>> right?\n>\n> I don't really see the point of trying that hard. Yes, we could make\n> some forward progress if we bent over backwards and got the current\n> working directory, but what would we be left with? A git repository the\n> user can't \"ls\" inside of.\n\nThe reason would be support for execute-only (e.g. 0711) /home, which\nsome systems have for privacy reasons.\n\n> So any number of other thing after that now-working xgetcwd() call would\n> fail, we couldn't list any files in the working tree or .git directory.\n\nUsers own their /home/directory in that scenario and have full\npermissions in their repositories.  They cannot verify the name of their\n/home/directory using readdir(), though.\n\n> Why not just fix the bug in there being disconnect between pwd and\n> getcwd() here and move on?\n\nIf you mean the false assumption that /bin/pwd uses getcwd(): I still\nthink that your patch makes sense, as I wrote in my first reply,\nimplying that I agree it should be applied.\n\n>\n>>>\n>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>>> ---\n>>>\n>>> Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd\n>>> send it now anyway in case we'd like these various AIX fixes in one\n>>> batch...\n>>>\n>>>  Makefile               |  1 +\n>>>  t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++\n>>>  t/helper/test-tool.c   |  1 +\n>>>  t/helper/test-tool.h   |  1 +\n>>>  t/t0001-init.sh        |  5 ++++-\n>>>  5 files changed, 33 insertions(+), 1 deletion(-)\n>>>  create mode 100644 t/helper/test-getcwd.c\n>>>\n>>> diff --git a/Makefile b/Makefile\n>>> index c3565fc0f8..8c000ba48b 100644\n>>> --- a/Makefile\n>>> +++ b/Makefile\n>>> @@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o\n>>>  TEST_BUILTINS_OBJS += test-fast-rebase.o\n>>>  TEST_BUILTINS_OBJS += test-genrandom.o\n>>>  TEST_BUILTINS_OBJS += test-genzeros.o\n>>> +TEST_BUILTINS_OBJS += test-getcwd.o\n>>>  TEST_BUILTINS_OBJS += test-hash-speed.o\n>>>  TEST_BUILTINS_OBJS += test-hash.o\n>>>  TEST_BUILTINS_OBJS += test-hashmap.o\n>>> diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c\n>>> new file mode 100644\n>>> index 0000000000..d680038a78\n>>> --- /dev/null\n>>> +++ b/t/helper/test-getcwd.c\n>>> @@ -0,0 +1,26 @@\n>>> +#include \"test-tool.h\"\n>>> +#include \"git-compat-util.h\"\n>>> +#include \"parse-options.h\"\n>>> +\n>>> +static const char *getcwd_usage[] = {\n>>> +\t\"test-tool getcwd\",\n>>> +\tNULL\n>>> +};\n>>> +\n>>> +int cmd__getcwd(int argc, const char **argv)\n>>> +{\n>>> +\tstruct option options[] = {\n>>> +\t\tOPT_END()\n>>> +\t};\n>>> +\tchar *cwd;\n>>> +\n>>> +\targc = parse_options(argc, argv, \"test-tools\", options, getcwd_usage, 0);\n>>> +\tif (argc > 0)\n>>> +\t\tusage_with_options(getcwd_usage, options);\n>>> +\n>>> +\tcwd = xgetcwd();\n>>> +\tputs(cwd);\n>>> +\tfree(cwd);\n>>> +\n>>> +\treturn 0;\n>>> +}\n>>> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\n>>> index c5bd0c6d4c..3c13cb19b5 100644\n>>> --- a/t/helper/test-tool.c\n>>> +++ b/t/helper/test-tool.c\n>>> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {\n>>>  \t{ \"fast-rebase\", cmd__fast_rebase },\n>>>  \t{ \"genrandom\", cmd__genrandom },\n>>>  \t{ \"genzeros\", cmd__genzeros },\n>>> +\t{ \"getcwd\", cmd__getcwd },\n>>>  \t{ \"hashmap\", cmd__hashmap },\n>>>  \t{ \"hash-speed\", cmd__hash_speed },\n>>>  \t{ \"index-version\", cmd__index_version },\n>>> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n>>> index e8069a3b22..e691a4d9e9 100644\n>>> --- a/t/helper/test-tool.h\n>>> +++ b/t/helper/test-tool.h\n>>> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);\n>>>  int cmd__fast_rebase(int argc, const char **argv);\n>>>  int cmd__genrandom(int argc, const char **argv);\n>>>  int cmd__genzeros(int argc, const char **argv);\n>>> +int cmd__getcwd(int argc, const char **argv);\n>>>  int cmd__hashmap(int argc, const char **argv);\n>>>  int cmd__hash_speed(int argc, const char **argv);\n>>>  int cmd__index_version(int argc, const char **argv);\n>>> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n>>> index acd662e403..df544bb321 100755\n>>> --- a/t/t0001-init.sh\n>>> +++ b/t/t0001-init.sh\n>>> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n>>>  \tchmod 100 $base ||\n>>>  \tBUG \"cannot prepare $base\"\n>>>\n>>> -\t(cd $base/dir && /bin/pwd -P)\n>>> +\t(\n>>> +\t\tcd $base/dir &&\n>>> +\t\ttest-tool getcwd\n>>> +\t)\n>>>  \tstatus=$?\n>>>\n>>>  \tchmod 700 $base &&\n>>>\n>\n"},{"id":"426618","messageId":"87pmwx4r94.fsf@evledraar.gmail.com","threadId":"46522","inReplyTo":"57c2f4dc-06b9-b330-a01c-d2ae0e86df7c@web.de","subject":"Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-07T15:38:07Z","receivedAt":"2021-06-07T15:41:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 07 2021, René Scharfe wrote:\n\n> Am 07.06.21 um 13:24 schrieb Ævar Arnfjörð Bjarmason:\n>>\n>> On Tue, Jun 01 2021, René Scharfe wrote:\n>>\n>>> Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:\n>>>> With a54e938e5b (strbuf: support long paths w/o read rights in\n>>>> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems\n>>>> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc\n>>>> et al) behavior.\n>>>>\n>>>> This was partially fixed in bed67874e2 (t0001: skip test with\n>>>> restrictive permissions if getpwd(3) respects them, 2017-08-07).\n>>>>\n>>>> The problem with that fix is that while its analysis of the problem is\n>>>> correct, it doesn't actually call getcwd(3), instead it invokes \"pwd\n>>>> -P\". There is no guarantee that \"pwd -P\" is actually going to call\n>>>> getcwd(3), as opposed to e.g. being a shell built-in.\n>>>>\n>>>> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n>>>> happily display the current working directory, but getcwd(3) called by\n>>>> the \"git init\" we're testing here will fail to get it.\n>>>>\n>>>> I checked whether clobbering the $PWD environment variable would\n>>>> affect it, and it didn't. Presumably these shells keep track of their\n>>>> working directory internally.\n>>>>\n>>>> Let's change the test to a new \"test-tool getcwd\".\n>>>\n>>> Makes sense.\n>>>\n>>> If /bin/pwd can figure out the path to the current working directory\n>>> without read permissions to parent directories then it might be possible\n>>> to teach strbuf_getcwd() the same trick, though.  How does it do it?\n>>>\n>>> Perhaps it falls back to $PWD; POSIX says the behavior of pwd is\n>>> unspecified if that variable would be changed, so a compliant\n>>> implementation would be allowed to do that.  I think that way is not\n>>> interesting for strbuf_getcwd(), though, because if we trust that\n>>> variable then we can read it directly instead.  It gets stale if any\n>>> parent directory is renamed.  E.g. the following commands would print a\n>>> string ending in \"stale\":\n>>>\n>>> \tmkdir stale\n>>> \tcd stale\n>>> \tmv ../stale ../fresh\n>>> \tchmod 111 ../fresh\n>>> \t/bin/pwd -P\n>>\n>> Yes, AIX prints \"stale\" here, but e.g. my Linux box prints \"fresh\".\n>\n> OK, thanks for checking.  I find it weird: Why would they add a command\n> that basically prints $PWD when callers can easily access this variable\n> directly?  Anyway, it is what it is.\n\nNot so much them but POSIX sayeth:\n\n    If an application sets or unsets the value of PWD , the behavior of\n    pwd is unspecified.\n    -- https://pubs.opengroup.org/onlinepubs/007904875/utilities/pwd.html\n\nAnd you can't be POSIX-compatible without a pwd(1) command. Ergo a\nsystem like AIX needs a pwd utility, whether it'll return the same thing\nas \"$PWD\" in some scenarios or not.\n\nBy they way: I don't know how AIX implements pwd(1), and whether it's\npurely redundant or whatever in this case.\n\n>>> Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would\n>>> be a bit weird to expose this functionality in a command line tool, but\n>>> not in the library function, so this is unlikely.  You seem to say that\n>>> /bin/pwd is a shell builtin on your system, which is also weird, though.\n>>> The commands above would print a string ending in \"fresh\" with the\n>>> syscall method.\n>>>\n>>> An evil way would be to temporarily add read permission to all parent\n>>> directories.  It would also print a string ending in \"fresh\".  You'd\n>>> probably see chmod calls when running /bin/pwd using truss in that\n>>> case, and it would fail if chmod is not allowed.\n>>>\n>>> That's all I can think of.\n>>>\n>>> If strbuf_getcwd() were to learn any of these tricks, then so would\n>>> \"test-tool getcwd\", via its xgetcwd() call.  At that point we'd better\n>>> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.\n>>>\n>>> But I guess we need none of that because we never got a request from\n>>> an AIX user to support a /home directory without read permissions,\n>>> right?\n>>\n>> I don't really see the point of trying that hard. Yes, we could make\n>> some forward progress if we bent over backwards and got the current\n>> working directory, but what would we be left with? A git repository the\n>> user can't \"ls\" inside of.\n>\n> The reason would be support for execute-only (e.g. 0711) /home, which\n> some systems have for privacy reasons.\n>\n>> So any number of other thing after that now-working xgetcwd() call would\n>> fail, we couldn't list any files in the working tree or .git directory.\n>\n> Users own their /home/directory in that scenario and have full\n> permissions in their repositories.  They cannot verify the name of their\n> /home/directory using readdir(), though.\n\nAh, I think I'd earlier misunderstood the test-case and thought it was\nmerely about a \"git init xyz\" where xyz itself is in that state..\n\n>> Why not just fix the bug in there being disconnect between pwd and\n>> getcwd() here and move on?\n>\n> If you mean the false assumption that /bin/pwd uses getcwd(): I still\n> think that your patch makes sense, as I wrote in my first reply,\n> implying that I agree it should be applied.\n\nAh, thanks. I wasn't quite clear or that, i.e. whether the\nstrbuf_getcwd() was a suggestion of some follow-up work or that this fix\nshould take that alternate route. Thanks!\n"},{"id":"431582","messageId":"patch-v2-1.1-d7d071441b0-20210730T161540Z-avarab@gmail.com","threadId":"46522","inReplyTo":"patch-1.1-c70791bbd3-20210601T003743Z-avarab@gmail.com","subject":"[PATCH v2] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-30T16:18:14Z","receivedAt":"2021-07-30T16:18:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"With a54e938e5b (strbuf: support long paths w/o read rights in\nstrbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems\nlike OpenBSD and AIX whose getcwd(3) has standard (but not like glibc\net al) behavior.\n\nThis was partially fixed in bed67874e2 (t0001: skip test with\nrestrictive permissions if getpwd(3) respects them, 2017-08-07).\n\nThe problem with that fix is that while its analysis of the problem is\ncorrect, it doesn't actually call getcwd(3), instead it invokes \"pwd\n-P\". There is no guarantee that \"pwd -P\" is going to call getcwd(3),\nas opposed to e.g. being a shell built-in.\n\nOn AIX under both bash and ksh this test breaks because \"pwd -P\" will\nhappily display the current working directory, but getcwd(3) called by\nthe \"git init\" we're testing here will fail to get it.\n\nI checked whether clobbering the $PWD environment variable would\naffect it, and it didn't. Presumably these shells keep track of their\nworking directory internally.\n\nThere's possible follow-up work here in teaching strbuf_getcwd() to\nget the working directory with whatever method \"pwd\" uses on these\nplatforms. See [1] for a discussion of that, but let's take the easy\nway out here and just skip these tests by fixing the\nGETCWD_IGNORES_PERMS prerequisite to match the limitations of\nstrbuf_getcwd().\n\n1. https://lore.kernel.org/git/b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nJunio: I tried to get this in around the v2.32.0 release, see previous\nproddings at:\n\n  https://lore.kernel.org/git/871r9d6hhy.fsf@evledraar.gmail.com/\n  https://lore.kernel.org/git/87y2bl52v4.fsf@evledraar.gmail.com/\n\nHere's another try with a slightly updated commit message discussing\nthat there's a possible more perfect solution here (per René's reply),\nbut that this fixes the immediate bug on AIX/OpenBSD etc.\n\nRange-diff against v1:\n1:  c70791bbd3a ! 1:  d7d071441b0 t0001: fix broken not-quite getcwd(3) test in bed67874e2\n    @@ Commit message\n     \n         The problem with that fix is that while its analysis of the problem is\n         correct, it doesn't actually call getcwd(3), instead it invokes \"pwd\n    -    -P\". There is no guarantee that \"pwd -P\" is actually going to call\n    -    getcwd(3), as opposed to e.g. being a shell built-in.\n    +    -P\". There is no guarantee that \"pwd -P\" is going to call getcwd(3),\n    +    as opposed to e.g. being a shell built-in.\n     \n         On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n         happily display the current working directory, but getcwd(3) called by\n    @@ Commit message\n         affect it, and it didn't. Presumably these shells keep track of their\n         working directory internally.\n     \n    -    Let's change the test to a new \"test-tool getcwd\".\n    +    There's possible follow-up work here in teaching strbuf_getcwd() to\n    +    get the working directory with whatever method \"pwd\" uses on these\n    +    platforms. See [1] for a discussion of that, but let's take the easy\n    +    way out here and just skip these tests by fixing the\n    +    GETCWD_IGNORES_PERMS prerequisite to match the limitations of\n    +    strbuf_getcwd().\n    +\n    +    1. https://lore.kernel.org/git/b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de/\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n\n Makefile               |  1 +\n t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++\n t/helper/test-tool.c   |  1 +\n t/helper/test-tool.h   |  1 +\n t/t0001-init.sh        |  5 ++++-\n 5 files changed, 33 insertions(+), 1 deletion(-)\n create mode 100644 t/helper/test-getcwd.c\n\ndiff --git a/Makefile b/Makefile\nindex c6f6246bf63..9573190f1d7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -715,6 +715,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o\n TEST_BUILTINS_OBJS += test-fast-rebase.o\n TEST_BUILTINS_OBJS += test-genrandom.o\n TEST_BUILTINS_OBJS += test-genzeros.o\n+TEST_BUILTINS_OBJS += test-getcwd.o\n TEST_BUILTINS_OBJS += test-hash-speed.o\n TEST_BUILTINS_OBJS += test-hash.o\n TEST_BUILTINS_OBJS += test-hashmap.o\ndiff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c\nnew file mode 100644\nindex 00000000000..d680038a780\n--- /dev/null\n+++ b/t/helper/test-getcwd.c\n@@ -0,0 +1,26 @@\n+#include \"test-tool.h\"\n+#include \"git-compat-util.h\"\n+#include \"parse-options.h\"\n+\n+static const char *getcwd_usage[] = {\n+\t\"test-tool getcwd\",\n+\tNULL\n+};\n+\n+int cmd__getcwd(int argc, const char **argv)\n+{\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tchar *cwd;\n+\n+\targc = parse_options(argc, argv, \"test-tools\", options, getcwd_usage, 0);\n+\tif (argc > 0)\n+\t\tusage_with_options(getcwd_usage, options);\n+\n+\tcwd = xgetcwd();\n+\tputs(cwd);\n+\tfree(cwd);\n+\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 490ac026c51..3ce5585e53a 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {\n \t{ \"fast-rebase\", cmd__fast_rebase },\n \t{ \"genrandom\", cmd__genrandom },\n \t{ \"genzeros\", cmd__genzeros },\n+\t{ \"getcwd\", cmd__getcwd },\n \t{ \"hashmap\", cmd__hashmap },\n \t{ \"hash-speed\", cmd__hash_speed },\n \t{ \"index-version\", cmd__index_version },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex f8dc266721f..9f0f5228508 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);\n int cmd__fast_rebase(int argc, const char **argv);\n int cmd__genrandom(int argc, const char **argv);\n int cmd__genzeros(int argc, const char **argv);\n+int cmd__getcwd(int argc, const char **argv);\n int cmd__hashmap(int argc, const char **argv);\n int cmd__hash_speed(int argc, const char **argv);\n int cmd__index_version(int argc, const char **argv);\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex acd662e403b..df544bb321f 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n \tchmod 100 $base ||\n \tBUG \"cannot prepare $base\"\n \n-\t(cd $base/dir && /bin/pwd -P)\n+\t(\n+\t\tcd $base/dir &&\n+\t\ttest-tool getcwd\n+\t)\n \tstatus=$?\n \n \tchmod 700 $base &&\n-- \n2.32.0.1071.g36f34456314\n\n"},{"id":"431588","messageId":"xmqqmtq3203g.fsf@gitster.g","threadId":"46522","inReplyTo":"patch-v2-1.1-d7d071441b0-20210730T161540Z-avarab@gmail.com","subject":"Re: [PATCH v2] t0001: fix broken not-quite getcwd(3) test in bed67874e2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-30T17:16:51Z","receivedAt":"2021-07-30T17:16:54Z","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> The problem with that fix is that while its analysis of the problem is\n> correct, it doesn't actually call getcwd(3), instead it invokes \"pwd\n> -P\". There is no guarantee that \"pwd -P\" is going to call getcwd(3),\n> as opposed to e.g. being a shell built-in.\n>\n> On AIX under both bash and ksh this test breaks because \"pwd -P\" will\n> happily display the current working directory, but getcwd(3) called by\n> the \"git init\" we're testing here will fail to get it.\n\nWell described.  And logically it leads to \"when we want to know\ngetcwd() is OK to use, trying to run getcwd() to see it works is the\nright way to do so\", which is your test-getcwd.c?  Nice.\n\nWill queue.\n\n> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\n> index 490ac026c51..3ce5585e53a 100644\n> --- a/t/helper/test-tool.c\n> +++ b/t/helper/test-tool.c\n> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {\n>  \t{ \"fast-rebase\", cmd__fast_rebase },\n>  \t{ \"genrandom\", cmd__genrandom },\n>  \t{ \"genzeros\", cmd__genzeros },\n> +\t{ \"getcwd\", cmd__getcwd },\n>  \t{ \"hashmap\", cmd__hashmap },\n>  \t{ \"hash-speed\", cmd__hash_speed },\n>  \t{ \"index-version\", cmd__index_version },\n> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n> index f8dc266721f..9f0f5228508 100644\n> --- a/t/helper/test-tool.h\n> +++ b/t/helper/test-tool.h\n> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);\n>  int cmd__fast_rebase(int argc, const char **argv);\n>  int cmd__genrandom(int argc, const char **argv);\n>  int cmd__genzeros(int argc, const char **argv);\n> +int cmd__getcwd(int argc, const char **argv);\n>  int cmd__hashmap(int argc, const char **argv);\n>  int cmd__hash_speed(int argc, const char **argv);\n>  int cmd__index_version(int argc, const char **argv);\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index acd662e403b..df544bb321f 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n>  \tchmod 100 $base ||\n>  \tBUG \"cannot prepare $base\"\n>  \n> -\t(cd $base/dir && /bin/pwd -P)\n> +\t(\n> +\t\tcd $base/dir &&\n> +\t\ttest-tool getcwd\n> +\t)\n>  \tstatus=$?\n>  \n>  \tchmod 700 $base &&\n"}]}