Code

Merge branch 'js/exec-error-report'
authorJunio C Hamano <gitster@pobox.com>
Wed, 20 Jan 2010 22:44:12 +0000 (14:44 -0800)
committerJunio C Hamano <gitster@pobox.com>
Wed, 20 Jan 2010 22:44:12 +0000 (14:44 -0800)
* 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

1  2 
Makefile
run-command.c
transport-helper.c

diff --cc Makefile
index d74c1962340597f2f4afc760f09c3b1bd764b4f0,22c1546e2f150d15a659ce4d9bab0c4fafb2f7ba..cf0723ea122b09c66a6da367a653381966729847
+++ b/Makefile
@@@ -1769,31 -1776,20 +1769,32 @@@ endi
  
  ### Testing rules
  
 -TEST_PROGRAMS += test-chmtime$X
 -TEST_PROGRAMS += test-ctype$X
 -TEST_PROGRAMS += test-date$X
 -TEST_PROGRAMS += test-delta$X
 -TEST_PROGRAMS += test-dump-cache-tree$X
 -TEST_PROGRAMS += test-genrandom$X
 -TEST_PROGRAMS += test-match-trees$X
 -TEST_PROGRAMS += test-parse-options$X
 -TEST_PROGRAMS += test-path-utils$X
 -TEST_PROGRAMS += test-run-command$X
 -TEST_PROGRAMS += test-sha1$X
 -TEST_PROGRAMS += test-sigchain$X
 -
 -all:: $(TEST_PROGRAMS)
 +TEST_PROGRAMS_NEED_X += test-chmtime
 +TEST_PROGRAMS_NEED_X += test-ctype
 +TEST_PROGRAMS_NEED_X += test-date
 +TEST_PROGRAMS_NEED_X += test-delta
 +TEST_PROGRAMS_NEED_X += test-dump-cache-tree
 +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
 +
 +TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
 +
 +test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 +
 +all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 +
 +bin-wrappers/%: wrap-for-bin.sh
 +      @mkdir -p bin-wrappers
 +      $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 +           -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
 +           -e 's|@@PROG@@|$(@F)|' < $< > $@ && \
 +      chmod +x $@
  
  # GNU make supports exporting all variables by "export" without parameters.
  # However, the environment gets quite big, and some programs have problems
diff --cc run-command.c
index a90984576411d237c1b83b66427e99d9e37ea7c8,efe9fe413887966ced2bb3cd976789ec8789ec4d..2feb493951322617692085998ac8507cdba9dd30
@@@ -15,52 -14,79 +15,124 @@@ static inline void dup_devnull(int to
        dup2(fd, to);
        close(fd);
  }
 +#endif
 +
 +static const char **prepare_shell_cmd(const char **argv)
 +{
 +      int argc, nargc = 0;
 +      const char **nargv;
 +
 +      for (argc = 0; argv[argc]; argc++)
 +              ; /* just counting */
 +      /* +1 for NULL, +3 for "sh -c" plus extra $0 */
 +      nargv = xmalloc(sizeof(*nargv) * (argc + 1 + 3));
 +
 +      if (argc < 1)
 +              die("BUG: shell command is empty");
 +
 +      if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
 +              nargv[nargc++] = "sh";
 +              nargv[nargc++] = "-c";
 +
 +              if (argc < 2)
 +                      nargv[nargc++] = argv[0];
 +              else {
 +                      struct strbuf arg0 = STRBUF_INIT;
 +                      strbuf_addf(&arg0, "%s \"$@\"", argv[0]);
 +                      nargv[nargc++] = strbuf_detach(&arg0, NULL);
 +              }
 +      }
 +
 +      for (argc = 0; argv[argc]; argc++)
 +              nargv[nargc++] = argv[argc];
 +      nargv[nargc] = NULL;
 +
 +      return nargv;
 +}
 +
 +#ifndef WIN32
 +static int execv_shell_cmd(const char **argv)
 +{
 +      const char **nargv = prepare_shell_cmd(argv);
 +      trace_argv_printf(nargv, "trace: exec:");
 +      execvp(nargv[0], (char **)nargv);
 +      free(nargv);
 +      return -1;
 +}
 +#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;
@@@ -165,12 -212,18 +258,20 @@@ 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) {
 +                      execv_shell_cmd(cmd->argv);
                } else {
                        execvp(cmd->argv[0], (char *const*) cmd->argv);
                }
        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 s0 = -1, s1 = -1, s2 = -1;  /* backups of stdin, stdout, stderr */
 +      int fhin = 0, fhout = 1, fherr = 2;
        const char **sargv = cmd->argv;
        char **env = environ;
  
Simple merge