summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2010-01-20 14:44:12 -0800
committerJunio C Hamano <gitster@pobox.com>2010-01-20 14:44:12 -0800
commit030b1a77f72a7e3307c7d7881ae570ca1c8ed877 (patch)
tree1c25e1ee45851d02c59bb5e420fd39f4c51520f6
parent3af59e6f31c5304d476884b69b6b88dfd492812b (diff)
parent6b02de3b9dc4ac8374cea4964e993ec6636d781c (diff)
downloadgit-030b1a77f72a7e3307c7d7881ae570ca1c8ed877.tar.gz
Merge branch 'js/exec-error-report'
* js/exec-error-report: Improve error message when a transport helper was not found start_command: detect execvp failures early run-command: move wait_or_whine earlier start_command: report child process setup errors to the parent's stderr Conflicts: Makefile
-rw-r--r--Makefile1
-rw-r--r--run-command.c177
-rwxr-xr-xt/t0061-run-command.sh14
-rw-r--r--test-run-command.c35
-rw-r--r--transport-helper.c14
5 files changed, 191 insertions, 50 deletions
diff --git a/Makefile b/Makefile
index d74c196234..cf0723ea12 100644
--- a/Makefile
+++ b/Makefile
@@ -1778,6 +1778,7 @@ TEST_PROGRAMS_NEED_X += test-genrandom
TEST_PROGRAMS_NEED_X += test-match-trees
TEST_PROGRAMS_NEED_X += test-parse-options
TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-run-command
TEST_PROGRAMS_NEED_X += test-sha1
TEST_PROGRAMS_NEED_X += test-sigchain
TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/run-command.c b/run-command.c
index a909845764..2feb493951 100644
--- a/run-command.c
+++ b/run-command.c
@@ -61,6 +61,78 @@ static int execv_shell_cmd(const char **argv)
}
#endif
+#ifndef WIN32
+static int child_err = 2;
+static int child_notifier = -1;
+
+static void notify_parent(void)
+{
+ write(child_notifier, "", 1);
+}
+
+static NORETURN void die_child(const char *err, va_list params)
+{
+ char msg[4096];
+ int len = vsnprintf(msg, sizeof(msg), err, params);
+ if (len > sizeof(msg))
+ len = sizeof(msg);
+
+ write(child_err, "fatal: ", 7);
+ write(child_err, msg, len);
+ write(child_err, "\n", 1);
+ exit(128);
+}
+
+static inline void set_cloexec(int fd)
+{
+ int flags = fcntl(fd, F_GETFD);
+ if (flags >= 0)
+ fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+}
+#endif
+
+static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
+{
+ int status, code = -1;
+ pid_t waiting;
+ int failed_errno = 0;
+
+ while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
+ ; /* nothing */
+
+ if (waiting < 0) {
+ failed_errno = errno;
+ error("waitpid for %s failed: %s", argv0, strerror(errno));
+ } else if (waiting != pid) {
+ error("waitpid is confused (%s)", argv0);
+ } else if (WIFSIGNALED(status)) {
+ code = WTERMSIG(status);
+ error("%s died of signal %d", argv0, code);
+ /*
+ * This return value is chosen so that code & 0xff
+ * mimics the exit code that a POSIX shell would report for
+ * a program that died from this signal.
+ */
+ code -= 128;
+ } else if (WIFEXITED(status)) {
+ code = WEXITSTATUS(status);
+ /*
+ * Convert special exit code when execvp failed.
+ */
+ if (code == 127) {
+ code = -1;
+ failed_errno = ENOENT;
+ if (!silent_exec_failure)
+ error("cannot run %s: %s", argv0,
+ strerror(ENOENT));
+ }
+ } else {
+ error("waitpid is confused (%s)", argv0);
+ }
+ errno = failed_errno;
+ return code;
+}
+
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@@ -122,9 +194,30 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
#ifndef WIN32
+{
+ int notify_pipe[2];
+ if (pipe(notify_pipe))
+ notify_pipe[0] = notify_pipe[1] = -1;
+
fflush(NULL);
cmd->pid = fork();
if (!cmd->pid) {
+ /*
+ * Redirect the channel to write syscall error messages to
+ * before redirecting the process's stderr so that all die()
+ * in subsequent call paths use the parent's stderr.
+ */
+ if (cmd->no_stderr || need_err) {
+ child_err = dup(2);
+ set_cloexec(child_err);
+ }
+ set_die_routine(die_child);
+
+ close(notify_pipe[0]);
+ set_cloexec(notify_pipe[1]);
+ child_notifier = notify_pipe[1];
+ atexit(notify_parent);
+
if (cmd->no_stdin)
dup_devnull(0);
else if (need_in) {
@@ -165,8 +258,16 @@ fail_pipe:
unsetenv(*cmd->env);
}
}
- if (cmd->preexec_cb)
+ if (cmd->preexec_cb) {
+ /*
+ * We cannot predict what the pre-exec callback does.
+ * Forgo parent notification.
+ */
+ close(child_notifier);
+ child_notifier = -1;
+
cmd->preexec_cb();
+ }
if (cmd->git_cmd) {
execv_git_cmd(cmd->argv);
} else if (cmd->use_shell) {
@@ -174,13 +275,39 @@ fail_pipe:
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
- trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
- strerror(errno));
- exit(127);
+ /*
+ * Do not check for cmd->silent_exec_failure; the parent
+ * process will check it when it sees this exit code.
+ */
+ if (errno == ENOENT)
+ exit(127);
+ else
+ die_errno("cannot exec '%s'", cmd->argv[0]);
}
if (cmd->pid < 0)
error("cannot fork() for %s: %s", cmd->argv[0],
strerror(failed_errno = errno));
+
+ /*
+ * Wait for child's execvp. If the execvp succeeds (or if fork()
+ * failed), EOF is seen immediately by the parent. Otherwise, the
+ * child process sends a single byte.
+ * Note that use of this infrastructure is completely advisory,
+ * therefore, we keep error checks minimal.
+ */
+ close(notify_pipe[1]);
+ if (read(notify_pipe[0], &notify_pipe[1], 1) == 1) {
+ /*
+ * At this point we know that fork() succeeded, but execvp()
+ * failed. Errors have been reported to our stderr.
+ */
+ wait_or_whine(cmd->pid, cmd->argv[0],
+ cmd->silent_exec_failure);
+ failed_errno = errno;
+ cmd->pid = -1;
+ }
+ close(notify_pipe[0]);
+}
#else
{
int fhin = 0, fhout = 1, fherr = 2;
@@ -271,48 +398,6 @@ fail_pipe:
return 0;
}
-static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
-{
- int status, code = -1;
- pid_t waiting;
- int failed_errno = 0;
-
- while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
- ; /* nothing */
-
- if (waiting < 0) {
- failed_errno = errno;
- error("waitpid for %s failed: %s", argv0, strerror(errno));
- } else if (waiting != pid) {
- error("waitpid is confused (%s)", argv0);
- } else if (WIFSIGNALED(status)) {
- code = WTERMSIG(status);
- error("%s died of signal %d", argv0, code);
- /*
- * This return value is chosen so that code & 0xff
- * mimics the exit code that a POSIX shell would report for
- * a program that died from this signal.
- */
- code -= 128;
- } else if (WIFEXITED(status)) {
- code = WEXITSTATUS(status);
- /*
- * Convert special exit code when execvp failed.
- */
- if (code == 127) {
- code = -1;
- failed_errno = ENOENT;
- if (!silent_exec_failure)
- error("cannot run %s: %s", argv0,
- strerror(ENOENT));
- }
- } else {
- error("waitpid is confused (%s)", argv0);
- }
- errno = failed_errno;
- return code;
-}
-
int finish_command(struct child_process *cmd)
{
return wait_or_whine(cmd->pid, cmd->argv[0], cmd->silent_exec_failure);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
new file mode 100755
index 0000000000..10b26e4d8e
--- /dev/null
+++ b/t/t0061-run-command.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Ilari Liusvaara
+#
+
+test_description='Test run command'
+
+. ./test-lib.sh
+
+test_expect_success 'start_command reports ENOENT' '
+ test-run-command start-command-ENOENT ./does-not-exist
+'
+
+test_done
diff --git a/test-run-command.c b/test-run-command.c
new file mode 100644
index 0000000000..0612bfa7cd
--- /dev/null
+++ b/test-run-command.c
@@ -0,0 +1,35 @@
+/*
+ * test-run-command.c: test run command API.
+ *
+ * (C) 2009 Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
+ *
+ * This code is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "git-compat-util.h"
+#include "run-command.h"
+#include <string.h>
+#include <errno.h>
+
+int main(int argc, char **argv)
+{
+ struct child_process proc;
+
+ memset(&proc, 0, sizeof(proc));
+
+ if (argc < 3)
+ return 1;
+ proc.argv = (const char **)argv+2;
+
+ if (!strcmp(argv[1], "start-command-ENOENT")) {
+ if (start_command(&proc) < 0 && errno == ENOENT)
+ return 0;
+ fprintf(stderr, "FAIL %s\n", argv[1]);
+ return 1;
+ }
+
+ fprintf(stderr, "check usage\n");
+ return 1;
+}
diff --git a/transport-helper.c b/transport-helper.c
index fdf2256220..107742891f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -102,6 +102,7 @@ static struct child_process *get_helper(struct transport *transport)
int refspec_nr = 0;
int refspec_alloc = 0;
int duped;
+ int code;
if (data->helper)
return data->helper;
@@ -111,13 +112,18 @@ static struct child_process *get_helper(struct transport *transport)
helper->out = -1;
helper->err = 0;
helper->argv = xcalloc(4, sizeof(*helper->argv));
- strbuf_addf(&buf, "remote-%s", data->name);
+ strbuf_addf(&buf, "git-remote-%s", data->name);
helper->argv[0] = strbuf_detach(&buf, NULL);
helper->argv[1] = transport->remote->name;
helper->argv[2] = remove_ext_force(transport->url);
- helper->git_cmd = 1;
- if (start_command(helper))
- die("Unable to run helper: git %s", helper->argv[0]);
+ helper->git_cmd = 0;
+ helper->silent_exec_failure = 1;
+ code = start_command(helper);
+ if (code < 0 && errno == ENOENT)
+ die("Unable to find remote helper for '%s'", data->name);
+ else if (code != 0)
+ exit(code);
+
data->helper = helper;
data->no_disconnect_req = 0;