git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
@ 2017-08-06 23:38 Ævar Arnfjörð Bjarmason
  2017-08-07  1:18 ` brian m. carlson
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2017-08-06 23:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, David Coppa, René Scharfe,
	Ævar Arnfjörð Bjarmason

Change an argument to test_line_count (which'll ultimately be turned
into a "test" expression) to use "-gt" instead of ">" for an
arithmetic test.

This broken on e.g. OpenBSD as of v2.13.0 with my commit
ac3f5a3468 ("ref-filter: add --no-contains option to
tag/branch/for-each-ref", 2017-03-24).

Upstream just worked around it by patching git and didn't tell us
about it, I discovered this when reading various Git packaging
implementations: https://github.com/openbsd/ports/commit/7e48bf88a20

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

David, it would be great to get a quick bug report to
git@vger.kernel.org if you end up having to monkeypatch something
we've done. We won't bite, promise :)

As shown in that linked Github commit OpenBSD has another recent
workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
related test, maybe René can make more sense of that?

There's more patches in their ports which indicate possible bugs of
ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/

 t/t7004-tag.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 0ef7b94394..0e2e57aa3d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1887,7 +1887,7 @@ EOF"
 	run_with_limited_stack git tag --contains HEAD >actual &&
 	test_cmp expect actual &&
 	run_with_limited_stack git tag --no-contains HEAD >actual &&
-	test_line_count ">" 10 actual
+	test_line_count "-gt" 10 actual
 '
 
 test_expect_success '--format should list tags as per format given' '
-- 
2.14.0.rc1.383.gd1ce394fe2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
  2017-08-06 23:38 [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Ævar Arnfjörð Bjarmason
@ 2017-08-07  1:18 ` brian m. carlson
  2017-08-07 10:17   ` René Scharfe
  2017-08-07  1:33 ` [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Junio C Hamano
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: brian m. carlson @ 2017-08-07  1:18 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, David Coppa, René Scharfe

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

On Sun, Aug 06, 2017 at 11:38:50PM +0000, Ævar Arnfjörð Bjarmason wrote:
> Change an argument to test_line_count (which'll ultimately be turned
> into a "test" expression) to use "-gt" instead of ">" for an
> arithmetic test.
> 
> This broken on e.g. OpenBSD as of v2.13.0 with my commit
> ac3f5a3468 ("ref-filter: add --no-contains option to
> tag/branch/for-each-ref", 2017-03-24).
> 
> Upstream just worked around it by patching git and didn't tell us
> about it, I discovered this when reading various Git packaging
> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 
> David, it would be great to get a quick bug report to
> git@vger.kernel.org if you end up having to monkeypatch something
> we've done. We won't bite, promise :)
> 
> As shown in that linked Github commit OpenBSD has another recent
> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
> related test, maybe René can make more sense of that?

I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want
DIR_HAS_BSD_GROUP_SEMANTICS.  MirBSD will also, according to its man
pages.

As I understand it, the only consequence of not setting this flag on BSD
systems is that some directories will be setgid, which, while ugly and
useless, should have no negative effect.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
  2017-08-06 23:38 [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Ævar Arnfjörð Bjarmason
  2017-08-07  1:18 ` brian m. carlson
@ 2017-08-07  1:33 ` Junio C Hamano
  2017-08-07 17:32 ` Junio C Hamano
  2017-08-07 18:06 ` Andreas Schwab
  3 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-08-07  1:33 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, David Coppa, René Scharfe

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

> Change an argument to test_line_count (which'll ultimately be turned
> into a "test" expression) to use "-gt" instead of ">" for an
> arithmetic test.
>
> This broken on e.g. OpenBSD as of v2.13.0 with my commit
> ac3f5a3468 ("ref-filter: add --no-contains option to
> tag/branch/for-each-ref", 2017-03-24).
>
> Upstream just worked around it by patching git and didn't tell us
> about it, I discovered this when reading various Git packaging
> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20

Thanks for finding and relaying this.

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> David, it would be great to get a quick bug report to
> git@vger.kernel.org if you end up having to monkeypatch something
> we've done. We won't bite, promise :)

Yeah.  I was hoping that your new list for platform porters would
help communicate these issues easier for those who are a bit shy to
be on this general development list ;-)

> As shown in that linked Github commit OpenBSD has another recent
> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
> related test, maybe René can make more sense of that?
>
> There's more patches in their ports which indicate possible bugs of
> ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/
>
>  t/t7004-tag.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 0ef7b94394..0e2e57aa3d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1887,7 +1887,7 @@ EOF"
>  	run_with_limited_stack git tag --contains HEAD >actual &&
>  	test_cmp expect actual &&
>  	run_with_limited_stack git tag --no-contains HEAD >actual &&
> -	test_line_count ">" 10 actual
> +	test_line_count "-gt" 10 actual
>  '
>  
>  test_expect_success '--format should list tags as per format given' '

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
  2017-08-07  1:18 ` brian m. carlson
@ 2017-08-07 10:17   ` René Scharfe
  2017-08-07 10:59     ` Ævar Arnfjörð Bjarmason
  2017-08-07 11:04     ` [PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them René Scharfe
  0 siblings, 2 replies; 17+ messages in thread
From: René Scharfe @ 2017-08-07 10:17 UTC (permalink / raw)
  To: brian m. carlson, Ævar Arnfjörð Bjarmason, git,
	Junio C Hamano, David Coppa

Am 07.08.2017 um 03:18 schrieb brian m. carlson:
> On Sun, Aug 06, 2017 at 11:38:50PM +0000, Ævar Arnfjörð Bjarmason wrote:
>> Change an argument to test_line_count (which'll ultimately be turned
>> into a "test" expression) to use "-gt" instead of ">" for an
>> arithmetic test.
>>
>> This broken on e.g. OpenBSD as of v2.13.0 with my commit
>> ac3f5a3468 ("ref-filter: add --no-contains option to
>> tag/branch/for-each-ref", 2017-03-24).
>>
>> Upstream just worked around it by patching git and didn't tell us
>> about it, I discovered this when reading various Git packaging
>> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>
>> David, it would be great to get a quick bug report to
>> git@vger.kernel.org if you end up having to monkeypatch something
>> we've done. We won't bite, promise :)
>>
>> As shown in that linked Github commit OpenBSD has another recent
>> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
>> related test, maybe René can make more sense of that?
> 
> I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want
> DIR_HAS_BSD_GROUP_SEMANTICS.  MirBSD will also, according to its man
> pages.
> 
> As I understand it, the only consequence of not setting this flag on BSD
> systems is that some directories will be setgid, which, while ugly and
> useless, should have no negative effect.

Right; specifically it's for newly created subdirectories of shared
directories, which we want to be owned by the same group as their
parent. That's the default on BSDs, and we have to set the setgid bit to
turn on that semantic only on other systems, e.g. on Linux.

81a24b52c1 (Do not use GUID on dir in git init --shared=all on FreeBSD)
introduced DIR_HAS_BSD_GROUP_SEMANTICS with the rationale that setting
setgid on directories may not even be allowed for normal users.  I can't
reproduce this (t1301 succeeds for me on FreeBSD 10.3 even without that
build flag), but apparently at least in some configurations it's not just
a cosmetic issue.

The skipped test 'init in long base path' in t0001 is a different kettle
of fish.  getcwd(3) on OpenBSD respects permissions on the parent
directories up to root.  E.g. after "chmod 711 /home" normal users would
get EACCES when they'd call getcwd(3) in their homes there, while e.g.
on Linux and FreeBSD they'd successfully get their current working dir.

René

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
  2017-08-07 10:17   ` René Scharfe
@ 2017-08-07 10:59     ` Ævar Arnfjörð Bjarmason
  2017-08-07 11:04     ` [PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them René Scharfe
  1 sibling, 0 replies; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2017-08-07 10:59 UTC (permalink / raw)
  To: René Scharfe; +Cc: brian m. carlson, git, Junio C Hamano, David Coppa


On Mon, Aug 07 2017, René Scharfe jotted:

> Am 07.08.2017 um 03:18 schrieb brian m. carlson:
>> On Sun, Aug 06, 2017 at 11:38:50PM +0000, Ævar Arnfjörð Bjarmason wrote:
>>> Change an argument to test_line_count (which'll ultimately be turned
>>> into a "test" expression) to use "-gt" instead of ">" for an
>>> arithmetic test.
>>>
>>> This broken on e.g. OpenBSD as of v2.13.0 with my commit
>>> ac3f5a3468 ("ref-filter: add --no-contains option to
>>> tag/branch/for-each-ref", 2017-03-24).
>>>
>>> Upstream just worked around it by patching git and didn't tell us
>>> about it, I discovered this when reading various Git packaging
>>> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>>>
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>> ---
>>>
>>> David, it would be great to get a quick bug report to
>>> git@vger.kernel.org if you end up having to monkeypatch something
>>> we've done. We won't bite, promise :)
>>>
>>> As shown in that linked Github commit OpenBSD has another recent
>>> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
>>> related test, maybe René can make more sense of that?
>>
>> I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want
>> DIR_HAS_BSD_GROUP_SEMANTICS.  MirBSD will also, according to its man
>> pages.
>>
>> As I understand it, the only consequence of not setting this flag on BSD
>> systems is that some directories will be setgid, which, while ugly and
>> useless, should have no negative effect.
>
> Right; specifically it's for newly created subdirectories of shared
> directories, which we want to be owned by the same group as their
> parent. That's the default on BSDs, and we have to set the setgid bit to
> turn on that semantic only on other systems, e.g. on Linux.
>
> 81a24b52c1 (Do not use GUID on dir in git init --shared=all on FreeBSD)
> introduced DIR_HAS_BSD_GROUP_SEMANTICS with the rationale that setting
> setgid on directories may not even be allowed for normal users.  I can't
> reproduce this (t1301 succeeds for me on FreeBSD 10.3 even without that
> build flag), but apparently at least in some configurations it's not just
> a cosmetic issue.
>
> The skipped test 'init in long base path' in t0001 is a different kettle
> of fish.  getcwd(3) on OpenBSD respects permissions on the parent
> directories up to root.  E.g. after "chmod 711 /home" normal users would
> get EACCES when they'd call getcwd(3) in their homes there, while e.g.
> on Linux and FreeBSD they'd successfully get their current working dir.

Good summary. As openbsd-tech points out it seems the bug is ours, if
I'm understanding this correctly:
http://marc.info/?l=openbsd-tech&m=149625768825173&w=2

    [EACCES]
        Search permission was denied for the current directory, or read or
        search permission was denied for a directory above the current
        directory in the file hierarchy.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them
  2017-08-07 10:17   ` René Scharfe
  2017-08-07 10:59     ` Ævar Arnfjörð Bjarmason
@ 2017-08-07 11:04     ` René Scharfe
  2021-06-01  0:38       ` [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2 Ævar Arnfjörð Bjarmason
  1 sibling, 1 reply; 17+ messages in thread
From: René Scharfe @ 2017-08-07 11:04 UTC (permalink / raw)
  To: brian m. carlson, Ævar Arnfjörð Bjarmason, git,
	Junio C Hamano, David Coppa

The sub-test "init in long base path" in t0001 checks the ability to
handle long base paths with restrictive permissions (--x).  On OpenBSD
getcwd(3) fails in that case even for short paths.  Check the two
aspects separately by trying to use a long base path both with and
without execute-only permissions.  Only attempt the former if we know
that getcwd(3) doesn't care.

Original-patch-by: David Coppa <dcoppa@openbsd.org>
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/t0001-init.sh | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index c4814d248f..86c1a51654 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -315,18 +315,44 @@ test_expect_success 'init with separate gitdir' '
 	test_path_is_dir realgitdir/refs
 '
 
-test_expect_success 'init in long base path' '
+test_lazy_prereq GETCWD_IGNORES_PERMS '
+	base=GETCWD_TEST_BASE_DIR &&
+	mkdir -p $base/dir &&
+	chmod 100 $base ||
+	error "bug in test script: cannot prepare $base"
+
+	(cd $base/dir && /bin/pwd -P)
+	status=$?
+
+	chmod 700 $base &&
+	rm -rf $base ||
+	error "bug in test script: cannot clean $base"
+	return $status
+'
+
+check_long_base_path () {
 	# exceed initial buffer size of strbuf_getcwd()
 	component=123456789abcdef &&
 	test_when_finished "chmod 0700 $component; rm -rf $component" &&
 	p31=$component/$component &&
 	p127=$p31/$p31/$p31/$p31 &&
 	mkdir -p $p127 &&
-	chmod 0111 $component &&
+	if test $# = 1
+	then
+		chmod $1 $component
+	fi &&
 	(
 		cd $p127 &&
 		git init newdir
 	)
+}
+
+test_expect_success 'init in long base path' '
+	check_long_base_path
+'
+
+test_expect_success GETCWD_IGNORES_PERMS 'init in long restricted base path' '
+	check_long_base_path 0111
 '
 
 test_expect_success 're-init on .git file' '
-- 
2.14.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
  2017-08-06 23:38 [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Ævar Arnfjörð Bjarmason
  2017-08-07  1:18 ` brian m. carlson
  2017-08-07  1:33 ` [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Junio C Hamano
@ 2017-08-07 17:32 ` Junio C Hamano
  2017-08-07 18:06 ` Andreas Schwab
  3 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-08-07 17:32 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, David Coppa, René Scharfe

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

> Change an argument to test_line_count (which'll ultimately be turned
> into a "test" expression) to use "-gt" instead of ">" for an
> arithmetic test.
>
> This broken on e.g. OpenBSD as of v2.13.0 with my commit
> ac3f5a3468 ("ref-filter: add --no-contains option to
> tag/branch/for-each-ref", 2017-03-24).
>
> Upstream just worked around it by patching git and didn't tell us

I'm tempted to do s/Upstream/Downstream/ while queuing, but please
tell me I am confused.

> about it, I discovered this when reading various Git packaging
> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> David, it would be great to get a quick bug report to
> git@vger.kernel.org if you end up having to monkeypatch something
> we've done. We won't bite, promise :)
>
> As shown in that linked Github commit OpenBSD has another recent
> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
> related test, maybe René can make more sense of that?
>
> There's more patches in their ports which indicate possible bugs of
> ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/
>
>  t/t7004-tag.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 0ef7b94394..0e2e57aa3d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1887,7 +1887,7 @@ EOF"
>  	run_with_limited_stack git tag --contains HEAD >actual &&
>  	test_cmp expect actual &&
>  	run_with_limited_stack git tag --no-contains HEAD >actual &&
> -	test_line_count ">" 10 actual
> +	test_line_count "-gt" 10 actual
>  '
>  
>  test_expect_success '--format should list tags as per format given' '

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
  2017-08-06 23:38 [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Ævar Arnfjörð Bjarmason
                   ` (2 preceding siblings ...)
  2017-08-07 17:32 ` Junio C Hamano
@ 2017-08-07 18:06 ` Andreas Schwab
  3 siblings, 0 replies; 17+ messages in thread
From: Andreas Schwab @ 2017-08-07 18:06 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, David Coppa, René Scharfe

On Aug 06 2017, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 0ef7b94394..0e2e57aa3d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1887,7 +1887,7 @@ EOF"
>  	run_with_limited_stack git tag --contains HEAD >actual &&
>  	test_cmp expect actual &&
>  	run_with_limited_stack git tag --no-contains HEAD >actual &&
> -	test_line_count ">" 10 actual
> +	test_line_count "-gt" 10 actual

Maybe also remove the quotes, they are no longer needed.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2017-08-07 11:04     ` [PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them René Scharfe
@ 2021-06-01  0:38       ` Ævar Arnfjörð Bjarmason
  2021-06-01 16:15         ` René Scharfe
  2021-07-30 16:18         ` [PATCH v2] " Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-06-01  0:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, René Scharfe, David Coppa,
	brian m . carlson, Ævar Arnfjörð Bjarmason

With a54e938e5b (strbuf: support long paths w/o read rights in
strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems
like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc
et al) behavior.

This was partially fixed in bed67874e2 (t0001: skip test with
restrictive permissions if getpwd(3) respects them, 2017-08-07).

The problem with that fix is that while its analysis of the problem is
correct, it doesn't actually call getcwd(3), instead it invokes "pwd
-P". There is no guarantee that "pwd -P" is actually going to call
getcwd(3), as opposed to e.g. being a shell built-in.

On AIX under both bash and ksh this test breaks because "pwd -P" will
happily display the current working directory, but getcwd(3) called by
the "git init" we're testing here will fail to get it.

I checked whether clobbering the $PWD environment variable would
affect it, and it didn't. Presumably these shells keep track of their
working directory internally.

Let's change the test to a new "test-tool getcwd".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd
send it now anyway in case we'd like these various AIX fixes in one
batch...

 Makefile               |  1 +
 t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++
 t/helper/test-tool.c   |  1 +
 t/helper/test-tool.h   |  1 +
 t/t0001-init.sh        |  5 ++++-
 5 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-getcwd.c

diff --git a/Makefile b/Makefile
index c3565fc0f8..8c000ba48b 100644
--- a/Makefile
+++ b/Makefile
@@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
 TEST_BUILTINS_OBJS += test-fast-rebase.o
 TEST_BUILTINS_OBJS += test-genrandom.o
 TEST_BUILTINS_OBJS += test-genzeros.o
+TEST_BUILTINS_OBJS += test-getcwd.o
 TEST_BUILTINS_OBJS += test-hash-speed.o
 TEST_BUILTINS_OBJS += test-hash.o
 TEST_BUILTINS_OBJS += test-hashmap.o
diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c
new file mode 100644
index 0000000000..d680038a78
--- /dev/null
+++ b/t/helper/test-getcwd.c
@@ -0,0 +1,26 @@
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+static const char *getcwd_usage[] = {
+	"test-tool getcwd",
+	NULL
+};
+
+int cmd__getcwd(int argc, const char **argv)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+	char *cwd;
+
+	argc = parse_options(argc, argv, "test-tools", options, getcwd_usage, 0);
+	if (argc > 0)
+		usage_with_options(getcwd_usage, options);
+
+	cwd = xgetcwd();
+	puts(cwd);
+	free(cwd);
+
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index c5bd0c6d4c..3c13cb19b5 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
 	{ "fast-rebase", cmd__fast_rebase },
 	{ "genrandom", cmd__genrandom },
 	{ "genzeros", cmd__genzeros },
+	{ "getcwd", cmd__getcwd },
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },
 	{ "index-version", cmd__index_version },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index e8069a3b22..e691a4d9e9 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);
 int cmd__fast_rebase(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
 int cmd__genzeros(int argc, const char **argv);
+int cmd__getcwd(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);
 int cmd__index_version(int argc, const char **argv);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index acd662e403..df544bb321 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '
 	chmod 100 $base ||
 	BUG "cannot prepare $base"
 
-	(cd $base/dir && /bin/pwd -P)
+	(
+		cd $base/dir &&
+		test-tool getcwd
+	)
 	status=$?
 
 	chmod 700 $base &&
-- 
2.32.0.rc1.460.g26a014da44c


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-01  0:38       ` [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2 Ævar Arnfjörð Bjarmason
@ 2021-06-01 16:15         ` René Scharfe
  2021-06-01 21:20           ` Junio C Hamano
  2021-06-07 11:24           ` Ævar Arnfjörð Bjarmason
  2021-07-30 16:18         ` [PATCH v2] " Ævar Arnfjörð Bjarmason
  1 sibling, 2 replies; 17+ messages in thread
From: René Scharfe @ 2021-06-01 16:15 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, git
  Cc: Junio C Hamano, David Coppa, brian m . carlson

Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:
> With a54e938e5b (strbuf: support long paths w/o read rights in
> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems
> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc
> et al) behavior.
>
> This was partially fixed in bed67874e2 (t0001: skip test with
> restrictive permissions if getpwd(3) respects them, 2017-08-07).
>
> The problem with that fix is that while its analysis of the problem is
> correct, it doesn't actually call getcwd(3), instead it invokes "pwd
> -P". There is no guarantee that "pwd -P" is actually going to call
> getcwd(3), as opposed to e.g. being a shell built-in.
>
> On AIX under both bash and ksh this test breaks because "pwd -P" will
> happily display the current working directory, but getcwd(3) called by
> the "git init" we're testing here will fail to get it.
>
> I checked whether clobbering the $PWD environment variable would
> affect it, and it didn't. Presumably these shells keep track of their
> working directory internally.
>
> Let's change the test to a new "test-tool getcwd".

Makes sense.

If /bin/pwd can figure out the path to the current working directory
without read permissions to parent directories then it might be possible
to teach strbuf_getcwd() the same trick, though.  How does it do it?

Perhaps it falls back to $PWD; POSIX says the behavior of pwd is
unspecified if that variable would be changed, so a compliant
implementation would be allowed to do that.  I think that way is not
interesting for strbuf_getcwd(), though, because if we trust that
variable then we can read it directly instead.  It gets stale if any
parent directory is renamed.  E.g. the following commands would print a
string ending in "stale":

	mkdir stale
	cd stale
	mv ../stale ../fresh
	chmod 111 ../fresh
	/bin/pwd -P

Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would
be a bit weird to expose this functionality in a command line tool, but
not in the library function, so this is unlikely.  You seem to say that
/bin/pwd is a shell builtin on your system, which is also weird, though.
The commands above would print a string ending in "fresh" with the
syscall method.

An evil way would be to temporarily add read permission to all parent
directories.  It would also print a string ending in "fresh".  You'd
probably see chmod calls when running /bin/pwd using truss in that
case, and it would fail if chmod is not allowed.

That's all I can think of.

If strbuf_getcwd() were to learn any of these tricks, then so would
"test-tool getcwd", via its xgetcwd() call.  At that point we'd better
rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.

But I guess we need none of that because we never got a request from
an AIX user to support a /home directory without read permissions,
right?

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd
> send it now anyway in case we'd like these various AIX fixes in one
> batch...
>
>  Makefile               |  1 +
>  t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++
>  t/helper/test-tool.c   |  1 +
>  t/helper/test-tool.h   |  1 +
>  t/t0001-init.sh        |  5 ++++-
>  5 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 t/helper/test-getcwd.c
>
> diff --git a/Makefile b/Makefile
> index c3565fc0f8..8c000ba48b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
>  TEST_BUILTINS_OBJS += test-fast-rebase.o
>  TEST_BUILTINS_OBJS += test-genrandom.o
>  TEST_BUILTINS_OBJS += test-genzeros.o
> +TEST_BUILTINS_OBJS += test-getcwd.o
>  TEST_BUILTINS_OBJS += test-hash-speed.o
>  TEST_BUILTINS_OBJS += test-hash.o
>  TEST_BUILTINS_OBJS += test-hashmap.o
> diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c
> new file mode 100644
> index 0000000000..d680038a78
> --- /dev/null
> +++ b/t/helper/test-getcwd.c
> @@ -0,0 +1,26 @@
> +#include "test-tool.h"
> +#include "git-compat-util.h"
> +#include "parse-options.h"
> +
> +static const char *getcwd_usage[] = {
> +	"test-tool getcwd",
> +	NULL
> +};
> +
> +int cmd__getcwd(int argc, const char **argv)
> +{
> +	struct option options[] = {
> +		OPT_END()
> +	};
> +	char *cwd;
> +
> +	argc = parse_options(argc, argv, "test-tools", options, getcwd_usage, 0);
> +	if (argc > 0)
> +		usage_with_options(getcwd_usage, options);
> +
> +	cwd = xgetcwd();
> +	puts(cwd);
> +	free(cwd);
> +
> +	return 0;
> +}
> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> index c5bd0c6d4c..3c13cb19b5 100644
> --- a/t/helper/test-tool.c
> +++ b/t/helper/test-tool.c
> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
>  	{ "fast-rebase", cmd__fast_rebase },
>  	{ "genrandom", cmd__genrandom },
>  	{ "genzeros", cmd__genzeros },
> +	{ "getcwd", cmd__getcwd },
>  	{ "hashmap", cmd__hashmap },
>  	{ "hash-speed", cmd__hash_speed },
>  	{ "index-version", cmd__index_version },
> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
> index e8069a3b22..e691a4d9e9 100644
> --- a/t/helper/test-tool.h
> +++ b/t/helper/test-tool.h
> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);
>  int cmd__fast_rebase(int argc, const char **argv);
>  int cmd__genrandom(int argc, const char **argv);
>  int cmd__genzeros(int argc, const char **argv);
> +int cmd__getcwd(int argc, const char **argv);
>  int cmd__hashmap(int argc, const char **argv);
>  int cmd__hash_speed(int argc, const char **argv);
>  int cmd__index_version(int argc, const char **argv);
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index acd662e403..df544bb321 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '
>  	chmod 100 $base ||
>  	BUG "cannot prepare $base"
>
> -	(cd $base/dir && /bin/pwd -P)
> +	(
> +		cd $base/dir &&
> +		test-tool getcwd
> +	)
>  	status=$?
>
>  	chmod 700 $base &&
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-01 16:15         ` René Scharfe
@ 2021-06-01 21:20           ` Junio C Hamano
  2021-06-07 11:29             ` Ævar Arnfjörð Bjarmason
  2021-06-07 11:24           ` Ævar Arnfjörð Bjarmason
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2021-06-01 21:20 UTC (permalink / raw)
  To: René Scharfe
  Cc: Ævar Arnfjörð Bjarmason, git, David Coppa,
	brian m . carlson

René Scharfe <l.s.r@web.de> writes:

>> On AIX under both bash and ksh this test breaks because "pwd -P" will
>> happily display the current working directory, but getcwd(3) called by
>> the "git init" we're testing here will fail to get it.
>>
>> I checked whether clobbering the $PWD environment variable would
>> affect it, and it didn't. Presumably these shells keep track of their
>> working directory internally.
>>
>> Let's change the test to a new "test-tool getcwd".
>
> Makes sense.
>
> If /bin/pwd can figure out the path to the current working directory
> without read permissions to parent directories then it might be possible
> to teach strbuf_getcwd() the same trick, though.  How does it do it?
> ...
> If strbuf_getcwd() were to learn any of these tricks, then so would
> "test-tool getcwd", via its xgetcwd() call.  At that point we'd better
> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.
>
> But I guess we need none of that because we never got a request from
> an AIX user to support a /home directory without read permissions,
> right?

Nice "thinking aloud".

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-01 16:15         ` René Scharfe
  2021-06-01 21:20           ` Junio C Hamano
@ 2021-06-07 11:24           ` Ævar Arnfjörð Bjarmason
  2021-06-07 15:26             ` René Scharfe
  1 sibling, 1 reply; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-06-07 11:24 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, Junio C Hamano, David Coppa, brian m . carlson


On Tue, Jun 01 2021, René Scharfe wrote:

> Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:
>> With a54e938e5b (strbuf: support long paths w/o read rights in
>> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems
>> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc
>> et al) behavior.
>>
>> This was partially fixed in bed67874e2 (t0001: skip test with
>> restrictive permissions if getpwd(3) respects them, 2017-08-07).
>>
>> The problem with that fix is that while its analysis of the problem is
>> correct, it doesn't actually call getcwd(3), instead it invokes "pwd
>> -P". There is no guarantee that "pwd -P" is actually going to call
>> getcwd(3), as opposed to e.g. being a shell built-in.
>>
>> On AIX under both bash and ksh this test breaks because "pwd -P" will
>> happily display the current working directory, but getcwd(3) called by
>> the "git init" we're testing here will fail to get it.
>>
>> I checked whether clobbering the $PWD environment variable would
>> affect it, and it didn't. Presumably these shells keep track of their
>> working directory internally.
>>
>> Let's change the test to a new "test-tool getcwd".
>
> Makes sense.
>
> If /bin/pwd can figure out the path to the current working directory
> without read permissions to parent directories then it might be possible
> to teach strbuf_getcwd() the same trick, though.  How does it do it?
>
> Perhaps it falls back to $PWD; POSIX says the behavior of pwd is
> unspecified if that variable would be changed, so a compliant
> implementation would be allowed to do that.  I think that way is not
> interesting for strbuf_getcwd(), though, because if we trust that
> variable then we can read it directly instead.  It gets stale if any
> parent directory is renamed.  E.g. the following commands would print a
> string ending in "stale":
>
> 	mkdir stale
> 	cd stale
> 	mv ../stale ../fresh
> 	chmod 111 ../fresh
> 	/bin/pwd -P

Yes, AIX prints "stale" here, but e.g. my Linux box prints "fresh".

> Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would
> be a bit weird to expose this functionality in a command line tool, but
> not in the library function, so this is unlikely.  You seem to say that
> /bin/pwd is a shell builtin on your system, which is also weird, though.
> The commands above would print a string ending in "fresh" with the
> syscall method.
>
> An evil way would be to temporarily add read permission to all parent
> directories.  It would also print a string ending in "fresh".  You'd
> probably see chmod calls when running /bin/pwd using truss in that
> case, and it would fail if chmod is not allowed.
>
> That's all I can think of.
>
> If strbuf_getcwd() were to learn any of these tricks, then so would
> "test-tool getcwd", via its xgetcwd() call.  At that point we'd better
> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.
>
> But I guess we need none of that because we never got a request from
> an AIX user to support a /home directory without read permissions,
> right?

I don't really see the point of trying that hard. Yes, we could make
some forward progress if we bent over backwards and got the current
working directory, but what would we be left with? A git repository the
user can't "ls" inside of.

So any number of other thing after that now-working xgetcwd() call would
fail, we couldn't list any files in the working tree or .git directory.

Why not just fix the bug in there being disconnect between pwd and
getcwd() here and move on?

>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>
>> Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd
>> send it now anyway in case we'd like these various AIX fixes in one
>> batch...
>>
>>  Makefile               |  1 +
>>  t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++
>>  t/helper/test-tool.c   |  1 +
>>  t/helper/test-tool.h   |  1 +
>>  t/t0001-init.sh        |  5 ++++-
>>  5 files changed, 33 insertions(+), 1 deletion(-)
>>  create mode 100644 t/helper/test-getcwd.c
>>
>> diff --git a/Makefile b/Makefile
>> index c3565fc0f8..8c000ba48b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
>>  TEST_BUILTINS_OBJS += test-fast-rebase.o
>>  TEST_BUILTINS_OBJS += test-genrandom.o
>>  TEST_BUILTINS_OBJS += test-genzeros.o
>> +TEST_BUILTINS_OBJS += test-getcwd.o
>>  TEST_BUILTINS_OBJS += test-hash-speed.o
>>  TEST_BUILTINS_OBJS += test-hash.o
>>  TEST_BUILTINS_OBJS += test-hashmap.o
>> diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c
>> new file mode 100644
>> index 0000000000..d680038a78
>> --- /dev/null
>> +++ b/t/helper/test-getcwd.c
>> @@ -0,0 +1,26 @@
>> +#include "test-tool.h"
>> +#include "git-compat-util.h"
>> +#include "parse-options.h"
>> +
>> +static const char *getcwd_usage[] = {
>> +	"test-tool getcwd",
>> +	NULL
>> +};
>> +
>> +int cmd__getcwd(int argc, const char **argv)
>> +{
>> +	struct option options[] = {
>> +		OPT_END()
>> +	};
>> +	char *cwd;
>> +
>> +	argc = parse_options(argc, argv, "test-tools", options, getcwd_usage, 0);
>> +	if (argc > 0)
>> +		usage_with_options(getcwd_usage, options);
>> +
>> +	cwd = xgetcwd();
>> +	puts(cwd);
>> +	free(cwd);
>> +
>> +	return 0;
>> +}
>> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
>> index c5bd0c6d4c..3c13cb19b5 100644
>> --- a/t/helper/test-tool.c
>> +++ b/t/helper/test-tool.c
>> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
>>  	{ "fast-rebase", cmd__fast_rebase },
>>  	{ "genrandom", cmd__genrandom },
>>  	{ "genzeros", cmd__genzeros },
>> +	{ "getcwd", cmd__getcwd },
>>  	{ "hashmap", cmd__hashmap },
>>  	{ "hash-speed", cmd__hash_speed },
>>  	{ "index-version", cmd__index_version },
>> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
>> index e8069a3b22..e691a4d9e9 100644
>> --- a/t/helper/test-tool.h
>> +++ b/t/helper/test-tool.h
>> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);
>>  int cmd__fast_rebase(int argc, const char **argv);
>>  int cmd__genrandom(int argc, const char **argv);
>>  int cmd__genzeros(int argc, const char **argv);
>> +int cmd__getcwd(int argc, const char **argv);
>>  int cmd__hashmap(int argc, const char **argv);
>>  int cmd__hash_speed(int argc, const char **argv);
>>  int cmd__index_version(int argc, const char **argv);
>> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
>> index acd662e403..df544bb321 100755
>> --- a/t/t0001-init.sh
>> +++ b/t/t0001-init.sh
>> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '
>>  	chmod 100 $base ||
>>  	BUG "cannot prepare $base"
>>
>> -	(cd $base/dir && /bin/pwd -P)
>> +	(
>> +		cd $base/dir &&
>> +		test-tool getcwd
>> +	)
>>  	status=$?
>>
>>  	chmod 700 $base &&
>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-01 21:20           ` Junio C Hamano
@ 2021-06-07 11:29             ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-06-07 11:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: René Scharfe, git, David Coppa, brian m . carlson


On Wed, Jun 02 2021, Junio C Hamano wrote:

> René Scharfe <l.s.r@web.de> writes:
>
>>> On AIX under both bash and ksh this test breaks because "pwd -P" will
>>> happily display the current working directory, but getcwd(3) called by
>>> the "git init" we're testing here will fail to get it.
>>>
>>> I checked whether clobbering the $PWD environment variable would
>>> affect it, and it didn't. Presumably these shells keep track of their
>>> working directory internally.
>>>
>>> Let's change the test to a new "test-tool getcwd".
>>
>> Makes sense.
>>
>> If /bin/pwd can figure out the path to the current working directory
>> without read permissions to parent directories then it might be possible
>> to teach strbuf_getcwd() the same trick, though.  How does it do it?
>> ...
>> If strbuf_getcwd() were to learn any of these tricks, then so would
>> "test-tool getcwd", via its xgetcwd() call.  At that point we'd better
>> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.
>>
>> But I guess we need none of that because we never got a request from
>> an AIX user to support a /home directory without read permissions,
>> right?
>
> Nice "thinking aloud".

I see (well, rc/release period and all) that you didn't pick this up. I
think per my just-sent
http://lore.kernel.org/git/871r9d6hhy.fsf@evledraar.gmail.com that it
makes sense to just leave this at s/pwd(1)/getcwd(3)/ as this patch
does.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-07 11:24           ` Ævar Arnfjörð Bjarmason
@ 2021-06-07 15:26             ` René Scharfe
  2021-06-07 15:38               ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 17+ messages in thread
From: René Scharfe @ 2021-06-07 15:26 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, David Coppa, brian m . carlson

Am 07.06.21 um 13:24 schrieb Ævar Arnfjörð Bjarmason:
>
> On Tue, Jun 01 2021, René Scharfe wrote:
>
>> Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:
>>> With a54e938e5b (strbuf: support long paths w/o read rights in
>>> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems
>>> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc
>>> et al) behavior.
>>>
>>> This was partially fixed in bed67874e2 (t0001: skip test with
>>> restrictive permissions if getpwd(3) respects them, 2017-08-07).
>>>
>>> The problem with that fix is that while its analysis of the problem is
>>> correct, it doesn't actually call getcwd(3), instead it invokes "pwd
>>> -P". There is no guarantee that "pwd -P" is actually going to call
>>> getcwd(3), as opposed to e.g. being a shell built-in.
>>>
>>> On AIX under both bash and ksh this test breaks because "pwd -P" will
>>> happily display the current working directory, but getcwd(3) called by
>>> the "git init" we're testing here will fail to get it.
>>>
>>> I checked whether clobbering the $PWD environment variable would
>>> affect it, and it didn't. Presumably these shells keep track of their
>>> working directory internally.
>>>
>>> Let's change the test to a new "test-tool getcwd".
>>
>> Makes sense.
>>
>> If /bin/pwd can figure out the path to the current working directory
>> without read permissions to parent directories then it might be possible
>> to teach strbuf_getcwd() the same trick, though.  How does it do it?
>>
>> Perhaps it falls back to $PWD; POSIX says the behavior of pwd is
>> unspecified if that variable would be changed, so a compliant
>> implementation would be allowed to do that.  I think that way is not
>> interesting for strbuf_getcwd(), though, because if we trust that
>> variable then we can read it directly instead.  It gets stale if any
>> parent directory is renamed.  E.g. the following commands would print a
>> string ending in "stale":
>>
>> 	mkdir stale
>> 	cd stale
>> 	mv ../stale ../fresh
>> 	chmod 111 ../fresh
>> 	/bin/pwd -P
>
> Yes, AIX prints "stale" here, but e.g. my Linux box prints "fresh".

OK, thanks for checking.  I find it weird: Why would they add a command
that basically prints $PWD when callers can easily access this variable
directly?  Anyway, it is what it is.

>> Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would
>> be a bit weird to expose this functionality in a command line tool, but
>> not in the library function, so this is unlikely.  You seem to say that
>> /bin/pwd is a shell builtin on your system, which is also weird, though.
>> The commands above would print a string ending in "fresh" with the
>> syscall method.
>>
>> An evil way would be to temporarily add read permission to all parent
>> directories.  It would also print a string ending in "fresh".  You'd
>> probably see chmod calls when running /bin/pwd using truss in that
>> case, and it would fail if chmod is not allowed.
>>
>> That's all I can think of.
>>
>> If strbuf_getcwd() were to learn any of these tricks, then so would
>> "test-tool getcwd", via its xgetcwd() call.  At that point we'd better
>> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.
>>
>> But I guess we need none of that because we never got a request from
>> an AIX user to support a /home directory without read permissions,
>> right?
>
> I don't really see the point of trying that hard. Yes, we could make
> some forward progress if we bent over backwards and got the current
> working directory, but what would we be left with? A git repository the
> user can't "ls" inside of.

The reason would be support for execute-only (e.g. 0711) /home, which
some systems have for privacy reasons.

> So any number of other thing after that now-working xgetcwd() call would
> fail, we couldn't list any files in the working tree or .git directory.

Users own their /home/directory in that scenario and have full
permissions in their repositories.  They cannot verify the name of their
/home/directory using readdir(), though.

> Why not just fix the bug in there being disconnect between pwd and
> getcwd() here and move on?

If you mean the false assumption that /bin/pwd uses getcwd(): I still
think that your patch makes sense, as I wrote in my first reply,
implying that I agree it should be applied.

>
>>>
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>> ---
>>>
>>> Not an issue new in v2.32.0, but an easy enough fix, so I thought I'd
>>> send it now anyway in case we'd like these various AIX fixes in one
>>> batch...
>>>
>>>  Makefile               |  1 +
>>>  t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++
>>>  t/helper/test-tool.c   |  1 +
>>>  t/helper/test-tool.h   |  1 +
>>>  t/t0001-init.sh        |  5 ++++-
>>>  5 files changed, 33 insertions(+), 1 deletion(-)
>>>  create mode 100644 t/helper/test-getcwd.c
>>>
>>> diff --git a/Makefile b/Makefile
>>> index c3565fc0f8..8c000ba48b 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -711,6 +711,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
>>>  TEST_BUILTINS_OBJS += test-fast-rebase.o
>>>  TEST_BUILTINS_OBJS += test-genrandom.o
>>>  TEST_BUILTINS_OBJS += test-genzeros.o
>>> +TEST_BUILTINS_OBJS += test-getcwd.o
>>>  TEST_BUILTINS_OBJS += test-hash-speed.o
>>>  TEST_BUILTINS_OBJS += test-hash.o
>>>  TEST_BUILTINS_OBJS += test-hashmap.o
>>> diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c
>>> new file mode 100644
>>> index 0000000000..d680038a78
>>> --- /dev/null
>>> +++ b/t/helper/test-getcwd.c
>>> @@ -0,0 +1,26 @@
>>> +#include "test-tool.h"
>>> +#include "git-compat-util.h"
>>> +#include "parse-options.h"
>>> +
>>> +static const char *getcwd_usage[] = {
>>> +	"test-tool getcwd",
>>> +	NULL
>>> +};
>>> +
>>> +int cmd__getcwd(int argc, const char **argv)
>>> +{
>>> +	struct option options[] = {
>>> +		OPT_END()
>>> +	};
>>> +	char *cwd;
>>> +
>>> +	argc = parse_options(argc, argv, "test-tools", options, getcwd_usage, 0);
>>> +	if (argc > 0)
>>> +		usage_with_options(getcwd_usage, options);
>>> +
>>> +	cwd = xgetcwd();
>>> +	puts(cwd);
>>> +	free(cwd);
>>> +
>>> +	return 0;
>>> +}
>>> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
>>> index c5bd0c6d4c..3c13cb19b5 100644
>>> --- a/t/helper/test-tool.c
>>> +++ b/t/helper/test-tool.c
>>> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
>>>  	{ "fast-rebase", cmd__fast_rebase },
>>>  	{ "genrandom", cmd__genrandom },
>>>  	{ "genzeros", cmd__genzeros },
>>> +	{ "getcwd", cmd__getcwd },
>>>  	{ "hashmap", cmd__hashmap },
>>>  	{ "hash-speed", cmd__hash_speed },
>>>  	{ "index-version", cmd__index_version },
>>> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
>>> index e8069a3b22..e691a4d9e9 100644
>>> --- a/t/helper/test-tool.h
>>> +++ b/t/helper/test-tool.h
>>> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);
>>>  int cmd__fast_rebase(int argc, const char **argv);
>>>  int cmd__genrandom(int argc, const char **argv);
>>>  int cmd__genzeros(int argc, const char **argv);
>>> +int cmd__getcwd(int argc, const char **argv);
>>>  int cmd__hashmap(int argc, const char **argv);
>>>  int cmd__hash_speed(int argc, const char **argv);
>>>  int cmd__index_version(int argc, const char **argv);
>>> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
>>> index acd662e403..df544bb321 100755
>>> --- a/t/t0001-init.sh
>>> +++ b/t/t0001-init.sh
>>> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '
>>>  	chmod 100 $base ||
>>>  	BUG "cannot prepare $base"
>>>
>>> -	(cd $base/dir && /bin/pwd -P)
>>> +	(
>>> +		cd $base/dir &&
>>> +		test-tool getcwd
>>> +	)
>>>  	status=$?
>>>
>>>  	chmod 700 $base &&
>>>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-07 15:26             ` René Scharfe
@ 2021-06-07 15:38               ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-06-07 15:38 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, Junio C Hamano, David Coppa, brian m . carlson


On Mon, Jun 07 2021, René Scharfe wrote:

> Am 07.06.21 um 13:24 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Tue, Jun 01 2021, René Scharfe wrote:
>>
>>> Am 01.06.21 um 02:38 schrieb Ævar Arnfjörð Bjarmason:
>>>> With a54e938e5b (strbuf: support long paths w/o read rights in
>>>> strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems
>>>> like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc
>>>> et al) behavior.
>>>>
>>>> This was partially fixed in bed67874e2 (t0001: skip test with
>>>> restrictive permissions if getpwd(3) respects them, 2017-08-07).
>>>>
>>>> The problem with that fix is that while its analysis of the problem is
>>>> correct, it doesn't actually call getcwd(3), instead it invokes "pwd
>>>> -P". There is no guarantee that "pwd -P" is actually going to call
>>>> getcwd(3), as opposed to e.g. being a shell built-in.
>>>>
>>>> On AIX under both bash and ksh this test breaks because "pwd -P" will
>>>> happily display the current working directory, but getcwd(3) called by
>>>> the "git init" we're testing here will fail to get it.
>>>>
>>>> I checked whether clobbering the $PWD environment variable would
>>>> affect it, and it didn't. Presumably these shells keep track of their
>>>> working directory internally.
>>>>
>>>> Let's change the test to a new "test-tool getcwd".
>>>
>>> Makes sense.
>>>
>>> If /bin/pwd can figure out the path to the current working directory
>>> without read permissions to parent directories then it might be possible
>>> to teach strbuf_getcwd() the same trick, though.  How does it do it?
>>>
>>> Perhaps it falls back to $PWD; POSIX says the behavior of pwd is
>>> unspecified if that variable would be changed, so a compliant
>>> implementation would be allowed to do that.  I think that way is not
>>> interesting for strbuf_getcwd(), though, because if we trust that
>>> variable then we can read it directly instead.  It gets stale if any
>>> parent directory is renamed.  E.g. the following commands would print a
>>> string ending in "stale":
>>>
>>> 	mkdir stale
>>> 	cd stale
>>> 	mv ../stale ../fresh
>>> 	chmod 111 ../fresh
>>> 	/bin/pwd -P
>>
>> Yes, AIX prints "stale" here, but e.g. my Linux box prints "fresh".
>
> OK, thanks for checking.  I find it weird: Why would they add a command
> that basically prints $PWD when callers can easily access this variable
> directly?  Anyway, it is what it is.

Not so much them but POSIX sayeth:

    If an application sets or unsets the value of PWD , the behavior of
    pwd is unspecified.
    -- https://pubs.opengroup.org/onlinepubs/007904875/utilities/pwd.html

And you can't be POSIX-compatible without a pwd(1) command. Ergo a
system like AIX needs a pwd utility, whether it'll return the same thing
as "$PWD" in some scenarios or not.

By they way: I don't know how AIX implements pwd(1), and whether it's
purely redundant or whatever in this case.

>>> Perhaps it asks the kernel, like getcwd() does on FreeBSD.  It would
>>> be a bit weird to expose this functionality in a command line tool, but
>>> not in the library function, so this is unlikely.  You seem to say that
>>> /bin/pwd is a shell builtin on your system, which is also weird, though.
>>> The commands above would print a string ending in "fresh" with the
>>> syscall method.
>>>
>>> An evil way would be to temporarily add read permission to all parent
>>> directories.  It would also print a string ending in "fresh".  You'd
>>> probably see chmod calls when running /bin/pwd using truss in that
>>> case, and it would fail if chmod is not allowed.
>>>
>>> That's all I can think of.
>>>
>>> If strbuf_getcwd() were to learn any of these tricks, then so would
>>> "test-tool getcwd", via its xgetcwd() call.  At that point we'd better
>>> rename GETCWD_IGNORES_PERMS to XGETCWD_IGNORES_PERMS.
>>>
>>> But I guess we need none of that because we never got a request from
>>> an AIX user to support a /home directory without read permissions,
>>> right?
>>
>> I don't really see the point of trying that hard. Yes, we could make
>> some forward progress if we bent over backwards and got the current
>> working directory, but what would we be left with? A git repository the
>> user can't "ls" inside of.
>
> The reason would be support for execute-only (e.g. 0711) /home, which
> some systems have for privacy reasons.
>
>> So any number of other thing after that now-working xgetcwd() call would
>> fail, we couldn't list any files in the working tree or .git directory.
>
> Users own their /home/directory in that scenario and have full
> permissions in their repositories.  They cannot verify the name of their
> /home/directory using readdir(), though.

Ah, I think I'd earlier misunderstood the test-case and thought it was
merely about a "git init xyz" where xyz itself is in that state..

>> Why not just fix the bug in there being disconnect between pwd and
>> getcwd() here and move on?
>
> If you mean the false assumption that /bin/pwd uses getcwd(): I still
> think that your patch makes sense, as I wrote in my first reply,
> implying that I agree it should be applied.

Ah, thanks. I wasn't quite clear or that, i.e. whether the
strbuf_getcwd() was a suggestion of some follow-up work or that this fix
should take that alternate route. Thanks!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-06-01  0:38       ` [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2 Ævar Arnfjörð Bjarmason
  2021-06-01 16:15         ` René Scharfe
@ 2021-07-30 16:18         ` Ævar Arnfjörð Bjarmason
  2021-07-30 17:16           ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-30 16:18 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, René Scharfe, David Coppa,
	brian m . carlson, Ævar Arnfjörð Bjarmason

With a54e938e5b (strbuf: support long paths w/o read rights in
strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems
like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc
et al) behavior.

This was partially fixed in bed67874e2 (t0001: skip test with
restrictive permissions if getpwd(3) respects them, 2017-08-07).

The problem with that fix is that while its analysis of the problem is
correct, it doesn't actually call getcwd(3), instead it invokes "pwd
-P". There is no guarantee that "pwd -P" is going to call getcwd(3),
as opposed to e.g. being a shell built-in.

On AIX under both bash and ksh this test breaks because "pwd -P" will
happily display the current working directory, but getcwd(3) called by
the "git init" we're testing here will fail to get it.

I checked whether clobbering the $PWD environment variable would
affect it, and it didn't. Presumably these shells keep track of their
working directory internally.

There's possible follow-up work here in teaching strbuf_getcwd() to
get the working directory with whatever method "pwd" uses on these
platforms. See [1] for a discussion of that, but let's take the easy
way out here and just skip these tests by fixing the
GETCWD_IGNORES_PERMS prerequisite to match the limitations of
strbuf_getcwd().

1. https://lore.kernel.org/git/b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Junio: I tried to get this in around the v2.32.0 release, see previous
proddings at:

  https://lore.kernel.org/git/871r9d6hhy.fsf@evledraar.gmail.com/
  https://lore.kernel.org/git/87y2bl52v4.fsf@evledraar.gmail.com/

Here's another try with a slightly updated commit message discussing
that there's a possible more perfect solution here (per René's reply),
but that this fixes the immediate bug on AIX/OpenBSD etc.

Range-diff against v1:
1:  c70791bbd3a ! 1:  d7d071441b0 t0001: fix broken not-quite getcwd(3) test in bed67874e2
    @@ Commit message
     
         The problem with that fix is that while its analysis of the problem is
         correct, it doesn't actually call getcwd(3), instead it invokes "pwd
    -    -P". There is no guarantee that "pwd -P" is actually going to call
    -    getcwd(3), as opposed to e.g. being a shell built-in.
    +    -P". There is no guarantee that "pwd -P" is going to call getcwd(3),
    +    as opposed to e.g. being a shell built-in.
     
         On AIX under both bash and ksh this test breaks because "pwd -P" will
         happily display the current working directory, but getcwd(3) called by
    @@ Commit message
         affect it, and it didn't. Presumably these shells keep track of their
         working directory internally.
     
    -    Let's change the test to a new "test-tool getcwd".
    +    There's possible follow-up work here in teaching strbuf_getcwd() to
    +    get the working directory with whatever method "pwd" uses on these
    +    platforms. See [1] for a discussion of that, but let's take the easy
    +    way out here and just skip these tests by fixing the
    +    GETCWD_IGNORES_PERMS prerequisite to match the limitations of
    +    strbuf_getcwd().
    +
    +    1. https://lore.kernel.org/git/b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     

 Makefile               |  1 +
 t/helper/test-getcwd.c | 26 ++++++++++++++++++++++++++
 t/helper/test-tool.c   |  1 +
 t/helper/test-tool.h   |  1 +
 t/t0001-init.sh        |  5 ++++-
 5 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-getcwd.c

diff --git a/Makefile b/Makefile
index c6f6246bf63..9573190f1d7 100644
--- a/Makefile
+++ b/Makefile
@@ -715,6 +715,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
 TEST_BUILTINS_OBJS += test-fast-rebase.o
 TEST_BUILTINS_OBJS += test-genrandom.o
 TEST_BUILTINS_OBJS += test-genzeros.o
+TEST_BUILTINS_OBJS += test-getcwd.o
 TEST_BUILTINS_OBJS += test-hash-speed.o
 TEST_BUILTINS_OBJS += test-hash.o
 TEST_BUILTINS_OBJS += test-hashmap.o
diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c
new file mode 100644
index 00000000000..d680038a780
--- /dev/null
+++ b/t/helper/test-getcwd.c
@@ -0,0 +1,26 @@
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+static const char *getcwd_usage[] = {
+	"test-tool getcwd",
+	NULL
+};
+
+int cmd__getcwd(int argc, const char **argv)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+	char *cwd;
+
+	argc = parse_options(argc, argv, "test-tools", options, getcwd_usage, 0);
+	if (argc > 0)
+		usage_with_options(getcwd_usage, options);
+
+	cwd = xgetcwd();
+	puts(cwd);
+	free(cwd);
+
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 490ac026c51..3ce5585e53a 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
 	{ "fast-rebase", cmd__fast_rebase },
 	{ "genrandom", cmd__genrandom },
 	{ "genzeros", cmd__genzeros },
+	{ "getcwd", cmd__getcwd },
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },
 	{ "index-version", cmd__index_version },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index f8dc266721f..9f0f5228508 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);
 int cmd__fast_rebase(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
 int cmd__genzeros(int argc, const char **argv);
+int cmd__getcwd(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);
 int cmd__index_version(int argc, const char **argv);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index acd662e403b..df544bb321f 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '
 	chmod 100 $base ||
 	BUG "cannot prepare $base"
 
-	(cd $base/dir && /bin/pwd -P)
+	(
+		cd $base/dir &&
+		test-tool getcwd
+	)
 	status=$?
 
 	chmod 700 $base &&
-- 
2.32.0.1071.g36f34456314


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] t0001: fix broken not-quite getcwd(3) test in bed67874e2
  2021-07-30 16:18         ` [PATCH v2] " Ævar Arnfjörð Bjarmason
@ 2021-07-30 17:16           ` Junio C Hamano
  0 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2021-07-30 17:16 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, René Scharfe, David Coppa, brian m . carlson

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

> The problem with that fix is that while its analysis of the problem is
> correct, it doesn't actually call getcwd(3), instead it invokes "pwd
> -P". There is no guarantee that "pwd -P" is going to call getcwd(3),
> as opposed to e.g. being a shell built-in.
>
> On AIX under both bash and ksh this test breaks because "pwd -P" will
> happily display the current working directory, but getcwd(3) called by
> the "git init" we're testing here will fail to get it.

Well described.  And logically it leads to "when we want to know
getcwd() is OK to use, trying to run getcwd() to see it works is the
right way to do so", which is your test-getcwd.c?  Nice.

Will queue.

> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> index 490ac026c51..3ce5585e53a 100644
> --- a/t/helper/test-tool.c
> +++ b/t/helper/test-tool.c
> @@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
>  	{ "fast-rebase", cmd__fast_rebase },
>  	{ "genrandom", cmd__genrandom },
>  	{ "genzeros", cmd__genzeros },
> +	{ "getcwd", cmd__getcwd },
>  	{ "hashmap", cmd__hashmap },
>  	{ "hash-speed", cmd__hash_speed },
>  	{ "index-version", cmd__index_version },
> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
> index f8dc266721f..9f0f5228508 100644
> --- a/t/helper/test-tool.h
> +++ b/t/helper/test-tool.h
> @@ -23,6 +23,7 @@ int cmd__example_decorate(int argc, const char **argv);
>  int cmd__fast_rebase(int argc, const char **argv);
>  int cmd__genrandom(int argc, const char **argv);
>  int cmd__genzeros(int argc, const char **argv);
> +int cmd__getcwd(int argc, const char **argv);
>  int cmd__hashmap(int argc, const char **argv);
>  int cmd__hash_speed(int argc, const char **argv);
>  int cmd__index_version(int argc, const char **argv);
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index acd662e403b..df544bb321f 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -356,7 +356,10 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '
>  	chmod 100 $base ||
>  	BUG "cannot prepare $base"
>  
> -	(cd $base/dir && /bin/pwd -P)
> +	(
> +		cd $base/dir &&
> +		test-tool getcwd
> +	)
>  	status=$?
>  
>  	chmod 700 $base &&

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-07-30 17:16 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-06 23:38 [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Ævar Arnfjörð Bjarmason
2017-08-07  1:18 ` brian m. carlson
2017-08-07 10:17   ` René Scharfe
2017-08-07 10:59     ` Ævar Arnfjörð Bjarmason
2017-08-07 11:04     ` [PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them René Scharfe
2021-06-01  0:38       ` [PATCH] t0001: fix broken not-quite getcwd(3) test in bed67874e2 Ævar Arnfjörð Bjarmason
2021-06-01 16:15         ` René Scharfe
2021-06-01 21:20           ` Junio C Hamano
2021-06-07 11:29             ` Ævar Arnfjörð Bjarmason
2021-06-07 11:24           ` Ævar Arnfjörð Bjarmason
2021-06-07 15:26             ` René Scharfe
2021-06-07 15:38               ` Ævar Arnfjörð Bjarmason
2021-07-30 16:18         ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2021-07-30 17:16           ` Junio C Hamano
2017-08-07  1:33 ` [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt Junio C Hamano
2017-08-07 17:32 ` Junio C Hamano
2017-08-07 18:06 ` Andreas Schwab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).