Linus Torvalds <torvalds@xxxxxxxx> writes:
> For some reason I thought we had fixed that by just generating the object
> list internally, but I guess we don't do that. That's just stupid.
Thanks. How about this?
-- >8 --
[PATCH] send-pack: tell pack-objects to use its internal rev-list.
This means one less process in the pipeline to worry about, and
removes about 1/8 of the code.
Signed-off-by: Junio C Hamano <junkio@xxxxxxx>
---
send-pack.c | 139 ++++++++++++++++++-----------------------------------------
1 files changed, 42 insertions(+), 97 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 29cf736..eaa6efb 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -14,100 +14,49 @@ static int send_all;
static int force_update;
static int use_thin_pack;
-static void exec_pack_objects(void)
-{
- static const char *args[] = {
- "pack-objects",
- "--all-progress",
- "--stdout",
- NULL
- };
- execv_git_cmd(args);
- die("git-pack-objects exec failed (%s)", strerror(errno));
-}
-
-static void exec_rev_list(struct ref *refs)
-{
- static const char *args[4];
- int i = 0;
-
- args[i++] = "rev-list"; /* 0 */
- if (use_thin_pack) /* 1 */
- args[i++] = "--objects-edge";
- else
- args[i++] = "--objects";
-
- args[i++] = "--stdin";
-
- args[i] = NULL;
- execv_git_cmd(args);
- die("git-rev-list exec failed (%s)", strerror(errno));
-}
-
/*
- * Run "rev-list --stdin | pack-objects" pipe.
- */
-static void rev_list(struct ref *refs)
-{
- int pipe_fd[2];
- pid_t pack_objects_pid;
-
- if (pipe(pipe_fd) < 0)
- die("rev-list setup: pipe failed");
- pack_objects_pid = fork();
- if (!pack_objects_pid) {
- /* The child becomes pack-objects; reads from pipe
- * and writes to the original fd
- */
- dup2(pipe_fd[0], 0);
- close(pipe_fd[0]);
- close(pipe_fd[1]);
- exec_pack_objects();
- die("pack-objects setup failed");
- }
- if (pack_objects_pid < 0)
- die("pack-objects fork failed");
-
- /* We become rev-list --stdin; output goes to pipe. */
- dup2(pipe_fd[1], 1);
- close(pipe_fd[0]);
- close(pipe_fd[1]);
- exec_rev_list(refs);
-}
-
-/*
- * Create "rev-list --stdin | pack-objects" pipe and feed
- * the refs into the pipeline.
+ * Make a pack stream and spit it out into file descriptor fd
*/
-static void rev_list_generate(int fd, struct ref *refs)
+static int pack_objects(int fd, struct ref *refs)
{
int pipe_fd[2];
- pid_t rev_list_generate_pid;
+ pid_t pid;
if (pipe(pipe_fd) < 0)
- die("rev-list-generate setup: pipe failed");
- rev_list_generate_pid = fork();
- if (!rev_list_generate_pid) {
- /* The child becomes the "rev-list | pack-objects"
- * pipeline. It takes input from us, and its output
- * goes to fd.
+ return error("send-pack: pipe failed");
+ pid = fork();
+ if (!pid) {
+ /*
+ * The child becomes pack-objects --revs; we feed
+ * the revision parameters to it via its stdin and
+ * let its stdout go back to the other end.
*/
+ static const char *args[] = {
+ "pack-objects",
+ "--all-progress",
+ "--revs",
+ "--stdout",
+ NULL,
+ NULL,
+ };
+ if (use_thin_pack)
+ args[4] = "--thin";
dup2(pipe_fd[0], 0);
dup2(fd, 1);
close(pipe_fd[0]);
close(pipe_fd[1]);
close(fd);
- rev_list(refs);
- die("rev-list setup failed");
+ execv_git_cmd(args);
+ die("git-pack-objects exec failed (%s)", strerror(errno));
}
- if (rev_list_generate_pid < 0)
- die("rev-list-generate fork failed");
- /* We feed the rev parameters to them. We do not write into
- * fd nor read from the pipe.
+ /*
+ * We feed the pack-objects we just spawned with revision
+ * parameters by writing to the pipe.
*/
close(pipe_fd[0]);
close(fd);
+
while (refs) {
char buf[42];
@@ -126,28 +75,24 @@ static void rev_list_generate(int fd, struct ref *refs)
refs = refs->next;
}
close(pipe_fd[1]);
- // waitpid(rev_list_generate_pid);
- exit(0);
-}
-/*
- * Make a pack stream and spit it out into file descriptor fd
- */
-static void pack_objects(int fd, struct ref *refs)
-{
- pid_t rev_list_pid;
+ for (;;) {
+ int status, code;
+ pid_t waiting = waitpid(pid, &status, 0);
- rev_list_pid = fork();
- if (!rev_list_pid) {
- rev_list_generate(fd, refs);
- die("rev-list setup failed");
+ if (waiting < 0) {
+ if (errno == EINTR)
+ continue;
+ return error("waitpid failed (%s)", strerror(errno));
+ }
+ if ((waiting != pid) || WIFSIGNALED(status) ||
+ !WIFEXITED(status))
+ return error("pack-objects died with strange error");
+ code = WEXITSTATUS(status);
+ if (code)
+ return -code;
+ return 0;
}
- if (rev_list_pid < 0)
- die("rev-list fork failed");
- /*
- * We don't wait for the rev-list pipeline in the parent:
- * we end up waiting for the other end instead
- */
}
static void unmark_and_free(struct commit_list *list, unsigned int mark)
@@ -379,7 +324,7 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
packet_flush(out);
if (new_refs)
- pack_objects(out, remote_refs);
+ ret = pack_objects(out, remote_refs);
close(out);
if (expect_status_report) {
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html