Re: [REGRESSION] git-wrapper to run-commands codepath regression

Junio C Hamano [off-list ref] writes:

There appears to be a regression in the codepath between git wrapper and
run_commands API.

	$ T=/var/tmp/test-commands
	$ mkdir $T
	$ cat >$T/git-hello <<\-EOF
	#!/bin/sh
	echo hello
	EOF
	$ chmod +x $T/git-hello
	$ oPATH=$PATH
	$ PATH=$T:$PATH
	$ export PATH
	$ git hello
	hello

So far, I added a "hello" subcommand to "git", and it runs correctly.

...
Note that we can observe the same regression if you instead make $T
unreadable with:

	$ chmod 755 $T/git-hello ;# make it executable again
	$ chmod a-rwx $T ;# but that directory cannot be read
	$ git hello

So that is the "regression" part.

The following is a tangent that was brought up at $work.

And that is the topic of this follow-up.

Some people might argue that we should skip $T/git-hello in the last case
and try to find git-hello in a later directory listed in $PATH, but I do
not personally think that is a right thing to do.  It would make the
problem harder to diagnose, and more importantly, the fact that the user
listed $T earlier in the $PATH is a strong indication that the user wants
the scripts in $T override the scripts with the same name in directories
that appear later in the $PATH, and we should report when that is not
happening, either

 (1) when $T/git-hello was found but was not executable; or

 (2) when we cannot read $T and we cannot even tell $T/git-hello exists or
     not.

So I think it is Ok to be silent only when we see ENOENT like the current
code does.

Unfortunately, as we are giving "git-hello" to underlying execvp(3),
instead of splitting $PATH, prefixing each element of it in front of
"git-hello" and trying execv() in a loop, when the call fails, we do not
know which component of $PATH caused execvp() to fail, and end up saying
"fatal: cannot exec 'git-hello': Permission denied".  So in that sense, we
are not helping the end user by making it easier to diagnose the problem.

We would need to emulate what execvp() does ourselves (i.e. split $PATH,
prefix each component and try execv(), ignoring ENOENT or EACCES while
trying next component in $PATH), plus note the first path that got EACCES
so that we can report which script (including its leading directories) had
trouble executing.  Perhaps a simple enough task for beginners.