Merge branch 'bw/forking-and-threading' into maint
authorJunio C Hamano <gitster@pobox.com>
Tue, 13 Jun 2017 20:26:59 +0000 (13:26 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 13 Jun 2017 20:27:00 +0000 (13:27 -0700)
The "run-command" API implementation has been made more robust
against dead-locking in a threaded environment.

* bw/forking-and-threading:
usage.c: drop set_error_handle()
run-command: restrict PATH search to executable files
run-command: expose is_executable function
run-command: block signals between fork and execve
run-command: add note about forking and threading
run-command: handle dup2 and close errors in child
run-command: eliminate calls to error handling functions in child
run-command: don't die in child when duping /dev/null
run-command: prepare child environment before forking
string-list: add string_list_remove function
run-command: use the async-signal-safe execv instead of execvp
run-command: prepare command before forking
t0061: run_command executes scripts without a #! line
t5550: use write_script to generate post-update hook

git-compat-util.h
help.c
run-command.c
run-command.h
string-list.c
string-list.h
t/t0061-run-command.sh
t/t5550-http-fetch-dumb.sh
usage.c
index 4575b3890b22599d147211576d2f1ea6e7c6833e..199042ac91466783201bdd483bad9f89c78db122 100644 (file)
@@ -445,7 +445,6 @@ extern void (*get_error_routine(void))(const char *err, va_list params);
 extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
 extern void (*get_warn_routine(void))(const char *warn, va_list params);
 extern void set_die_is_recursing_routine(int (*routine)(void));
-extern void set_error_handle(FILE *);
 
 extern int starts_with(const char *str, const char *prefix);
 
diff --git a/help.c b/help.c
index a07f01e6f9fc7895f27f3ee9a4a7e291ce995b0f..db7f3d79a016881639a8c0640451afe35b011e5e 100644 (file)
--- a/help.c
+++ b/help.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
+#include "run-command.h"
 #include "levenshtein.h"
 #include "help.h"
 #include "common-cmds.h"
@@ -96,48 +97,6 @@ static void pretty_print_cmdnames(struct cmdnames *cmds, unsigned int colopts)
        string_list_clear(&list, 0);
 }
 
-static int is_executable(const char *name)
-{
-       struct stat st;
-
-       if (stat(name, &st) || /* stat, not lstat */
-           !S_ISREG(st.st_mode))
-               return 0;
-
-#if defined(GIT_WINDOWS_NATIVE)
-       /*
-        * On Windows there is no executable bit. The file extension
-        * indicates whether it can be run as an executable, and Git
-        * has special-handling to detect scripts and launch them
-        * through the indicated script interpreter. We test for the
-        * file extension first because virus scanners may make
-        * it quite expensive to open many files.
-        */
-       if (ends_with(name, ".exe"))
-               return S_IXUSR;
-
-{
-       /*
-        * Now that we know it does not have an executable extension,
-        * peek into the file instead.
-        */
-       char buf[3] = { 0 };
-       int n;
-       int fd = open(name, O_RDONLY);
-       st.st_mode &= ~S_IXUSR;
-       if (fd >= 0) {
-               n = read(fd, buf, 2);
-               if (n == 2)
-                       /* look for a she-bang */
-                       if (!strcmp(buf, "#!"))
-                               st.st_mode |= S_IXUSR;
-               close(fd);
-       }
-}
-#endif
-       return st.st_mode & S_IXUSR;
-}
-
 static void list_commands_in_dir(struct cmdnames *cmds,
                                         const char *path,
                                         const char *prefix)
index 574b81d3e82bbe6de31b141c3951ab408daad5e6..9e36151bf97d36945ca3464e719687718ee6d1c7 100644 (file)
@@ -117,18 +117,65 @@ static inline void close_pair(int fd[2])
        close(fd[1]);
 }
 
-#ifndef GIT_WINDOWS_NATIVE
-static inline void dup_devnull(int to)
+int is_executable(const char *name)
 {
-       int fd = open("/dev/null", O_RDWR);
-       if (fd < 0)
-               die_errno(_("open /dev/null failed"));
-       if (dup2(fd, to) < 0)
-               die_errno(_("dup2(%d,%d) failed"), fd, to);
-       close(fd);
+       struct stat st;
+
+       if (stat(name, &st) || /* stat, not lstat */
+           !S_ISREG(st.st_mode))
+               return 0;
+
+#if defined(GIT_WINDOWS_NATIVE)
+       /*
+        * On Windows there is no executable bit. The file extension
+        * indicates whether it can be run as an executable, and Git
+        * has special-handling to detect scripts and launch them
+        * through the indicated script interpreter. We test for the
+        * file extension first because virus scanners may make
+        * it quite expensive to open many files.
+        */
+       if (ends_with(name, ".exe"))
+               return S_IXUSR;
+
+{
+       /*
+        * Now that we know it does not have an executable extension,
+        * peek into the file instead.
+        */
+       char buf[3] = { 0 };
+       int n;
+       int fd = open(name, O_RDONLY);
+       st.st_mode &= ~S_IXUSR;
+       if (fd >= 0) {
+               n = read(fd, buf, 2);
+               if (n == 2)
+                       /* look for a she-bang */
+                       if (!strcmp(buf, "#!"))
+                               st.st_mode |= S_IXUSR;
+               close(fd);
+       }
 }
 #endif
+       return st.st_mode & S_IXUSR;
+}
 
+/*
+ * Search $PATH for a command.  This emulates the path search that
+ * execvp would perform, without actually executing the command so it
+ * can be used before fork() to prepare to run a command using
+ * execve() or after execvp() to diagnose why it failed.
+ *
+ * The caller should ensure that file contains no directory
+ * separators.
+ *
+ * Returns the path to the command, as found in $PATH or NULL if the
+ * command could not be found.  The caller inherits ownership of the memory
+ * used to store the resultant path.
+ *
+ * This should not be used on Windows, where the $PATH search rules
+ * are more complicated (e.g., a search for "foo" should find
+ * "foo.exe").
+ */
 static char *locate_in_PATH(const char *file)
 {
        const char *p = getenv("PATH");
@@ -149,7 +196,7 @@ static char *locate_in_PATH(const char *file)
                }
                strbuf_addstr(&buf, file);
 
-               if (!access(buf.buf, F_OK))
+               if (is_executable(buf.buf))
                        return strbuf_detach(&buf, NULL);
 
                if (!*end)
@@ -221,31 +268,248 @@ static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
 }
 
 #ifndef GIT_WINDOWS_NATIVE
-static int execv_shell_cmd(const char **argv)
+static int child_notifier = -1;
+
+enum child_errcode {
+       CHILD_ERR_CHDIR,
+       CHILD_ERR_DUP2,
+       CHILD_ERR_CLOSE,
+       CHILD_ERR_SIGPROCMASK,
+       CHILD_ERR_ENOENT,
+       CHILD_ERR_SILENT,
+       CHILD_ERR_ERRNO
+};
+
+struct child_err {
+       enum child_errcode err;
+       int syserr; /* errno */
+};
+
+static void child_die(enum child_errcode err)
 {
-       struct argv_array nargv = ARGV_ARRAY_INIT;
-       prepare_shell_cmd(&nargv, argv);
-       trace_argv_printf(nargv.argv, "trace: exec:");
-       sane_execvp(nargv.argv[0], (char **)nargv.argv);
-       argv_array_clear(&nargv);
-       return -1;
+       struct child_err buf;
+
+       buf.err = err;
+       buf.syserr = errno;
+
+       /* write(2) on buf smaller than PIPE_BUF (min 512) is atomic: */
+       xwrite(child_notifier, &buf, sizeof(buf));
+       _exit(1);
 }
-#endif
 
-#ifndef GIT_WINDOWS_NATIVE
-static int child_notifier = -1;
+static void child_dup2(int fd, int to)
+{
+       if (dup2(fd, to) < 0)
+               child_die(CHILD_ERR_DUP2);
+}
 
-static void notify_parent(void)
+static void child_close(int fd)
 {
+       if (close(fd))
+               child_die(CHILD_ERR_CLOSE);
+}
+
+static void child_close_pair(int fd[2])
+{
+       child_close(fd[0]);
+       child_close(fd[1]);
+}
+
+/*
+ * parent will make it look like the child spewed a fatal error and died
+ * this is needed to prevent changes to t0061.
+ */
+static void fake_fatal(const char *err, va_list params)
+{
+       vreportf("fatal: ", err, params);
+}
+
+static void child_error_fn(const char *err, va_list params)
+{
+       const char msg[] = "error() should not be called in child\n";
+       xwrite(2, msg, sizeof(msg) - 1);
+}
+
+static void child_warn_fn(const char *err, va_list params)
+{
+       const char msg[] = "warn() should not be called in child\n";
+       xwrite(2, msg, sizeof(msg) - 1);
+}
+
+static void NORETURN child_die_fn(const char *err, va_list params)
+{
+       const char msg[] = "die() should not be called in child\n";
+       xwrite(2, msg, sizeof(msg) - 1);
+       _exit(2);
+}
+
+/* this runs in the parent process */
+static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
+{
+       static void (*old_errfn)(const char *err, va_list params);
+
+       old_errfn = get_error_routine();
+       set_error_routine(fake_fatal);
+       errno = cerr->syserr;
+
+       switch (cerr->err) {
+       case CHILD_ERR_CHDIR:
+               error_errno("exec '%s': cd to '%s' failed",
+                           cmd->argv[0], cmd->dir);
+               break;
+       case CHILD_ERR_DUP2:
+               error_errno("dup2() in child failed");
+               break;
+       case CHILD_ERR_CLOSE:
+               error_errno("close() in child failed");
+               break;
+       case CHILD_ERR_SIGPROCMASK:
+               error_errno("sigprocmask failed restoring signals");
+               break;
+       case CHILD_ERR_ENOENT:
+               error_errno("cannot run %s", cmd->argv[0]);
+               break;
+       case CHILD_ERR_SILENT:
+               break;
+       case CHILD_ERR_ERRNO:
+               error_errno("cannot exec '%s'", cmd->argv[0]);
+               break;
+       }
+       set_error_routine(old_errfn);
+}
+
+static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
+{
+       if (!cmd->argv[0])
+               die("BUG: command is empty");
+
+       /*
+        * Add SHELL_PATH so in the event exec fails with ENOEXEC we can
+        * attempt to interpret the command with 'sh'.
+        */
+       argv_array_push(out, SHELL_PATH);
+
+       if (cmd->git_cmd) {
+               argv_array_push(out, "git");
+               argv_array_pushv(out, cmd->argv);
+       } else if (cmd->use_shell) {
+               prepare_shell_cmd(out, cmd->argv);
+       } else {
+               argv_array_pushv(out, cmd->argv);
+       }
+
        /*
-        * execvp failed.  If possible, we'd like to let start_command
-        * know, so failures like ENOENT can be handled right away; but
-        * otherwise, finish_command will still report the error.
+        * If there are no '/' characters in the command then perform a path
+        * lookup and use the resolved path as the command to exec.  If there
+        * are no '/' characters or if the command wasn't found in the path,
+        * have exec attempt to invoke the command directly.
         */
-       xwrite(child_notifier, "", 1);
+       if (!strchr(out->argv[1], '/')) {
+               char *program = locate_in_PATH(out->argv[1]);
+               if (program) {
+                       free((char *)out->argv[1]);
+                       out->argv[1] = program;
+               }
+       }
+}
+
+static char **prep_childenv(const char *const *deltaenv)
+{
+       extern char **environ;
+       char **childenv;
+       struct string_list env = STRING_LIST_INIT_DUP;
+       struct strbuf key = STRBUF_INIT;
+       const char *const *p;
+       int i;
+
+       /* Construct a sorted string list consisting of the current environ */
+       for (p = (const char *const *) environ; p && *p; p++) {
+               const char *equals = strchr(*p, '=');
+
+               if (equals) {
+                       strbuf_reset(&key);
+                       strbuf_add(&key, *p, equals - *p);
+                       string_list_append(&env, key.buf)->util = (void *) *p;
+               } else {
+                       string_list_append(&env, *p)->util = (void *) *p;
+               }
+       }
+       string_list_sort(&env);
+
+       /* Merge in 'deltaenv' with the current environ */
+       for (p = deltaenv; p && *p; p++) {
+               const char *equals = strchr(*p, '=');
+
+               if (equals) {
+                       /* ('key=value'), insert or replace entry */
+                       strbuf_reset(&key);
+                       strbuf_add(&key, *p, equals - *p);
+                       string_list_insert(&env, key.buf)->util = (void *) *p;
+               } else {
+                       /* otherwise ('key') remove existing entry */
+                       string_list_remove(&env, *p, 0);
+               }
+       }
+
+       /* Create an array of 'char *' to be used as the childenv */
+       childenv = xmalloc((env.nr + 1) * sizeof(char *));
+       for (i = 0; i < env.nr; i++)
+               childenv[i] = env.items[i].util;
+       childenv[env.nr] = NULL;
+
+       string_list_clear(&env, 0);
+       strbuf_release(&key);
+       return childenv;
+}
+
+struct atfork_state {
+#ifndef NO_PTHREADS
+       int cs;
+#endif
+       sigset_t old;
+};
+
+#ifndef NO_PTHREADS
+static void bug_die(int err, const char *msg)
+{
+       if (err) {
+               errno = err;
+               die_errno("BUG: %s", msg);
+       }
 }
 #endif
 
+static void atfork_prepare(struct atfork_state *as)
+{
+       sigset_t all;
+
+       if (sigfillset(&all))
+               die_errno("sigfillset");
+#ifdef NO_PTHREADS
+       if (sigprocmask(SIG_SETMASK, &all, &as->old))
+               die_errno("sigprocmask");
+#else
+       bug_die(pthread_sigmask(SIG_SETMASK, &all, &as->old),
+               "blocking all signals");
+       bug_die(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &as->cs),
+               "disabling cancellation");
+#endif
+}
+
+static void atfork_parent(struct atfork_state *as)
+{
+#ifdef NO_PTHREADS
+       if (sigprocmask(SIG_SETMASK, &as->old, NULL))
+               die_errno("sigprocmask");
+#else
+       bug_die(pthread_setcancelstate(as->cs, NULL),
+               "re-enabling cancellation");
+       bug_die(pthread_sigmask(SIG_SETMASK, &as->old, NULL),
+               "restoring signal mask");
+#endif
+}
+#endif /* GIT_WINDOWS_NATIVE */
+
 static inline void set_cloexec(int fd)
 {
        int flags = fcntl(fd, F_GETFD);
@@ -281,13 +545,6 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_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;
-               }
        } else {
                error("waitpid is confused (%s)", argv0);
        }
@@ -372,109 +629,149 @@ int start_command(struct child_process *cmd)
 #ifndef GIT_WINDOWS_NATIVE
 {
        int notify_pipe[2];
+       int null_fd = -1;
+       char **childenv;
+       struct argv_array argv = ARGV_ARRAY_INIT;
+       struct child_err cerr;
+       struct atfork_state as;
+
        if (pipe(notify_pipe))
                notify_pipe[0] = notify_pipe[1] = -1;
 
+       if (cmd->no_stdin || cmd->no_stdout || cmd->no_stderr) {
+               null_fd = open("/dev/null", O_RDWR | O_CLOEXEC);
+               if (null_fd < 0)
+                       die_errno(_("open /dev/null failed"));
+               set_cloexec(null_fd);
+       }
+
+       prepare_cmd(&argv, cmd);
+       childenv = prep_childenv(cmd->env);
+       atfork_prepare(&as);
+
+       /*
+        * NOTE: In order to prevent deadlocking when using threads special
+        * care should be taken with the function calls made in between the
+        * fork() and exec() calls.  No calls should be made to functions which
+        * require acquiring a lock (e.g. malloc) as the lock could have been
+        * held by another thread at the time of forking, causing the lock to
+        * never be released in the child process.  This means only
+        * Async-Signal-Safe functions are permitted in the child.
+        */
        cmd->pid = fork();
        failed_errno = errno;
        if (!cmd->pid) {
+               int sig;
                /*
-                * 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.
+                * Ensure the default die/error/warn routines do not get
+                * called, they can take stdio locks and malloc.
                 */
-               if (cmd->no_stderr || need_err) {
-                       int child_err = dup(2);
-                       set_cloexec(child_err);
-                       set_error_handle(fdopen(child_err, "w"));
-               }
+               set_die_routine(child_die_fn);
+               set_error_routine(child_error_fn);
+               set_warn_routine(child_warn_fn);
 
                close(notify_pipe[0]);
                set_cloexec(notify_pipe[1]);
                child_notifier = notify_pipe[1];
-               atexit(notify_parent);
 
                if (cmd->no_stdin)
-                       dup_devnull(0);
+                       child_dup2(null_fd, 0);
                else if (need_in) {
-                       dup2(fdin[0], 0);
-                       close_pair(fdin);
+                       child_dup2(fdin[0], 0);
+                       child_close_pair(fdin);
                } else if (cmd->in) {
-                       dup2(cmd->in, 0);
-                       close(cmd->in);
+                       child_dup2(cmd->in, 0);
+                       child_close(cmd->in);
                }
 
                if (cmd->no_stderr)
-                       dup_devnull(2);
+                       child_dup2(null_fd, 2);
                else if (need_err) {
-                       dup2(fderr[1], 2);
-                       close_pair(fderr);
+                       child_dup2(fderr[1], 2);
+                       child_close_pair(fderr);
                } else if (cmd->err > 1) {
-                       dup2(cmd->err, 2);
-                       close(cmd->err);
+                       child_dup2(cmd->err, 2);
+                       child_close(cmd->err);
                }
 
                if (cmd->no_stdout)
-                       dup_devnull(1);
+                       child_dup2(null_fd, 1);
                else if (cmd->stdout_to_stderr)
-                       dup2(2, 1);
+                       child_dup2(2, 1);
                else if (need_out) {
-                       dup2(fdout[1], 1);
-                       close_pair(fdout);
+                       child_dup2(fdout[1], 1);
+                       child_close_pair(fdout);
                } else if (cmd->out > 1) {
-                       dup2(cmd->out, 1);
-                       close(cmd->out);
+                       child_dup2(cmd->out, 1);
+                       child_close(cmd->out);
                }
 
                if (cmd->dir && chdir(cmd->dir))
-                       die_errno("exec '%s': cd to '%s' failed", cmd->argv[0],
-                           cmd->dir);
-               if (cmd->env) {
-                       for (; *cmd->env; cmd->env++) {
-                               if (strchr(*cmd->env, '='))
-                                       putenv((char *)*cmd->env);
-                               else
-                                       unsetenv(*cmd->env);
-                       }
+                       child_die(CHILD_ERR_CHDIR);
+
+               /*
+                * restore default signal handlers here, in case
+                * we catch a signal right before execve below
+                */
+               for (sig = 1; sig < NSIG; sig++) {
+                       /* ignored signals get reset to SIG_DFL on execve */
+                       if (signal(sig, SIG_DFL) == SIG_IGN)
+                               signal(sig, SIG_IGN);
                }
-               if (cmd->git_cmd)
-                       execv_git_cmd(cmd->argv);
-               else if (cmd->use_shell)
-                       execv_shell_cmd(cmd->argv);
-               else
-                       sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
+
+               if (sigprocmask(SIG_SETMASK, &as.old, NULL) != 0)
+                       child_die(CHILD_ERR_SIGPROCMASK);
+
+               /*
+                * Attempt to exec using the command and arguments starting at
+                * argv.argv[1].  argv.argv[0] contains SHELL_PATH which will
+                * be used in the event exec failed with ENOEXEC at which point
+                * we will try to interpret the command using 'sh'.
+                */
+               execve(argv.argv[1], (char *const *) argv.argv + 1,
+                      (char *const *) childenv);
+               if (errno == ENOEXEC)
+                       execve(argv.argv[0], (char *const *) argv.argv,
+                              (char *const *) childenv);
+
                if (errno == ENOENT) {
-                       if (!cmd->silent_exec_failure)
-                               error("cannot run %s: %s", cmd->argv[0],
-                                       strerror(ENOENT));
-                       exit(127);
+                       if (cmd->silent_exec_failure)
+                               child_die(CHILD_ERR_SILENT);
+                       child_die(CHILD_ERR_ENOENT);
                } else {
-                       die_errno("cannot exec '%s'", cmd->argv[0]);
+                       child_die(CHILD_ERR_ERRNO);
                }
        }
+       atfork_parent(&as);
        if (cmd->pid < 0)
                error_errno("cannot fork() for %s", cmd->argv[0]);
        else if (cmd->clean_on_exit)
                mark_child_for_cleanup(cmd->pid, cmd);
 
        /*
-        * Wait for child's execvp. If the execvp succeeds (or if fork()
+        * Wait for child's exec. If the exec succeeds (or if fork()
         * failed), EOF is seen immediately by the parent. Otherwise, the
-        * child process sends a single byte.
+        * child process sends a child_err struct.
         * 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) {
+       if (xread(notify_pipe[0], &cerr, sizeof(cerr)) == sizeof(cerr)) {
                /*
-                * At this point we know that fork() succeeded, but execvp()
+                * At this point we know that fork() succeeded, but exec()
                 * failed. Errors have been reported to our stderr.
                 */
                wait_or_whine(cmd->pid, cmd->argv[0], 0);
+               child_err_spew(cmd, &cerr);
                failed_errno = errno;
                cmd->pid = -1;
        }
        close(notify_pipe[0]);
+
+       if (null_fd >= 0)
+               close(null_fd);
+       argv_array_clear(&argv);
+       free(childenv);
 }
 #else
 {
index 4fa8f65adbdcea4d007435d9c1d54c622df54474..3932420ec8a560479c9697e3626e9da15e952853 100644 (file)
@@ -51,6 +51,7 @@ struct child_process {
 #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
 void child_process_init(struct child_process *);
 void child_process_clear(struct child_process *);
+extern int is_executable(const char *name);
 
 int start_command(struct child_process *);
 int finish_command(struct child_process *);
index 003ca1879ef560b3db0ad9e1af8022c38800b500..c650500c6e51d983dc45a4be3b8dad98f1dece92 100644 (file)
@@ -64,6 +64,24 @@ struct string_list_item *string_list_insert(struct string_list *list, const char
        return list->items + index;
 }
 
+void string_list_remove(struct string_list *list, const char *string,
+                       int free_util)
+{
+       int exact_match;
+       int i = get_entry_index(list, string, &exact_match);
+
+       if (exact_match) {
+               if (list->strdup_strings)
+                       free(list->items[i].string);
+               if (free_util)
+                       free(list->items[i].util);
+
+               list->nr--;
+               memmove(list->items + i, list->items + i + 1,
+                       (list->nr - i) * sizeof(struct string_list_item));
+       }
+}
+
 int string_list_has_string(const struct string_list *list, const char *string)
 {
        int exact_match;
index d3809a14172da5b1512b08fa411136821465659e..29bfb7ae45931686e14c91b1a47a703e7b32fe52 100644 (file)
@@ -62,6 +62,13 @@ int string_list_find_insert_index(const struct string_list *list, const char *st
  */
 struct string_list_item *string_list_insert(struct string_list *list, const char *string);
 
+/*
+ * Removes the given string from the sorted list.
+ * If the string doesn't exist, the list is not altered.
+ */
+extern void string_list_remove(struct string_list *list, const char *string,
+                              int free_util);
+
 /*
  * Checks if the given string is part of a sorted list. If it is part of the list,
  * return the coresponding string_list_item, NULL otherwise.
index 12228b4aa6cb6a393a006130b31c73625911cec6..e4739170aa2b7c833cd51a06a7ac6764c8bf0494 100755 (executable)
@@ -26,6 +26,47 @@ test_expect_success 'run_command can run a command' '
        test_cmp empty err
 '
 
+test_expect_success !MINGW 'run_command can run a script without a #! line' '
+       cat >hello <<-\EOF &&
+       cat hello-script
+       EOF
+       chmod +x hello &&
+       test-run-command run-command ./hello >actual 2>err &&
+
+       test_cmp hello-script actual &&
+       test_cmp empty err
+'
+
+test_expect_success 'run_command does not try to execute a directory' '
+       test_when_finished "rm -rf bin1 bin2" &&
+       mkdir -p bin1/greet bin2 &&
+       write_script bin2/greet <<-\EOF &&
+       cat bin2/greet
+       EOF
+
+       PATH=$PWD/bin1:$PWD/bin2:$PATH \
+               test-run-command run-command greet >actual 2>err &&
+       test_cmp bin2/greet actual &&
+       test_cmp empty err
+'
+
+test_expect_success POSIXPERM 'run_command passes over non-executable file' '
+       test_when_finished "rm -rf bin1 bin2" &&
+       mkdir -p bin1 bin2 &&
+       write_script bin1/greet <<-\EOF &&
+       cat bin1/greet
+       EOF
+       chmod -x bin1/greet &&
+       write_script bin2/greet <<-\EOF &&
+       cat bin2/greet
+       EOF
+
+       PATH=$PWD/bin1:$PWD/bin2:$PATH \
+               test-run-command run-command greet >actual 2>err &&
+       test_cmp bin2/greet actual &&
+       test_cmp empty err
+'
+
 test_expect_success POSIXPERM 'run_command reports EACCES' '
        cat hello-script >hello.sh &&
        chmod -x hello.sh &&
index 87308cdced9af11365d5eb0644b9ac86fac21177..8552184e741fe2465e746a3ac42d19edddb15576 100755 (executable)
@@ -20,8 +20,9 @@ test_expect_success 'create http-accessible bare repository with loose objects'
        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
         git config core.bare true &&
         mkdir -p hooks &&
-        echo "exec git update-server-info" >hooks/post-update &&
-        chmod +x hooks/post-update &&
+        write_script "hooks/post-update" <<-\EOF &&
+        exec git update-server-info
+       EOF
         hooks/post-update
        ) &&
        git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
diff --git a/usage.c b/usage.c
index 1f63e033e99fe3325a96132bdc5b6514d951cecb..2f87ca69a8299485ee013187f99ffbdb2618d7c8 100644 (file)
--- a/usage.c
+++ b/usage.c
@@ -6,12 +6,9 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
-static FILE *error_handle;
-
 void vreportf(const char *prefix, const char *err, va_list params)
 {
        char msg[4096];
-       FILE *fh = error_handle ? error_handle : stderr;
        char *p;
 
        vsnprintf(msg, sizeof(msg), err, params);
@@ -19,7 +16,7 @@ void vreportf(const char *prefix, const char *err, va_list params)
                if (iscntrl(*p) && *p != '\t' && *p != '\n')
                        *p = '?';
        }
-       fprintf(fh, "%s%s\n", prefix, msg);
+       fprintf(stderr, "%s%s\n", prefix, msg);
 }
 
 static NORETURN void usage_builtin(const char *err, va_list params)
@@ -88,11 +85,6 @@ void set_die_is_recursing_routine(int (*routine)(void))
        die_is_recursing = routine;
 }
 
-void set_error_handle(FILE *fh)
-{
-       error_handle = fh;
-}
-
 void NORETURN usagef(const char *err, ...)
 {
        va_list params;