This adds start_async() and finish_async(), which runs a function

asynchronously. Communication with the caller happens only via pipes.

For this reason, this implementation forks off a child process that runs

the function.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---

 run-command.c |   53 +++++++++++++++++++++++++++++++++++++++++++++--------

 run-command.h |   23 +++++++++++++++++++++++

 2 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/run-command.c b/run-command.c

index d00c03b..111d584 100644

--- a/run-command.c

+++ b/run-command.c

@@ -127,16 +127,11 @@ int start_command(struct child_process *cmd)

 	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)

@@ -144,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;

@@ -158,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);

@@ -200,3 +204,36 @@ 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("canno...
[ message continues ]

This adds start_async() and finish_async(), which runs a function

asynchronously. Communication with the caller happens only via pipes.

For this reason, this implementation forks off a child process that runs

the function.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---
This is the same as submitted previously except it uses close_pair()

to close the pipe in an error patch and improves the comments in

run-command.h.
-- Hannes
 run-command.c |   52 ++++++++++++++++++++++++++++++++++++++++++++--------

 run-command.h |   22 ++++++++++++++++++++++

 2 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/run-command.c b/run-command.c

index d00c03b..db02f75 100644

--- a/run-command.c

+++ b/run-command.c

@@ -127,16 +127,11 @@ int start_command(struct child_process *cmd)

 	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)

@@ -144,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;

@@ -158,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);

@@ -200,3 +204,35 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const

 	cmd.env =...
[ message continues ]

We run the sideband demultiplexer in an asynchronous function.
Note that earlier there was a check in the child process that closed

xd[1] only if it was different from xd[0]; this test is no longer needed

because git_connect() always returns two different file descriptors

(see ec587fde0a76780931c7ac32474c8c000aa45134).
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---

 builtin-fetch-pack.c |   39 ++++++++++++++++++---------------------

 1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c

index 871b704..51d8a32 100644

--- a/builtin-fetch-pack.c

+++ b/builtin-fetch-pack.c

@@ -457,42 +457,37 @@ 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)

 {

-	pid_t side_pid;

+	struct async demux;

 	int fd[2];

 	con...
[ message continues ]

While this is mostly a cleanup and makes a long function shorter, it later

also allows us to use start_async() with this function.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---

 upload-pack.c |   70 ++++++++++++++++++++++++++++++--------------------------

 1 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c

index ef2894a..c5aa0ea 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...
[ message continues ]

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 <johannes.sixt@telecom.at>

---

 upload-pack.c |   46 ++++++++++++++++++----------------------------

 1 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c

index c5aa0ea..6799468 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...
[ message continues ]

This test uses a rot13 filter, which is its own inverse. It tested only

that the content was the same as the original after both the 'clean' and

the 'smudge' filter were applied. This way it would not detect whether

any filter was run at all. Hence, here we add another test that checks

that the repository contained content that was processed by the 'clean'

filter.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---

 t/t0021-conversion.sh |    7 ++++++-

 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh

index a839f4e..cb86029 100755

--- a/t/t0021-conversion.sh

+++ b/t/t0021-conversion.sh

@@ -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

--

1.5.3.3.1134.gee562
-

When apply_filter() runs the external (clean or smudge) filter program, it

needs to pass the writable end of a pipe as its stdout. For this purpose,

it used to dup2(2) the file descriptor explicitly to stdout. Now we use

the facilities of start_command() to do it for us.
Furthermore, the path argument of a subordinate function, filter_buffer(),

was not used, so here we replace it to pass the fd instead.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---

 convert.c |    7 +++----

 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/convert.c b/convert.c

index 6d64994..c870817 100644

--- a/convert.c

+++ b/convert.c

@@ -201,7 +201,7 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *

 	return buffer;

 }
-static int filter_buffer(const char *path, const char *src,

+static int filter_buffer(int fd, const char *src,

 			 unsigned long size, const char *cmd)

 {

 	/*

@@ -214,6 +214,7 @@ static int filter_buffer(const char *path, const char *src,

 	memset(&child_process, 0, sizeof(child_process));

 	child_process.argv = argv;

 	child_process.in = -1;

+	child_process.out = fd;
 	if (start_command(&child_process))

 		return error("cannot fork to run external filter %s", cmd);

@@ -265,10 +266,8 @@ static char *apply_filter(const char *path, const char *src,

 		return NULL;

 	}

 	if (!child_process.pid) {

-		dup2(pipe_feed[1], 1);

 		close(pipe_feed[0]);

-		close(pipe_feed[1]);

-		exit(filter_buffer(path, src, *sizep, cmd));

+		exit(filter_buffer(pipe_feed[1], src, *sizep, cmd));

 	}

 	close(pipe_feed[1]);
--

1.5.3.3.1134.gee562
-

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---

 convert.c |   60 +++++++++++++++++++++++++++---------------------------------

 1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/convert.c b/convert.c

index c870817..10161a0 100644

--- a/convert.c

+++ b/convert.c

@@ -201,15 +201,21 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *

 	return buffer;

 }
-static int filter_buffer(int fd, 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;

+	struct filter_params *params = (struct filter_params *)data;

 	int write_err, status;

-	const char *argv[] = { "sh", "-c", cmd, NULL };

+	const char *argv[] = { "sh", "-c", params->cmd, NULL };
 	memset(&child_process, 0, sizeof(child_process));

 	child_process.argv = argv;

@@ -217,17 +223,17 @@ static int filter_buffer(int fd, const char *src,

 	child_process.out = fd;
 	if (start_command(&child_process))

-		return error("cannot fork to run external filter %s", cmd);

+		return error("cannot fork to run external filter %s", params->cmd);
-	write_err = (write_in_full(child_process.in, src, size) < 0);

+	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", params->cmd);

 	return (write_err || status);

 }
@@ -241,42 +247,31 @@ static char *apply_filter(const char *path, const char *src,

 	 * (chil...
[ message continues ]

Hi,
Did you mean to remove the status from the output (it should probably read

"(exit status %d)" instead of just "%d", but an exit status can help 
Please write "return NULL;"
Thanks,

Dscho
-

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

---
This patch now does just that.
-- Hannes
 convert.c |   60 +++++++++++++++++++++++++++---------------------------------

 1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/convert.c b/convert.c

index c870817..ac04157 100644

--- a/convert.c

+++ b/convert.c

@@ -201,15 +201,21 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *

 	return buffer;

 }
-static int filter_buffer(int fd, 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;

+	struct filter_params *params = (struct filter_params *)data;

 	int write_err, status;

-	const char *argv[] = { "sh", "-c", cmd, NULL };

+	const char *argv[] = { "sh", "-c", params->cmd, NULL };
 	memset(&child_process, 0, sizeof(child_process));

 	child_process.argv = argv;

@@ -217,17 +223,17 @@ static int filter_buffer(int fd, const char *src,

 	child_process.out = fd;
 	if (start_command(&child_process))

-		return error("cannot fork to run external filter %s", cmd);

+		return error("cannot fork to run external filter %s", params->cmd);
-	write_err = (write_in_full(child_process.in, src, size) < 0);

+	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);

 }
@@ -241,42 +247,31 @@ static ch...
[ message continues ]

Will do.
-- Hannes

-