{"thread":{"id":"31157","subject":"[PATCH/RFC] sane_execvp(): ignore non-directory on PATH","startedAt":"2012-07-31T19:46:13Z","lastAt":"2012-07-31T20:23:40Z","messageCount":2,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196256","messageId":"7vobmviuii.fsf@alter.siamese.dyndns.org","threadId":"31157","inReplyTo":null,"subject":"[PATCH/RFC] sane_execvp(): ignore non-directory on PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-31T19:46:13Z","receivedAt":"2012-07-31T19:46:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have a non-directory on your PATH, a funny thing happens:\n\n\t$ PATH=$PATH:/bin/sh git foo\n\tfatal: cannot exec 'git-foo': Not a directory?\n\nWorse yet, as real commands always take precedence over aliases,\nthis behaviour interacts rather badly with them:\n\n\t$ PATH=$PATH:/bin/sh git -c alias.foo=show git foo -s\n\tfatal: cannot exec 'git-foo': Not a directory?\n\nThis is because an ENOTDIR error from the underlying execvp(2) is\nreported back to the caller of our sane_execvp() wrapper as-is.  By\ntranslating it to ENOENT, just like the case where we _might_ have\nthe command in an unreadable directory, fixes it.  Without an alias,\nwe would get\n\n\tgit: 'foo' is not a git command. See 'git --help'.\n\nand we use the 'foo' alias when it is available.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * We can view this as a follow-up to 38f865c (run-command: treat\n   inaccessible directories as ENOENT, 2012-03-30).\n\n run-command.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 805d41f..f9b7db2 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -77,6 +77,8 @@ int sane_execvp(const char *file, char * const argv[])\n \t */\n \tif (errno == EACCES && !strchr(file, '/'))\n \t\terrno = exists_in_PATH(file) ? EACCES : ENOENT;\n+\telse if (errno == ENOTDIR && !strchr(file, '/'))\n+\t\terrno = ENOENT;\n \treturn -1;\n }\n \n"},{"id":"196263","messageId":"20120731202340.GA31348@sigill.intra.peff.net","threadId":"31157","inReplyTo":"7vobmviuii.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] sane_execvp(): ignore non-directory on PATH","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-31T20:23:40Z","receivedAt":"2012-07-31T20:23:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 31, 2012 at 12:46:13PM -0700, Junio C Hamano wrote:\n\n> When you have a non-directory on your PATH, a funny thing happens:\n> \n> \t$ PATH=$PATH:/bin/sh git foo\n> \tfatal: cannot exec 'git-foo': Not a directory?\n> \n> Worse yet, as real commands always take precedence over aliases,\n> this behaviour interacts rather badly with them:\n> \n> \t$ PATH=$PATH:/bin/sh git -c alias.foo=show git foo -s\n> \tfatal: cannot exec 'git-foo': Not a directory?\n> \n> This is because an ENOTDIR error from the underlying execvp(2) is\n> reported back to the caller of our sane_execvp() wrapper as-is.  By\n> translating it to ENOENT, just like the case where we _might_ have\n> the command in an unreadable directory, fixes it.  Without an alias,\n> we would get\n> \n> \tgit: 'foo' is not a git command. See 'git --help'.\n> \n> and we use the 'foo' alias when it is available.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * We can view this as a follow-up to 38f865c (run-command: treat\n>    inaccessible directories as ENOENT, 2012-03-30).\n\nHrm. EACCES is somewhat special, in that the underlying execvp will\ncontinue after seeing EACCES, and will only report it back to us if we\ndon't eventually find a good candidate.\n\nIs ENOTDIR the same? IOW, If I do:\n\n  PATH=/bin/cat:/bin\n  ls\n\nwill I still run \"ls\"? Testing on my glibc system says \"yes\", which I\nthink makes this a sane thing to do (if it were not the case and ENOTDIR\ncauses an early return, then that ENOENT is kind of a lie, since we\nsimply don't know the answer).\n\n> diff --git a/run-command.c b/run-command.c\n> index 805d41f..f9b7db2 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -77,6 +77,8 @@ int sane_execvp(const char *file, char * const argv[])\n>  \t */\n>  \tif (errno == EACCES && !strchr(file, '/'))\n>  \t\terrno = exists_in_PATH(file) ? EACCES : ENOENT;\n> +\telse if (errno == ENOTDIR && !strchr(file, '/'))\n> +\t\terrno = ENOENT;\n>  \treturn -1;\n\nYay. I remember the original 38f865c going through several iterations,\nand I am glad we took the time to end up with one that made adding this\ncase in so simple.\n\n-Peff\n"}]}