upload-pack: Use start_command() to run pack-objects in create_pack_file().

This gets rid of an explicit fork/exec.

Since upload-pack has to coordinate two processes (rev-list and
pack-objects), we cannot use the normal finish_command(), but have to
monitor the processes explicitly. Hence, the waitpid() call remains.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/upload-pack.c b/upload-pack.c
index fe96ef15c43fa6e3f8f947f84ddce3c498e82859..5c0c0cc8e5ee1105584ec0128b391e7d518e4a5c 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -9,6 +9,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
 #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>";
 
 
 static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
 


@@ -98,16 +99,18 @@ static void show_edge(struct commit *commit)
 
 static void create_pack_file(void)
 {
 
 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.
+       /* Pipe from rev-list to pack-objects

         */
         */

-       int lp_pipe[2], pu_pipe[2], pe_pipe[2];
-       pid_t pid_rev_list, pid_pack_objects;
+       int lp_pipe[2];
+       pid_t pid_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;
        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");
 
        if (pipe(lp_pipe) < 0)
                die("git-upload-pack: unable to create pipe");


@@ -154,52 +157,32 @@ static void create_pack_file(void)
                exit(0);
        }
 
                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) {
+       /* writable pipe end must not be inherited by pack_objects */
+       close(lp_pipe[1]);
+
+       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 = lp_pipe[0];   /* 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);
                die("git-upload-pack: unable to fork git-pack-objects");
        }
                /* daemon sets things up to ignore TERM */
                kill(pid_rev_list, 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;
 
        while (1) {
                const char *who;


@@ -214,14 +197,14 @@ static void create_pack_file(void)
                pollsize = 0;
                pe = pu = -1;
 
                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++;
                }
                        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++;
                        pfd[pollsize].events = POLLIN;
                        pe = pollsize;
                        pollsize++;


@@ -254,13 +237,13 @@ static void create_pack_file(void)
                                        *cp++ = buffered;
                                        outsz++;
                                }
                                        *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) {
                                          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;
                                }
                                else
                                        goto fail;


@@ -279,13 +262,13 @@ static void create_pack_file(void)
                                /* Status ready; we ship that in the side-band
                                 * or dump to the standard error.
                                 */
                                /* 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) {
                                          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;
                                }
                                else
                                        goto fail;


@@ -293,12 +276,12 @@ static void create_pack_file(void)
                }
 
                /* See if the children are still there */
                }
 
                /* See if the children are still there */

-               if (pid_rev_list || pid_pack_objects) {
+               if (pid_rev_list || pack_objects.pid) {

                        pid = waitpid(-1, &status, WNOHANG);
                        if (!pid)
                                continue;
                        who = ((pid == pid_rev_list) ? "git-rev-list" :
                        pid = waitpid(-1, &status, WNOHANG);
                        if (!pid)
                                continue;
                        who = ((pid == pid_rev_list) ? "git-rev-list" :

-                              (pid == pid_pack_objects) ? "git-pack-objects" :
+                              (pid == pack_objects.pid) ? "git-pack-objects" :

                               NULL);
                        if (!who) {
                                if (pid < 0) {
                               NULL);
                        if (!who) {
                                if (pid < 0) {


@@ -317,9 +300,9 @@ static void create_pack_file(void)
                        }
                        if (pid == pid_rev_list)
                                pid_rev_list = 0;
                        }
                        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 == pack_objects.pid)
+                               pack_objects.pid = 0;
+                       if (pid_rev_list || pack_objects.pid)

                                continue;
                }
 
                                continue;
                }
 


@@ -340,8 +323,8 @@ static void create_pack_file(void)
                return;
        }
  fail:
                return;
        }
  fail:

-       if (pid_pack_objects)
-               kill(pid_pack_objects, SIGKILL);
+       if (pack_objects.pid)
+               kill(pack_objects.pid, SIGKILL);

        if (pid_rev_list)
                kill(pid_rev_list, SIGKILL);
        send_client_data(3, abort_msg, sizeof(abort_msg));
        if (pid_rev_list)
                kill(pid_rev_list, SIGKILL);
        send_client_data(3, abort_msg, sizeof(abort_msg));