Re: [PATCH 1/2] run-command: propagate EACCES errors to parent
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 27, 2012, 18:24 UTC
- Message-ID
- <7vhax9j41p.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120327180425.GA4659@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> The caller of run_command does not directly get access to > the errno from exec, because it happens in the forked child. > However, knowing the specific reason for an exec failure can > help the parent respond better or produce better error > messages. > > We already propagate ENOENT to the parent via exit code 127. > Let's do the same for EACCES with exit code 126, which is > already used by bash to indicate the same thing. > > Signed-off-by: Jeff King <peff@peff.net> > --- > Actually, there is a slight bending of the truth in the commit message. > bash implements its own execvp, and it will only return 126/EACCES if a > file is found via stat(), but is not executable. If there is an > inaccessible directory in the PATH (meaning that stat() will fail), it > will silently convert that to 127/ENOENT.
I am wondering what would happen if we treated EACCESS and ENOENT exactly the same way. Wouldn't the four breakage scenarios in the cover letter end up being even better? Case (3) will still say does-not-exist is not a git command (instead of "permission denied", which this patch gives), but your case (2) will see a much better diagnosis.
Take the above with a grain of salt, though, as this is written soon after I wrote my response to James (the one with "I may be a mean person").
Show 30 quoted lines
> run-command.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/run-command.c b/run-command.c
> index 1db8abf..e303beb 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -185,6 +185,10 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
> code = -1;
> failed_errno = ENOENT;
> }
> + else if (code == 126) {
> + code = -1;
> + failed_errno = EACCES;
> + }
> } else {
> error("waitpid is confused (%s)", argv0);
> }
> @@ -346,6 +350,11 @@ fail_pipe:
> error("cannot run %s: %s", cmd->argv[0],
> strerror(ENOENT));
> exit(127);
> + } else if (errno == EACCES) {
> + if (!cmd->silent_exec_failure)
> + error("cannot run %s: %s", cmd->argv[0],
> + strerror(errno));
> + exit(126);
> } else {
> die_errno("cannot exec '%s'", cmd->argv[0]);
> }