[PATCH 3/5] run-command: Elaborate execvp error checking

[Frans Klaver <fransklaver@gmail.com>]

The interpretation of errors from execvp was rather terse. For user
convenience communication of the nature of the error can be improved.

This patch introduces a function with more elaborate investigation of
the errors encountered. After a failure occurs on a program or script,
inspect_failure will run through the PATH entries if necessary, and if
the file exists, perform some tests on the file. If nothing is found,
ENOENT is returned.

The function inspect_file() will try to find out if the file itself or
an interpreter caused the issue. Some scripts don't have the hash-bang
and will not be reported as scripts. If the interpreter cannot be found,
the existing behavior is kept -- i.e. ENOENT will be reported to the
parent process.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 run-command.c          |  144 +++++++++++++++++++++++++++++++++++++++++++++---
 t/t0061-run-command.sh |    6 +-
 2 files changed, 140 insertions(+), 10 deletions(-)

diff --git a/run-command.c b/run-command.c
index 1c51043..17a65fe 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "argv-array.h"
+#include "dir.h"
 
 static inline void close_pair(int fd[2])
 {
@@ -134,6 +135,140 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
 	return code;
 }
 
+#ifndef WIN32
+static int is_executable_file(const char *path)
+{
+	return access(path, X_OK) == 0 && !is_directory(path);
+}
+
+static int is_searchable(const char *path)
+{
+	return access(path, X_OK) == 0 && is_directory(path);
+}
+
+static void die_file_error(const char *file, int err)
+{
+	die("cannot exec '%s': %s", file, strerror(err));
+}
+
+static char *get_interpreter(const char *first_line)
+{
+	struct strbuf sb = STRBUF_INIT;
+	size_t start = strspn(first_line + 2, " \t") + 2;
+	size_t end = strcspn(first_line + start, " \t\r\n") + start;
+
+	if (start >= end)
+		return NULL;
+
+	strbuf_add(&sb, first_line + start, end - start);
+	return strbuf_detach(&sb, NULL);
+}
+
+static void inspect_file(struct strbuf *fn, int err, const char *argv0)
+{
+	/*
+	 * Typical line length of 80. BSD only allows 32 bytes, but that
+	 * won't really make a difference.
+	 */
+	char buf[80];
+	size_t read;
+	FILE *file;
+	if (!is_executable_file(fn->buf)) {
+		strbuf_release(fn);
+		die_file_error(argv0, err);
+	}
+
+	file = fopen(fn->buf, "r");
+	if (!file) {
+		strbuf_release(fn);
+		die_file_error(argv0, err);
+	}
+
+	read = fread(buf, 1, sizeof(buf), file);
+	fclose(file);
+	if (read > 2 && buf[0] == '#' && buf[1] == '!') {
+		char *i = get_interpreter(buf);
+		char es[1024];
+		if (i) {
+			sprintf(es, "cannot exec '%s': "
+					"bad interpreter '%s': %s",
+					argv0, i,
+					strerror(err));
+		} else {
+			sprintf(es, "cannot exec '%s': "
+					"bad interpreter: %s",
+					argv0, strerror(err));
+		}
+		free(i);
+		strbuf_release(fn);
+		if (err == ENOENT) {
+			error("%s", es);
+			exit(127);
+		} else {
+			die("%s", es);
+		}
+	}
+
+	strbuf_release(fn);
+	die_file_error(argv0, err);
+}
+
+static void inspect_failure(const char *argv0, int silent_exec_failure)
+{
+	int err = errno;
+	struct strbuf sb = STRBUF_INIT;
+
+	/* errors not related to path */
+	if (errno == E2BIG || errno == ENOMEM)
+		die_file_error(argv0, err);
+
+	if (strchr(argv0, '/')) {
+		if (file_exists(argv0)) {
+			strbuf_add(&sb, argv0, strlen(argv0));
+			inspect_file(&sb, err, argv0);
+		}
+	} else {
+		char *path, *next;
+		path = getenv("PATH");
+		while (path) {
+			next = strchrnul(path, ':');
+			if (path < next)
+				strbuf_add(&sb, path, next - path);
+			else
+				strbuf_addch(&sb, '.');
+
+			if (!*next)
+				path = NULL;
+			else
+				path = next + 1;
+
+			if (!is_searchable(sb.buf)) {
+				strbuf_release(&sb);
+				continue;
+			}
+
+			if (sb.len && sb.buf[sb.len - 1] != '/')
+				strbuf_addch(&sb, '/');
+			strbuf_addstr(&sb, argv0);
+
+			if (file_exists(sb.buf))
+				inspect_file(&sb, err, argv0);
+
+			strbuf_release(&sb);
+		}
+	}
+
+	if (err == ENOENT) {
+		if (!silent_exec_failure)
+			error("cannot exec '%s': %s", argv0,
+					strerror(ENOENT));
+		exit(127);
+	} else {
+		die_file_error(argv0, err);
+	}
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
@@ -280,14 +415,7 @@ fail_pipe:
 		} else {
 			execvp(cmd->argv[0], (char *const*) cmd->argv);
 		}
-		if (errno == ENOENT) {
-			if (!cmd->silent_exec_failure)
-				error("cannot run %s: %s", cmd->argv[0],
-					strerror(ENOENT));
-			exit(127);
-		} else {
-			die_errno("cannot exec '%s'", cmd->argv[0]);
-		}
+		inspect_failure(cmd->argv[0], cmd->silent_exec_failure);
 	}
 	if (cmd->pid < 0)
 		error("cannot fork() for %s: %s", cmd->argv[0],
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 31eb3c3..14b4ea6 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -71,7 +71,8 @@ test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
 	chmod -x someinterpreter &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&
 
-	grep "fatal: cannot exec.*hello.sh" err
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
 '
 
 test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
@@ -79,7 +80,8 @@ test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
 	chmod +x hello.sh &&
 	test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err &&
 
-	grep "error: cannot exec.*hello.sh" err
+	grep "error: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
 '
 
 test_done
-- 
1.7.8.1