Re: [PATCH] run-command.c: Accept EACCES as command not found

On Mon, 21 Nov 2011 23:13:58 +0100, Junio C Hamano [off-list ref]  
wrote:

Frans Klaver [off-list ref] writes:

quoted

execvp returns ENOENT if a command was not found after searching PATH.
If path contains a directory that current user has insufficient
privileges to, EACCES is returned. This may still mean the program
wasn't found.

If the latter case is encountered, git errors out without giving aliases
a try,...

Isn't that a *good* thing in general, though, so that the user can
diagnose the breakage in the $PATH and fix it?

Actually I went through diagnosing and fixing it. After tracking it down,  
I did wonder about this question myself and I didn't come to a definitive  
conclusion on it. On one hand I do agree that it may be an incentive for  
the user to fix his path. On the other hand I found it an obscure one to  
track down; git's behavior doesn't match bash behavior:

$ git config --global alias.aliasedinit init &&
mkdir searchpath && chmod 400 searchpath && PATH=$(pwd)/searchpath:$PATH  
&& export PATH &&
mkdir someproject && cd someproject &&
git aliasedinit
fatal: cannot exec 'git-aliasedinit': Permission denied

$ git-aliasedinit
bash: git-aliasedinit: command not found

This isn't very intuitive to track down an incorrect PATH with, imo. You  
have to dig into git core code, learn about how git handles commands,  
learn about debugging forked processes, and find out that execvp uses  
EACCES for more than just "permission denied" _just_ to find out you've  
got a wrong environment variable lying about. That's a full day of work  
gone for a newbie. If bash would also tell me in natural language that  
permission was denied, I wouldn't even have considered doing this patch.

For my part the root of the problem lies with the use of EACCES by execvp  
here, not so much with how git uses it. For this particular case, EACCES  
doesn't just mean "it exists, but you cannot execute this", it may also  
mean "not found, but one of the paths could not be accessed". If git were  
to provide a really helpful message, we'd have to detect which paths got  
denied. Once we know that, we can even on the spot decide to error out or  
not. In other words, we'd have to figure out which meaning of EACCES is  
actually used. Based on that, git can error out, warn or ignore at will.

In any case, I thought it best to have other developers have a look at it.  
I can put a bit more of that information in the commit message, but I'd be  
just as happy to drop the patch and keep the exercise.

Frans