Re: [PATCH] report which $PATH entry had trouble running execvp(3)

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Tue, Apr 19, 2011 at 09:01:21PM -0700, Junio C Hamano wrote:
>
>> You can add your own custom subcommand 'frotz' to the system by adding
>> 'git-frotz' in a directory somewhere in your $PATH environment variable.
>> When you ask "git frotz" from the command line, "git-frotz" is run via
>> execvp(3).
>> [...]
>> we do not report 'git-frotz' in which directory we had trouble with.
>> We could do better if we implemented the command search behaviour of
>> execvp(3) ourselves.
>
> I like the idea of giving the user more information about which
> git-frotz was the problem. Usually there is just one, and pointing them
> to it saves them time.
>
> But what about the case of
>
>   mkdir one two
>   touch one/frotz two/frotz
>   PATH=one:two:$PATH
>
> We would report two/frotz, but might it be even better to say "we found
> 2 frotzes, but neither of them were executable"?

No, one/frotz is the first one found along $PATH, and we report that we
cannot exec 'one/frotz'.  We are trying to imitate the semantics of the
usual command search done by execvp() and by the shell.  Three possible
user reactions are (1) Huh? I wanted to see two/frotz be used.  My $PATH
is wrong, and I'll fix it by reordering elements on $PATH; (2) Huh? I
wanted to see two/frotz be used. I have a stale one in one/frotz, and I'll
fix it by removing it; and (3) Yuck, I forgot to chmod +x one/frotz.

As J6t mentioned, the user needs to examine what went wrong anyway.  The
point of the change is to make it easier by giving more information than
what execvp(3) gives us (especially there may be hidden GIT_EXEC_PATH that
the user of a third-party scripted Porcelain may not be aware of, which is
tacked before the usual $PATH).

>> Three plausible scenarios that the execvp(3) would fail for us are:
>> 
>>  * The first 'git-frotz' found in a directory on $PATH was not a proper
>>    executable binary, and we got "Exec format error" (ENOEXEC);
>
> What about the magic "unknown things get executed as shell scripts"
> behavior that is implemented by libc's execvp?
> ...
> I have always found that behavior slightly insane, but it is
> well-established, and your sane_execvp breaks anybody who is depending
> on it.

We can choose to add that on top of the sane_execvp() patch.