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!