From cc41fa8da9b9e9d23221d3be47a80409a89732d4 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 19 Oct 2007 21:47:59 +0200 Subject: 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 Signed-off-by: Shawn O. Pearce --- upload-pack.c | 105 ++++++++++++++++++++++++---------------------------------- 1 file changed, 44 insertions(+), 61 deletions(-) (limited to 'upload-pack.c') diff --git a/upload-pack.c b/upload-pack.c index fe96ef15c4..5c0c0cc8e5 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -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] "; @@ -98,16 +99,18 @@ static void show_edge(struct commit *commit) 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; + const char *argv[10]; + int arg = 0; 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); } - 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"); } - 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 +197,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 +237,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 +262,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 +276,12 @@ static void create_pack_file(void) } /* 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 == pid_pack_objects) ? "git-pack-objects" : + (pid == pack_objects.pid) ? "git-pack-objects" : 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_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; } @@ -340,8 +323,8 @@ static void create_pack_file(void) 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)); -- cgit v1.2.3 From 80ccaa78a8b95ad3b4f6e24dc35a9aa3cae5fd86 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 19 Oct 2007 21:48:02 +0200 Subject: upload-pack: Move the revision walker into a separate function. This allows us later to use start_async() with this function, and at the same time is a nice cleanup that makes a long function (create_pack_file()) shorter. Signed-off-by: Johannes Sixt Signed-off-by: Shawn O. Pearce --- upload-pack.c | 70 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 33 deletions(-) (limited to 'upload-pack.c') diff --git a/upload-pack.c b/upload-pack.c index 5c0c0cc8e5..ccdc30653b 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -97,6 +97,42 @@ static void show_edge(struct commit *commit) fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1)); } +static void do_rev_list(int create_full_pack) +{ + int i; + struct rev_info revs; + + 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); +} + static void create_pack_file(void) { /* Pipe from rev-list to pack-objects @@ -119,41 +155,9 @@ static void create_pack_file(void) 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); + do_rev_list(create_full_pack); exit(0); } -- cgit v1.2.3 From 21edd3f197df80e9493233a3b9849b61764ebf46 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 19 Oct 2007 21:48:03 +0200 Subject: upload-pack: Run rev-list in an asynchronous function. This gets rid of an explicit fork(). Since upload-pack has to coordinate two processes (rev-list and pack-objects), we cannot use the normal finish_async(), but have to monitor the process explicitly. Hence, there are no changes at this front. Signed-off-by: Johannes Sixt Signed-off-by: Shawn O. Pearce --- upload-pack.c | 46 ++++++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 28 deletions(-) (limited to 'upload-pack.c') diff --git a/upload-pack.c b/upload-pack.c index ccdc30653b..67994680f2 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -97,11 +97,12 @@ static void show_edge(struct commit *commit) fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1)); } -static void do_rev_list(int create_full_pack) +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); @@ -131,14 +132,12 @@ static void do_rev_list(int create_full_pack) 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) { - /* Pipe from rev-list to pack-objects - */ - int lp_pipe[2]; - pid_t pid_rev_list; + 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]; @@ -148,22 +147,12 @@ static void create_pack_file(void) 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) { - close(lp_pipe[0]); - pack_pipe = fdopen(lp_pipe[1], "w"); - do_rev_list(create_full_pack); - exit(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) @@ -173,14 +162,15 @@ static void create_pack_file(void) argv[arg++] = NULL; memset(&pack_objects, 0, sizeof(pack_objects)); - pack_objects.in = lp_pipe[0]; /* start_command closes it */ + 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"); } @@ -280,11 +270,11 @@ static void create_pack_file(void) } /* See if the children are still there */ - if (pid_rev_list || pack_objects.pid) { + if (rev_list.pid || pack_objects.pid) { pid = waitpid(-1, &status, WNOHANG); if (!pid) continue; - who = ((pid == pid_rev_list) ? "git-rev-list" : + who = ((pid == rev_list.pid) ? "git-rev-list" : (pid == pack_objects.pid) ? "git-pack-objects" : NULL); if (!who) { @@ -302,11 +292,11 @@ static void create_pack_file(void) who); goto fail; } - if (pid == pid_rev_list) - pid_rev_list = 0; + if (pid == rev_list.pid) + rev_list.pid = 0; if (pid == pack_objects.pid) pack_objects.pid = 0; - if (pid_rev_list || pack_objects.pid) + if (rev_list.pid || pack_objects.pid) continue; } @@ -329,8 +319,8 @@ static void create_pack_file(void) fail: if (pack_objects.pid) kill(pack_objects.pid, SIGKILL); - if (pid_rev_list) - kill(pid_rev_list, 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); } -- cgit v1.2.3