[PATCH 6/6] Redirect receive-pack hook output into sideband channel

[PATCH 6/6] Redirect receive-pack hook output into sideband channel

[Shawn O. Pearce]

If both send-pack and receive-pack have selected to enable the
side-band-64k protocol extension we can redirect the stdout and
stderr from any hooks we execute into the multiplexed band #2
channel.  This allows recv_sideband running as an async "thread"
in send-pack to automatically unpack the additional messages and
display them on send-pack's stderr.

There are two different styles of hooks in use within receive-pack,
so our solution for perform this redirection varies a little.

In the update and post-update hooks there is no stdin provided to
the hook so we can perform a naive loop around the stderr pipe,
copying any received messages into band #2.  The stderr pipe is
also dup'd over to stdout by start_command, so we get both sets of
messages over this single fd.

In the pre-receive and post-receive hooks we need to supply the set
of commands on stdin to the hook.  To avoid a deadlock with the
hook we implement a poll loop within receive-pack, watching for
when we can feed additional data into the hook and also for when
we need to copy messages off the stderr pipe to band #2.  By doing
this here in receive-pack we allow the hook author to not need to
perform the same sort of complex poll based IO for its stdin and
stdout/stderr handling.

Test vectors in t5401 needed to be modified slightly as any messages
received through sideband #2 are prefixed on the send-pack side with
"remote: ", indicating their origin.  As the test suite is always
run with the same version of send-pack/receive-pack code we know
they will agree to enable the side-band-64k extension.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 receive-pack.c          |   91 +++++++++++++++++++++++++++++++++++++++-------
 t/t5401-update-hooks.sh |   22 ++++++------
 2 files changed, 88 insertions(+), 25 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index 8962c4c..f9f080b 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -56,7 +56,14 @@ static void write_head_info(void)
 	for_each_ref(show_ref, NULL);
 	if (!capabilities_sent)
 		show_ref("capabilities^{}", null_sha1, 0, NULL);
+}
 
+static void show_hook_output(char *msgs, ssize_t n)
+{
+	if (use_sideband)
+		send_sideband(1, 2, msgs, n, use_sideband);
+	else
+		write_in_full(2, msgs, n);
 }
 
 struct command {
@@ -100,10 +107,13 @@ static int hook_status(int code, const char *hook_name)
 static int run_hook(const char *hook_name)
 {
 	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
+	static char msgs[128];
 	struct command *cmd;
 	struct child_process proc;
 	const char *argv[2];
 	int have_input = 0, code;
+	size_t buf_len, buf_off;
+	struct pollfd pfd[2];
 
 	for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
 		if (!cmd->error_string)
@@ -119,28 +129,87 @@ static int run_hook(const char *hook_name)
 	memset(&proc, 0, sizeof(proc));
 	proc.argv = argv;
 	proc.in = -1;
+	proc.err = -1;
 	proc.stdout_to_stderr = 1;
 
 	code = start_command(&proc);
 	if (code)
 		return hook_status(code, hook_name);
+
+	pfd[0].fd = proc.in;
+	pfd[0].events = POLLOUT;
+	pfd[1].fd = proc.err;
+	pfd[1].events = POLLIN;
+
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (!cmd->error_string) {
-			size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
+		if (cmd->error_string)
+			continue;
+		buf_off = 0;
+		buf_len = snprintf(buf, sizeof(buf), "%s %s %s\n",
 				sha1_to_hex(cmd->old_sha1),
 				sha1_to_hex(cmd->new_sha1),
 				cmd->ref_name);
-			if (write_in_full(proc.in, buf, n) != n)
-				break;
-		}
+		do {
+			if (poll(pfd, 2, -1) < 0) {
+				if (errno == EINTR)
+					continue;
+				goto finish;
+			}
+			if (pfd[0].revents & POLLOUT) {
+				ssize_t r = xwrite(proc.in, buf + buf_off, buf_len);
+				if (r <= 0)
+					goto finish;
+				buf_len -= r;
+				buf_off += r;
+			}
+			if (pfd[1].revents & POLLIN) {
+				ssize_t r = xread(proc.err, msgs, sizeof(msgs));
+				if (r <= 0)
+					goto finish;
+				show_hook_output(msgs, r);
+			}
+		} while (buf_len > 0);
+	}
+
+finish:
+	close(proc.in);
+	proc.close_in = 0;
+	for (;;) {
+		ssize_t r = xread(proc.err, msgs, sizeof(msgs));
+		if (r <= 0)
+			break;
+		show_hook_output(msgs, r);
 	}
 	return hook_status(finish_command(&proc), hook_name);
 }
 
+static int run_noinput_hook(const char **argv)
+{
+	static char msgs[128];
+	struct child_process proc;
+	int code;
+
+	memset(&proc, 0, sizeof(proc));
+	proc.argv = argv;
+	proc.no_stdin = 1;
+	proc.stdout_to_stderr = 1;
+	proc.err = -1;
+
+	code = start_command(&proc);
+	if (code)
+		return code;
+	for (;;) {
+		ssize_t sz = xread(proc.err, msgs, sizeof(msgs));
+		if (sz <= 0)
+			break;
+		show_hook_output(msgs, sz);
+	}
+	return finish_command(&proc);
+}
+
 static int run_update_hook(struct command *cmd)
 {
 	static const char update_hook[] = "hooks/update";
-	struct child_process proc;
 	const char *argv[5];
 
 	if (access(update_hook, X_OK) < 0)
@@ -152,12 +221,7 @@ static int run_update_hook(struct command *cmd)
 	argv[3] = sha1_to_hex(cmd->new_sha1);
 	argv[4] = NULL;
 
-	memset(&proc, 0, sizeof(proc));
-	proc.argv = argv;
-	proc.no_stdin = 1;
-	proc.stdout_to_stderr = 1;
-
-	return hook_status(run_command(&proc), update_hook);
+	return hook_status(run_noinput_hook(argv), update_hook);
 }
 
 static const char *update(struct command *cmd)
@@ -264,8 +328,7 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
-		| RUN_COMMAND_STDOUT_TO_STDERR);
+	run_noinput_hook(argv);
 }
 
 static void execute_commands(const char *unpacker_error)
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 9a12024..7e77a7d 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -117,19 +117,19 @@ test_expect_success 'send-pack produced no output' '
 '
 
 cat <<EOF >expect
-STDOUT pre-receive
-STDERR pre-receive
-STDOUT update refs/heads/master
-STDERR update refs/heads/master
-STDOUT update refs/heads/tofail
-STDERR update refs/heads/tofail
-STDOUT post-receive
-STDERR post-receive
-STDOUT post-update
-STDERR post-update
+remote: STDOUT pre-receive^[[K
+remote: STDERR pre-receive^[[K
+remote: STDOUT update refs/heads/master^[[K
+remote: STDERR update refs/heads/master^[[K
+remote: STDOUT update refs/heads/tofail^[[K
+remote: STDERR update refs/heads/tofail^[[K
+remote: STDOUT post-receive^[[K
+remote: STDERR post-receive^[[K
+remote: STDOUT post-update^[[K
+remote: STDERR post-update^[[K
 EOF
 test_expect_success 'send-pack stderr contains hook messages' '
-	grep ^STD send.err >actual &&
+	grep ^remote: send.err >actual &&
 	git diff - actual <expect
 '
 
-- 
1.5.4.1.1309.g833c2