Merge branch 'js/forkexec'
authorJunio C Hamano <gitster@pobox.com>
Thu, 1 Nov 2007 20:47:47 +0000 (13:47 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 1 Nov 2007 20:47:47 +0000 (13:47 -0700)
* js/forkexec:
Use the asyncronous function infrastructure to run the content filter.
Avoid a dup2(2) in apply_filter() - start_command() can do it for us.
t0021-conversion.sh: Test that the clean filter really cleans content.
upload-pack: Run rev-list in an asynchronous function.
upload-pack: Move the revision walker into a separate function.
Use the asyncronous function infrastructure in builtin-fetch-pack.c.
Add infrastructure to run a function asynchronously.
upload-pack: Use start_command() to run pack-objects in create_pack_file().
Have start_command() create a pipe to read the stderr of the child.
Use start_comand() in builtin-fetch-pack.c instead of explicit fork/exec.
Use run_command() to spawn external diff programs instead of fork/exec.
Use start_command() to run content filters instead of explicit fork/exec.
Use start_command() in git_connect() instead of explicit fork/exec.
Change git_connect() to return a struct child_process instead of a pid_t.

Conflicts:

builtin-fetch-pack.c

13 files changed:
builtin-archive.c
builtin-fetch-pack.c
cache.h
connect.c
convert.c
diff.c
peek-remote.c
run-command.c
run-command.h
send-pack.c
t/t0021-conversion.sh
transport.c
upload-pack.c
index 6f29c2f40a01b3c60eaa9ecfa1ca4d63fe90c8eb..14a1b3077cd7a5c4d69672bccfadc1354568dc4a 100644 (file)
@@ -30,7 +30,7 @@ static int run_remote_archiver(const char *remote, int argc,
 {
        char *url, buf[LARGE_PACKET_MAX];
        int fd[2], i, len, rv;
-       pid_t pid;
+       struct child_process *conn;
        const char *exec = "git-upload-archive";
        int exec_at = 0;
 
@@ -46,9 +46,7 @@ static int run_remote_archiver(const char *remote, int argc,
        }
 
        url = xstrdup(remote);
-       pid = git_connect(fd, url, exec, 0);
-       if (pid < 0)
-               return pid;
+       conn = git_connect(fd, url, exec, 0);
 
        for (i = 1; i < argc; i++) {
                if (i == exec_at)
@@ -76,7 +74,7 @@ static int run_remote_archiver(const char *remote, int argc,
        rv = recv_sideband("archive", fd[0], 1, 2);
        close(fd[0]);
        close(fd[1]);
-       rv |= finish_connect(pid);
+       rv |= finish_connect(conn);
 
        return !!rv;
 }
index 8753840a825d35249fa74ee80867f2dfd542708b..862652be92f65c50aaa900fd9983f8c54840d672 100644 (file)
@@ -7,6 +7,7 @@
 #include "pack.h"
 #include "sideband.h"
 #include "fetch-pack.h"
+#include "run-command.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -457,53 +458,49 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
        return retval;
 }
 
-static pid_t setup_sideband(int fd[2], int xd[2])
+static int sideband_demux(int fd, void *data)
 {
-       pid_t side_pid;
+       int *xd = data;
 
+       close(xd[1]);
+       return recv_sideband("fetch-pack", xd[0], fd, 2);
+}
+
+static void setup_sideband(int fd[2], int xd[2], struct async *demux)
+{
        if (!use_sideband) {
                fd[0] = xd[0];
                fd[1] = xd[1];
-               return 0;
+               return;
        }
        /* xd[] is talking with upload-pack; subprocess reads from
         * xd[0], spits out band#2 to stderr, and feeds us band#1
-        * through our fd[0].
+        * through demux->out.
         */
-       if (pipe(fd) < 0)
-               die("fetch-pack: unable to set up pipe");
-       side_pid = fork();
-       if (side_pid < 0)
+       demux->proc = sideband_demux;
+       demux->data = xd;
+       if (start_async(demux))
                die("fetch-pack: unable to fork off sideband demultiplexer");
-       if (!side_pid) {
-               /* subprocess */
-               close(fd[0]);
-               if (xd[0] != xd[1])
-                       close(xd[1]);
-               if (recv_sideband("fetch-pack", xd[0], fd[1], 2))
-                       exit(1);
-               exit(0);
-       }
        close(xd[0]);
-       close(fd[1]);
+       fd[0] = demux->out;
        fd[1] = xd[1];
-       return side_pid;
 }
 
 static int get_pack(int xd[2], char **pack_lockfile)
 {
-       int status;
-       pid_t pid, side_pid;
+       struct async demux;
        int fd[2];
        const char *argv[20];
        char keep_arg[256];
        char hdr_arg[256];
        const char **av;
        int do_keep = args.keep_pack;
-       int keep_pipe[2];
+       struct child_process cmd;
 
-       side_pid = setup_sideband(fd, xd);
+       setup_sideband(fd, xd, &demux);
 
+       memset(&cmd, 0, sizeof(cmd));
+       cmd.argv = argv;
        av = argv;
        *hdr_arg = 0;
        if (!args.keep_pack && unpack_limit) {
@@ -520,8 +517,8 @@ static int get_pack(int xd[2], char **pack_lockfile)
        }
 
        if (do_keep) {
-               if (pack_lockfile && pipe(keep_pipe))
-                       die("fetch-pack: pipe setup failure: %s", strerror(errno));
+               if (pack_lockfile)
+                       cmd.out = -1;
                *av++ = "index-pack";
                *av++ = "--stdin";
                if (!args.quiet && !args.no_progress)
@@ -545,43 +542,19 @@ static int get_pack(int xd[2], char **pack_lockfile)
                *av++ = hdr_arg;
        *av++ = NULL;
 
-       pid = fork();
-       if (pid < 0)
+       cmd.in = fd[0];
+       cmd.git_cmd = 1;
+       if (start_command(&cmd))
                die("fetch-pack: unable to fork off %s", argv[0]);
-       if (!pid) {
-               dup2(fd[0], 0);
-               if (do_keep && pack_lockfile) {
-                       dup2(keep_pipe[1], 1);
-                       close(keep_pipe[0]);
-                       close(keep_pipe[1]);
-               }
-               close(fd[0]);
-               close(fd[1]);
-               execv_git_cmd(argv);
-               die("%s exec failed", argv[0]);
-       }
-       close(fd[0]);
        close(fd[1]);
-       if (do_keep && pack_lockfile) {
-               close(keep_pipe[1]);
-               *pack_lockfile = index_pack_lockfile(keep_pipe[0]);
-               close(keep_pipe[0]);
-       }
-       while (waitpid(pid, &status, 0) < 0) {
-               if (errno != EINTR)
-                       die("waiting for %s: %s", argv[0], strerror(errno));
-       }
-       if (WIFEXITED(status)) {
-               int code = WEXITSTATUS(status);
-               if (code)
-                       die("%s died with error code %d", argv[0], code);
-               return 0;
-       }
-       if (WIFSIGNALED(status)) {
-               int sig = WTERMSIG(status);
-               die("%s died of signal %d", argv[0], sig);
-       }
-       die("%s died of unnatural causes %d", argv[0], status);
+       if (do_keep && pack_lockfile)
+               *pack_lockfile = index_pack_lockfile(cmd.out);
+
+       if (finish_command(&cmd))
+               die("%s failed", argv[0]);
+       if (use_sideband && finish_async(&demux))
+               die("error in sideband demultiplexer");
+       return 0;
 }
 
 static struct ref *do_fetch_pack(int fd[2],
@@ -763,7 +736,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 {
        int i, ret;
        int fd[2];
-       pid_t pid;
+       struct child_process *conn;
        struct ref *ref;
        struct stat st;
 
@@ -774,16 +747,14 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
                        st.st_mtime = 0;
        }
 
-       pid = git_connect(fd, (char *)dest, args.uploadpack,
+       conn = git_connect(fd, (char *)dest, args.uploadpack,
                           args.verbose ? CONNECT_VERBOSE : 0);
-       if (pid < 0)
-               return NULL;
        if (heads && nr_heads)
                nr_heads = remove_duplicates(nr_heads, heads);
        ref = do_fetch_pack(fd, nr_heads, heads, pack_lockfile);
        close(fd[0]);
        close(fd[1]);
-       ret = finish_connect(pid);
+       ret = finish_connect(conn);
 
        if (!ret && nr_heads) {
                /* If the heads to pull were given, we should have
diff --git a/cache.h b/cache.h
index 27485d36c2f56b6832f96e68ca8cbc7ffad0b957..bfffa05dff149a948e46b04f6b15589e6f4be07e 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -503,8 +503,8 @@ struct ref {
 #define REF_TAGS       (1u << 2)
 
 #define CONNECT_VERBOSE       (1u << 0)
-extern pid_t git_connect(int fd[2], char *url, const char *prog, int flags);
-extern int finish_connect(pid_t pid);
+extern struct child_process *git_connect(int fd[2], char *url, const char *prog, int flags);
+extern int finish_connect(struct child_process *conn);
 extern int path_match(const char *path, int nr, char **match);
 extern int get_ack(int fd, unsigned char *result_sha1);
 extern struct ref **get_remote_heads(int in, struct ref **list, int nr_match, char **match, unsigned int flags);
index 3d5c4ab7550d3665a4b24265c0c052e3c7e00231..44e423dafd5c5e6566124fd1bb05f3d419b19a40 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -468,24 +468,26 @@ char *get_port(char *host)
 }
 
 /*
- * This returns 0 if the transport protocol does not need fork(2),
- * or a process id if it does.  Once done, finish the connection
+ * This returns NULL if the transport protocol does not need fork(2), or a
+ * struct child_process object if it does.  Once done, finish the connection
  * with finish_connect() with the value returned from this function
- * (it is safe to call finish_connect() with 0 to support the former
+ * (it is safe to call finish_connect() with NULL to support the former
  * case).
  *
- * Does not return a negative value on error; it just dies.
+ * If it returns, the connect is successful; it just dies on errors.
  */
-pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
+struct child_process *git_connect(int fd[2], char *url,
+                                 const char *prog, int flags)
 {
        char *host, *path = url;
        char *end;
        int c;
-       int pipefd[2][2];
-       pid_t pid;
+       struct child_process *conn;
        enum protocol protocol = PROTO_LOCAL;
        int free_path = 0;
        char *port = NULL;
+       const char **arg;
+       struct strbuf cmd;
 
        /* Without this we cannot rely on waitpid() to tell
         * what happened to our children.
@@ -568,74 +570,68 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
                free(target_host);
                if (free_path)
                        free(path);
-               return 0;
+               return NULL;
        }
 
-       if (pipe(pipefd[0]) < 0 || pipe(pipefd[1]) < 0)
-               die("unable to create pipe pair for communication");
-       pid = fork();
-       if (pid < 0)
-               die("unable to fork");
-       if (!pid) {
-               struct strbuf cmd;
-
-               strbuf_init(&cmd, MAX_CMD_LEN);
-               strbuf_addstr(&cmd, prog);
-               strbuf_addch(&cmd, ' ');
-               sq_quote_buf(&cmd, path);
-               if (cmd.len >= MAX_CMD_LEN)
-                       die("command line too long");
-
-               dup2(pipefd[1][0], 0);
-               dup2(pipefd[0][1], 1);
-               close(pipefd[0][0]);
-               close(pipefd[0][1]);
-               close(pipefd[1][0]);
-               close(pipefd[1][1]);
-               if (protocol == PROTO_SSH) {
-                       const char *ssh, *ssh_basename;
-                       ssh = getenv("GIT_SSH");
-                       if (!ssh) ssh = "ssh";
-                       ssh_basename = strrchr(ssh, '/');
-                       if (!ssh_basename)
-                               ssh_basename = ssh;
-                       else
-                               ssh_basename++;
-
-                       if (!port)
-                               execlp(ssh, ssh_basename, host, cmd.buf, NULL);
-                       else
-                               execlp(ssh, ssh_basename, "-p", port, host,
-                                      cmd.buf, NULL);
-               }
-               else {
-                       unsetenv(ALTERNATE_DB_ENVIRONMENT);
-                       unsetenv(DB_ENVIRONMENT);
-                       unsetenv(GIT_DIR_ENVIRONMENT);
-                       unsetenv(GIT_WORK_TREE_ENVIRONMENT);
-                       unsetenv(GRAFT_ENVIRONMENT);
-                       unsetenv(INDEX_ENVIRONMENT);
-                       execlp("sh", "sh", "-c", cmd.buf, NULL);
+       conn = xcalloc(1, sizeof(*conn));
+
+       strbuf_init(&cmd, MAX_CMD_LEN);
+       strbuf_addstr(&cmd, prog);
+       strbuf_addch(&cmd, ' ');
+       sq_quote_buf(&cmd, path);
+       if (cmd.len >= MAX_CMD_LEN)
+               die("command line too long");
+
+       conn->in = conn->out = -1;
+       conn->argv = arg = xcalloc(6, sizeof(*arg));
+       if (protocol == PROTO_SSH) {
+               const char *ssh = getenv("GIT_SSH");
+               if (!ssh) ssh = "ssh";
+
+               *arg++ = ssh;
+               if (port) {
+                       *arg++ = "-p";
+                       *arg++ = port;
                }
-               die("exec failed");
+               *arg++ = host;
        }
-       fd[0] = pipefd[0][0];
-       fd[1] = pipefd[1][1];
-       close(pipefd[0][1]);
-       close(pipefd[1][0]);
+       else {
+               /* remove these from the environment */
+               const char *env[] = {
+                       ALTERNATE_DB_ENVIRONMENT,
+                       DB_ENVIRONMENT,
+                       GIT_DIR_ENVIRONMENT,
+                       GIT_WORK_TREE_ENVIRONMENT,
+                       GRAFT_ENVIRONMENT,
+                       INDEX_ENVIRONMENT,
+                       NULL
+               };
+               conn->env = env;
+               *arg++ = "sh";
+               *arg++ = "-c";
+       }
+       *arg++ = cmd.buf;
+       *arg = NULL;
+
+       if (start_command(conn))
+               die("unable to fork");
+
+       fd[0] = conn->out; /* read from child's stdout */
+       fd[1] = conn->in;  /* write to child's stdin */
+       strbuf_release(&cmd);
        if (free_path)
                free(path);
-       return pid;
+       return conn;
 }
 
-int finish_connect(pid_t pid)
+int finish_connect(struct child_process *conn)
 {
-       if (pid == 0)
+       int code;
+       if (!conn)
                return 0;
 
-       while (waitpid(pid, NULL, 0) < 0) {
-               if (errno != EINTR)
-                       return -1;
-       }
-       return 0;
+       code = finish_command(conn);
+       free(conn->argv);
+       free(conn);
+       return code;
 }
index aa95834eb3e3f55a2bcb8e6c6f8258f4a57b594e..4df75595b1f3e689a9685039c372ffb4c8688291 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -192,48 +192,39 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
        return 1;
 }
 
-static int filter_buffer(const char *path, const char *src,
-                        unsigned long size, const char *cmd)
+struct filter_params {
+       const char *src;
+       unsigned long size;
+       const char *cmd;
+};
+
+static int filter_buffer(int fd, void *data)
 {
        /*
         * Spawn cmd and feed the buffer contents through its stdin.
         */
        struct child_process child_process;
-       int pipe_feed[2];
+       struct filter_params *params = (struct filter_params *)data;
        int write_err, status;
+       const char *argv[] = { "sh", "-c", params->cmd, NULL };
 
        memset(&child_process, 0, sizeof(child_process));
+       child_process.argv = argv;
+       child_process.in = -1;
+       child_process.out = fd;
 
-       if (pipe(pipe_feed) < 0) {
-               error("cannot create pipe to run external filter %s", cmd);
-               return 1;
-       }
-
-       child_process.pid = fork();
-       if (child_process.pid < 0) {
-               error("cannot fork to run external filter %s", cmd);
-               close(pipe_feed[0]);
-               close(pipe_feed[1]);
-               return 1;
-       }
-       if (!child_process.pid) {
-               dup2(pipe_feed[0], 0);
-               close(pipe_feed[0]);
-               close(pipe_feed[1]);
-               execlp("sh", "sh", "-c", cmd, NULL);
-               return 1;
-       }
-       close(pipe_feed[0]);
+       if (start_command(&child_process))
+               return error("cannot fork to run external filter %s", params->cmd);
 
-       write_err = (write_in_full(pipe_feed[1], src, size) < 0);
-       if (close(pipe_feed[1]))
+       write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
+       if (close(child_process.in))
                write_err = 1;
        if (write_err)
-               error("cannot feed the input to external filter %s", cmd);
+               error("cannot feed the input to external filter %s", params->cmd);
 
        status = finish_command(&child_process);
        if (status)
-               error("external filter %s failed %d", cmd, -status);
+               error("external filter %s failed %d", params->cmd, -status);
        return (write_err || status);
 }
 
@@ -246,49 +237,36 @@ static int apply_filter(const char *path, const char *src, size_t len,
         *
         * (child --> cmd) --> us
         */
-       int pipe_feed[2];
-       int status, ret = 1;
-       struct child_process child_process;
+       int ret = 1;
        struct strbuf nbuf;
+       struct async async;
+       struct filter_params params;
 
        if (!cmd)
                return 0;
 
-       memset(&child_process, 0, sizeof(child_process));
-
-       if (pipe(pipe_feed) < 0) {
-               error("cannot create pipe to run external filter %s", cmd);
-               return 0;
-       }
+       memset(&async, 0, sizeof(async));
+       async.proc = filter_buffer;
+       async.data = &params;
+       params.src = src;
+       params.size = len;
+       params.cmd = cmd;
 
        fflush(NULL);
-       child_process.pid = fork();
-       if (child_process.pid < 0) {
-               error("cannot fork to run external filter %s", cmd);
-               close(pipe_feed[0]);
-               close(pipe_feed[1]);
-               return 0;
-       }
-       if (!child_process.pid) {
-               dup2(pipe_feed[1], 1);
-               close(pipe_feed[0]);
-               close(pipe_feed[1]);
-               exit(filter_buffer(path, src, len, cmd));
-       }
-       close(pipe_feed[1]);
+       if (start_async(&async))
+               return 0;       /* error was already reported */
 
        strbuf_init(&nbuf, 0);
-       if (strbuf_read(&nbuf, pipe_feed[0], len) < 0) {
+       if (strbuf_read(&nbuf, async.out, len) < 0) {
                error("read from external filter %s failed", cmd);
                ret = 0;
        }
-       if (close(pipe_feed[0])) {
+       if (close(async.out)) {
                error("read from external filter %s failed", cmd);
                ret = 0;
        }
-       status = finish_command(&child_process);
-       if (status) {
-               error("external filter %s failed %d", cmd, -status);
+       if (finish_async(&async)) {
+               error("external filter %s failed", cmd);
                ret = 0;
        }
 
diff --git a/diff.c b/diff.c
index af85b94d1b5183e2007b5a221054d9cdcce0faff..a6aaaf7e5a91f3e5ad5f85341429c80a3452504e 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -9,6 +9,7 @@
 #include "xdiff-interface.h"
 #include "color.h"
 #include "attr.h"
+#include "run-command.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0
@@ -1761,40 +1762,6 @@ static void remove_tempfile_on_signal(int signo)
        raise(signo);
 }
 
-static int spawn_prog(const char *pgm, const char **arg)
-{
-       pid_t pid;
-       int status;
-
-       fflush(NULL);
-       pid = fork();
-       if (pid < 0)
-               die("unable to fork");
-       if (!pid) {
-               execvp(pgm, (char *const*) arg);
-               exit(255);
-       }
-
-       while (waitpid(pid, &status, 0) < 0) {
-               if (errno == EINTR)
-                       continue;
-               return -1;
-       }
-
-       /* Earlier we did not check the exit status because
-        * diff exits non-zero if files are different, and
-        * we are not interested in knowing that.  It was a
-        * mistake which made it harder to quit a diff-*
-        * session that uses the git-apply-patch-script as
-        * the GIT_EXTERNAL_DIFF.  A custom GIT_EXTERNAL_DIFF
-        * should also exit non-zero only when it wants to
-        * abort the entire diff-* session.
-        */
-       if (WIFEXITED(status) && !WEXITSTATUS(status))
-               return 0;
-       return -1;
-}
-
 /* An external diff command takes:
  *
  * diff-cmd name infile1 infile1-sha1 infile1-mode \
@@ -1847,7 +1814,8 @@ static void run_external_diff(const char *pgm,
                *arg++ = name;
        }
        *arg = NULL;
-       retval = spawn_prog(pgm, spawn_arg);
+       fflush(NULL);
+       retval = run_command_v_opt(spawn_arg, 0);
        remove_tempfile();
        if (retval) {
                fprintf(stderr, "external diff died, stopping at %s.\n", name);
index ceb787170e17df130e26658e5a189bb63684ab3e..8d20f7c9c626787d157be0f07c18c126e272db52 100644 (file)
@@ -25,7 +25,7 @@ int main(int argc, char **argv)
        int i, ret;
        char *dest = NULL;
        int fd[2];
-       pid_t pid;
+       struct child_process *conn;
        int nongit = 0;
        unsigned flags = 0;
 
@@ -64,12 +64,10 @@ int main(int argc, char **argv)
        if (!dest || i != argc - 1)
                usage(peek_remote_usage);
 
-       pid = git_connect(fd, dest, uploadpack, 0);
-       if (pid < 0)
-               return 1;
+       conn = git_connect(fd, dest, uploadpack, 0);
        ret = peek_remote(fd, flags);
        close(fd[0]);
        close(fd[1]);
-       ret |= finish_connect(pid);
+       ret |= finish_connect(conn);
        return !!ret;
 }
index 7e779d33ee9ea5f7d2e6aedc8c3a0a0476e87135..d99a6c4ea7c776bec717c9952f77db7fa9a69c21 100644 (file)
@@ -17,8 +17,8 @@ static inline void dup_devnull(int to)
 
 int start_command(struct child_process *cmd)
 {
-       int need_in, need_out;
-       int fdin[2], fdout[2];
+       int need_in, need_out, need_err;
+       int fdin[2], fdout[2], fderr[2];
 
        need_in = !cmd->no_stdin && cmd->in < 0;
        if (need_in) {
@@ -41,12 +41,26 @@ int start_command(struct child_process *cmd)
                cmd->close_out = 1;
        }
 
+       need_err = cmd->err < 0;
+       if (need_err) {
+               if (pipe(fderr) < 0) {
+                       if (need_in)
+                               close_pair(fdin);
+                       if (need_out)
+                               close_pair(fdout);
+                       return -ERR_RUN_COMMAND_PIPE;
+               }
+               cmd->err = fderr[0];
+       }
+
        cmd->pid = fork();
        if (cmd->pid < 0) {
                if (need_in)
                        close_pair(fdin);
                if (need_out)
                        close_pair(fdout);
+               if (need_err)
+                       close_pair(fderr);
                return -ERR_RUN_COMMAND_FORK;
        }
 
@@ -73,6 +87,11 @@ int start_command(struct child_process *cmd)
                        close(cmd->out);
                }
 
+               if (need_err) {
+                       dup2(fderr[1], 2);
+                       close_pair(fderr);
+               }
+
                if (cmd->dir && chdir(cmd->dir))
                        die("exec %s: cd to %s failed (%s)", cmd->argv[0],
                            cmd->dir, strerror(errno));
@@ -102,19 +121,17 @@ int start_command(struct child_process *cmd)
        else if (cmd->out > 1)
                close(cmd->out);
 
+       if (need_err)
+               close(fderr[1]);
+
        return 0;
 }
 
-int finish_command(struct child_process *cmd)
+static int wait_or_whine(pid_t pid)
 {
-       if (cmd->close_in)
-               close(cmd->in);
-       if (cmd->close_out)
-               close(cmd->out);
-
        for (;;) {
                int status, code;
-               pid_t waiting = waitpid(cmd->pid, &status, 0);
+               pid_t waiting = waitpid(pid, &status, 0);
 
                if (waiting < 0) {
                        if (errno == EINTR)
@@ -122,7 +139,7 @@ int finish_command(struct child_process *cmd)
                        error("waitpid failed (%s)", strerror(errno));
                        return -ERR_RUN_COMMAND_WAITPID;
                }
-               if (waiting != cmd->pid)
+               if (waiting != pid)
                        return -ERR_RUN_COMMAND_WAITPID_WRONG_PID;
                if (WIFSIGNALED(status))
                        return -ERR_RUN_COMMAND_WAITPID_SIGNAL;
@@ -136,6 +153,15 @@ int finish_command(struct child_process *cmd)
        }
 }
 
+int finish_command(struct child_process *cmd)
+{
+       if (cmd->close_in)
+               close(cmd->in);
+       if (cmd->close_out)
+               close(cmd->out);
+       return wait_or_whine(cmd->pid);
+}
+
 int run_command(struct child_process *cmd)
 {
        int code = start_command(cmd);
@@ -178,3 +204,34 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
        cmd.env = env;
        return run_command(&cmd);
 }
+
+int start_async(struct async *async)
+{
+       int pipe_out[2];
+
+       if (pipe(pipe_out) < 0)
+               return error("cannot create pipe: %s", strerror(errno));
+
+       async->pid = fork();
+       if (async->pid < 0) {
+               error("fork (async) failed: %s", strerror(errno));
+               close_pair(pipe_out);
+               return -1;
+       }
+       if (!async->pid) {
+               close(pipe_out[0]);
+               exit(!!async->proc(pipe_out[1], async->data));
+       }
+       async->out = pipe_out[0];
+       close(pipe_out[1]);
+       return 0;
+}
+
+int finish_async(struct async *async)
+{
+       int ret = 0;
+
+       if (wait_or_whine(async->pid))
+               ret = error("waitpid (async) failed");
+       return ret;
+}
index 7958eb1e0b7a927019460e06d7a01622eddf81df..94e1e9d516887d818f99f8f6d6b3ded3f3be6d6f 100644 (file)
@@ -16,6 +16,7 @@ struct child_process {
        pid_t pid;
        int in;
        int out;
+       int err;
        const char *dir;
        const char *const *env;
        unsigned close_in:1;
@@ -42,4 +43,26 @@ int run_command_v_opt_cd(const char **argv, int opt, const char *dir);
  */
 int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);
 
+/*
+ * The purpose of the following functions is to feed a pipe by running
+ * a function asynchronously and providing output that the caller reads.
+ *
+ * It is expected that no synchronization and mutual exclusion between
+ * the caller and the feed function is necessary so that the function
+ * can run in a thread without interfering with the caller.
+ */
+struct async {
+       /*
+        * proc writes to fd and closes it;
+        * returns 0 on success, non-zero on failure
+        */
+       int (*proc)(int fd, void *data);
+       void *data;
+       int out;        /* caller reads from here and closes it */
+       pid_t pid;
+};
+
+int start_async(struct async *async);
+int finish_async(struct async *async);
+
 #endif
index 25053d2c2f7daa9aa392693a892db7e80c8b6469..5e127a1b7b33bdc9aaafc80873c5c987573566bd 100644 (file)
@@ -384,7 +384,7 @@ int main(int argc, char **argv)
        char *dest = NULL;
        char **heads = NULL;
        int fd[2], ret;
-       pid_t pid;
+       struct child_process *conn;
        char *remote_name = NULL;
        struct remote *remote = NULL;
 
@@ -452,12 +452,10 @@ int main(int argc, char **argv)
                }
        }
 
-       pid = git_connect(fd, dest, receivepack, verbose ? CONNECT_VERBOSE : 0);
-       if (pid < 0)
-               return 1;
+       conn = git_connect(fd, dest, receivepack, verbose ? CONNECT_VERBOSE : 0);
        ret = send_pack(fd[0], fd[1], remote, nr_heads, heads);
        close(fd[0]);
        close(fd[1]);
-       ret |= finish_connect(pid);
+       ret |= finish_connect(conn);
        return !!ret;
 }
index a839f4e0744cd9344a3d71a48fe2224a99750729..cb860296edfab2d117fc8b62505df00a51baed02 100755 (executable)
@@ -42,7 +42,12 @@ test_expect_success check '
        git diff --raw --exit-code :test :test.i &&
        id=$(git rev-parse --verify :test) &&
        embedded=$(sed -ne "$script" test.i) &&
-       test "z$id" = "z$embedded"
+       test "z$id" = "z$embedded" &&
+
+       git cat-file blob :test.t > test.r &&
+
+       ./rot13.sh < test.o > test.t &&
+       cmp test.r test.t
 '
 
 # If an expanded ident ever gets into the repository, we want to make sure that
index 400af71c76d8cf699778f98b1b07ea7f719701a3..5132d289dac711c73e3eb7c249694ffa7ce8193e 100644 (file)
@@ -601,18 +601,13 @@ static struct ref *get_refs_via_connect(const struct transport *transport)
        struct git_transport_data *data = transport->data;
        struct ref *refs;
        int fd[2];
-       pid_t pid;
        char *dest = xstrdup(transport->url);
-
-       pid = git_connect(fd, dest, data->uploadpack, 0);
-
-       if (pid < 0)
-               die("Failed to connect to \"%s\"", transport->url);
+       struct child_process *conn = git_connect(fd, dest, data->uploadpack, 0);
 
        get_remote_heads(fd[0], &refs, 0, NULL, 0);
        packet_flush(fd[1]);
 
-       finish_connect(pid);
+       finish_connect(conn);
 
        free(dest);
 
index fe96ef15c43fa6e3f8f947f84ddce3c498e82859..67994680f2f48e573f78c592c853422e6d8995cf 100644 (file)
@@ -9,6 +9,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
+#include "run-command.h"
 
 static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
 
@@ -96,110 +97,86 @@ static void show_edge(struct commit *commit)
        fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
 }
 
+static int do_rev_list(int fd, void *create_full_pack)
+{
+       int i;
+       struct rev_info revs;
+
+       pack_pipe = fdopen(fd, "w");
+       if (create_full_pack)
+               use_thin_pack = 0; /* no point doing it */
+       init_revisions(&revs, NULL);
+       revs.tag_objects = 1;
+       revs.tree_objects = 1;
+       revs.blob_objects = 1;
+       if (use_thin_pack)
+               revs.edge_hint = 1;
+
+       if (create_full_pack) {
+               const char *args[] = {"rev-list", "--all", NULL};
+               setup_revisions(2, args, &revs, NULL);
+       } else {
+               for (i = 0; i < want_obj.nr; i++) {
+                       struct object *o = want_obj.objects[i].item;
+                       /* why??? */
+                       o->flags &= ~UNINTERESTING;
+                       add_pending_object(&revs, o, NULL);
+               }
+               for (i = 0; i < have_obj.nr; i++) {
+                       struct object *o = have_obj.objects[i].item;
+                       o->flags |= UNINTERESTING;
+                       add_pending_object(&revs, o, NULL);
+               }
+               setup_revisions(0, NULL, &revs, NULL);
+       }
+       prepare_revision_walk(&revs);
+       mark_edges_uninteresting(revs.commits, &revs, show_edge);
+       traverse_commit_list(&revs, show_commit, show_object);
+       return 0;
+}
+
 static void create_pack_file(void)
 {
-       /* Pipes between rev-list to pack-objects, pack-objects to us
-        * and pack-objects error stream for progress bar.
-        */
-       int lp_pipe[2], pu_pipe[2], pe_pipe[2];
-       pid_t pid_rev_list, pid_pack_objects;
+       struct async rev_list;
+       struct child_process pack_objects;
        int create_full_pack = (nr_our_refs == want_obj.nr && !have_obj.nr);
        char data[8193], progress[128];
        char abort_msg[] = "aborting due to possible repository "
                "corruption on the remote side.";
        int buffered = -1;
+       const char *argv[10];
+       int arg = 0;
 
-       if (pipe(lp_pipe) < 0)
-               die("git-upload-pack: unable to create pipe");
-       pid_rev_list = fork();
-       if (pid_rev_list < 0)
+       rev_list.proc = do_rev_list;
+       /* .data is just a boolean: any non-NULL value will do */
+       rev_list.data = create_full_pack ? &rev_list : NULL;
+       if (start_async(&rev_list))
                die("git-upload-pack: unable to fork git-rev-list");
 
-       if (!pid_rev_list) {
-               int i;
-               struct rev_info revs;
-
-               close(lp_pipe[0]);
-               pack_pipe = fdopen(lp_pipe[1], "w");
-
-               if (create_full_pack)
-                       use_thin_pack = 0; /* no point doing it */
-               init_revisions(&revs, NULL);
-               revs.tag_objects = 1;
-               revs.tree_objects = 1;
-               revs.blob_objects = 1;
-               if (use_thin_pack)
-                       revs.edge_hint = 1;
-
-               if (create_full_pack) {
-                       const char *args[] = {"rev-list", "--all", NULL};
-                       setup_revisions(2, args, &revs, NULL);
-               } else {
-                       for (i = 0; i < want_obj.nr; i++) {
-                               struct object *o = want_obj.objects[i].item;
-                               /* why??? */
-                               o->flags &= ~UNINTERESTING;
-                               add_pending_object(&revs, o, NULL);
-                       }
-                       for (i = 0; i < have_obj.nr; i++) {
-                               struct object *o = have_obj.objects[i].item;
-                               o->flags |= UNINTERESTING;
-                               add_pending_object(&revs, o, NULL);
-                       }
-                       setup_revisions(0, NULL, &revs, NULL);
-               }
-               prepare_revision_walk(&revs);
-               mark_edges_uninteresting(revs.commits, &revs, show_edge);
-               traverse_commit_list(&revs, show_commit, show_object);
-               exit(0);
-       }
-
-       if (pipe(pu_pipe) < 0)
-               die("git-upload-pack: unable to create pipe");
-       if (pipe(pe_pipe) < 0)
-               die("git-upload-pack: unable to create pipe");
-       pid_pack_objects = fork();
-       if (pid_pack_objects < 0) {
+       argv[arg++] = "pack-objects";
+       argv[arg++] = "--stdout";
+       if (!no_progress)
+               argv[arg++] = "--progress";
+       if (use_ofs_delta)
+               argv[arg++] = "--delta-base-offset";
+       argv[arg++] = NULL;
+
+       memset(&pack_objects, 0, sizeof(pack_objects));
+       pack_objects.in = rev_list.out; /* start_command closes it */
+       pack_objects.out = -1;
+       pack_objects.err = -1;
+       pack_objects.git_cmd = 1;
+       pack_objects.argv = argv;
+
+       if (start_command(&pack_objects)) {
                /* daemon sets things up to ignore TERM */
-               kill(pid_rev_list, SIGKILL);
+               kill(rev_list.pid, SIGKILL);
                die("git-upload-pack: unable to fork git-pack-objects");
        }
-       if (!pid_pack_objects) {
-               const char *argv[10];
-               int i = 0;
-
-               dup2(lp_pipe[0], 0);
-               dup2(pu_pipe[1], 1);
-               dup2(pe_pipe[1], 2);
-
-               close(lp_pipe[0]);
-               close(lp_pipe[1]);
-               close(pu_pipe[0]);
-               close(pu_pipe[1]);
-               close(pe_pipe[0]);
-               close(pe_pipe[1]);
-
-               argv[i++] = "pack-objects";
-               argv[i++] = "--stdout";
-               if (!no_progress)
-                       argv[i++] = "--progress";
-               if (use_ofs_delta)
-                       argv[i++] = "--delta-base-offset";
-               argv[i++] = NULL;
-
-               execv_git_cmd(argv);
-               kill(pid_rev_list, SIGKILL);
-               die("git-upload-pack: unable to exec git-pack-objects");
-       }
-
-       close(lp_pipe[0]);
-       close(lp_pipe[1]);
 
-       /* We read from pe_pipe[0] to capture stderr output for
-        * progress bar, and pu_pipe[0] to capture the pack data.
+       /* We read from pack_objects.err to capture stderr output for
+        * progress bar, and pack_objects.out to capture the pack data.
         */
-       close(pe_pipe[1]);
-       close(pu_pipe[1]);
 
        while (1) {
                const char *who;
@@ -214,14 +191,14 @@ static void create_pack_file(void)
                pollsize = 0;
                pe = pu = -1;
 
-               if (0 <= pu_pipe[0]) {
-                       pfd[pollsize].fd = pu_pipe[0];
+               if (0 <= pack_objects.out) {
+                       pfd[pollsize].fd = pack_objects.out;
                        pfd[pollsize].events = POLLIN;
                        pu = pollsize;
                        pollsize++;
                }
-               if (0 <= pe_pipe[0]) {
-                       pfd[pollsize].fd = pe_pipe[0];
+               if (0 <= pack_objects.err) {
+                       pfd[pollsize].fd = pack_objects.err;
                        pfd[pollsize].events = POLLIN;
                        pe = pollsize;
                        pollsize++;
@@ -254,13 +231,13 @@ static void create_pack_file(void)
                                        *cp++ = buffered;
                                        outsz++;
                                }
-                               sz = xread(pu_pipe[0], cp,
+                               sz = xread(pack_objects.out, cp,
                                          sizeof(data) - outsz);
                                if (0 < sz)
                                                ;
                                else if (sz == 0) {
-                                       close(pu_pipe[0]);
-                                       pu_pipe[0] = -1;
+                                       close(pack_objects.out);
+                                       pack_objects.out = -1;
                                }
                                else
                                        goto fail;
@@ -279,13 +256,13 @@ static void create_pack_file(void)
                                /* Status ready; we ship that in the side-band
                                 * or dump to the standard error.
                                 */
-                               sz = xread(pe_pipe[0], progress,
+                               sz = xread(pack_objects.err, progress,
                                          sizeof(progress));
                                if (0 < sz)
                                        send_client_data(2, progress, sz);
                                else if (sz == 0) {
-                                       close(pe_pipe[0]);
-                                       pe_pipe[0] = -1;
+                                       close(pack_objects.err);
+                                       pack_objects.err = -1;
                                }
                                else
                                        goto fail;
@@ -293,12 +270,12 @@ static void create_pack_file(void)
                }
 
                /* See if the children are still there */
-               if (pid_rev_list || pid_pack_objects) {
+               if (rev_list.pid || pack_objects.pid) {
                        pid = waitpid(-1, &status, WNOHANG);
                        if (!pid)
                                continue;
-                       who = ((pid == pid_rev_list) ? "git-rev-list" :
-                              (pid == pid_pack_objects) ? "git-pack-objects" :
+                       who = ((pid == rev_list.pid) ? "git-rev-list" :
+                              (pid == pack_objects.pid) ? "git-pack-objects" :
                               NULL);
                        if (!who) {
                                if (pid < 0) {
@@ -315,11 +292,11 @@ static void create_pack_file(void)
                                      who);
                                goto fail;
                        }
-                       if (pid == pid_rev_list)
-                               pid_rev_list = 0;
-                       if (pid == pid_pack_objects)
-                               pid_pack_objects = 0;
-                       if (pid_rev_list || pid_pack_objects)
+                       if (pid == rev_list.pid)
+                               rev_list.pid = 0;
+                       if (pid == pack_objects.pid)
+                               pack_objects.pid = 0;
+                       if (rev_list.pid || pack_objects.pid)
                                continue;
                }
 
@@ -340,10 +317,10 @@ static void create_pack_file(void)
                return;
        }
  fail:
-       if (pid_pack_objects)
-               kill(pid_pack_objects, SIGKILL);
-       if (pid_rev_list)
-               kill(pid_rev_list, SIGKILL);
+       if (pack_objects.pid)
+               kill(pack_objects.pid, SIGKILL);
+       if (rev_list.pid)
+               kill(rev_list.pid, SIGKILL);
        send_client_data(3, abort_msg, sizeof(abort_msg));
        die("git-upload-pack: %s", abort_msg);
 }