{"thread":{"id":"12090","subject":"[PATCH 6/6] Redirect receive-pack hook output into sideband channel","startedAt":"2008-02-14T06:23:09Z","lastAt":"2008-02-14T06:23:09Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"68712","messageId":"20080214062309.GF30516@spearce.org","threadId":"12090","inReplyTo":null,"subject":"[PATCH 6/6] Redirect receive-pack hook output into sideband channel","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-02-14T06:23:09Z","receivedAt":"2008-02-14T06:23:09Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If both send-pack and receive-pack have selected to enable the\nside-band-64k protocol extension we can redirect the stdout and\nstderr from any hooks we execute into the multiplexed band #2\nchannel.  This allows recv_sideband running as an async \"thread\"\nin send-pack to automatically unpack the additional messages and\ndisplay them on send-pack's stderr.\n\nThere are two different styles of hooks in use within receive-pack,\nso our solution for perform this redirection varies a little.\n\nIn the update and post-update hooks there is no stdin provided to\nthe hook so we can perform a naive loop around the stderr pipe,\ncopying any received messages into band #2.  The stderr pipe is\nalso dup'd over to stdout by start_command, so we get both sets of\nmessages over this single fd.\n\nIn the pre-receive and post-receive hooks we need to supply the set\nof commands on stdin to the hook.  To avoid a deadlock with the\nhook we implement a poll loop within receive-pack, watching for\nwhen we can feed additional data into the hook and also for when\nwe need to copy messages off the stderr pipe to band #2.  By doing\nthis here in receive-pack we allow the hook author to not need to\nperform the same sort of complex poll based IO for its stdin and\nstdout/stderr handling.\n\nTest vectors in t5401 needed to be modified slightly as any messages\nreceived through sideband #2 are prefixed on the send-pack side with\n\"remote: \", indicating their origin.  As the test suite is always\nrun with the same version of send-pack/receive-pack code we know\nthey will agree to enable the side-band-64k extension.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n receive-pack.c          |   91 +++++++++++++++++++++++++++++++++++++++-------\n t/t5401-update-hooks.sh |   22 ++++++------\n 2 files changed, 88 insertions(+), 25 deletions(-)\n\ndiff --git a/receive-pack.c b/receive-pack.c\nindex 8962c4c..f9f080b 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -56,7 +56,14 @@ static void write_head_info(void)\n \tfor_each_ref(show_ref, NULL);\n \tif (!capabilities_sent)\n \t\tshow_ref(\"capabilities^{}\", null_sha1, 0, NULL);\n+}\n \n+static void show_hook_output(char *msgs, ssize_t n)\n+{\n+\tif (use_sideband)\n+\t\tsend_sideband(1, 2, msgs, n, use_sideband);\n+\telse\n+\t\twrite_in_full(2, msgs, n);\n }\n \n struct command {\n@@ -100,10 +107,13 @@ static int hook_status(int code, const char *hook_name)\n static int run_hook(const char *hook_name)\n {\n \tstatic char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];\n+\tstatic char msgs[128];\n \tstruct command *cmd;\n \tstruct child_process proc;\n \tconst char *argv[2];\n \tint have_input = 0, code;\n+\tsize_t buf_len, buf_off;\n+\tstruct pollfd pfd[2];\n \n \tfor (cmd = commands; !have_input && cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string)\n@@ -119,28 +129,87 @@ static int run_hook(const char *hook_name)\n \tmemset(&proc, 0, sizeof(proc));\n \tproc.argv = argv;\n \tproc.in = -1;\n+\tproc.err = -1;\n \tproc.stdout_to_stderr = 1;\n \n \tcode = start_command(&proc);\n \tif (code)\n \t\treturn hook_status(code, hook_name);\n+\n+\tpfd[0].fd = proc.in;\n+\tpfd[0].events = POLLOUT;\n+\tpfd[1].fd = proc.err;\n+\tpfd[1].events = POLLIN;\n+\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n-\t\tif (!cmd->error_string) {\n-\t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n+\t\tif (cmd->error_string)\n+\t\t\tcontinue;\n+\t\tbuf_off = 0;\n+\t\tbuf_len = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n \t\t\t\tsha1_to_hex(cmd->old_sha1),\n \t\t\t\tsha1_to_hex(cmd->new_sha1),\n \t\t\t\tcmd->ref_name);\n-\t\t\tif (write_in_full(proc.in, buf, n) != n)\n-\t\t\t\tbreak;\n-\t\t}\n+\t\tdo {\n+\t\t\tif (poll(pfd, 2, -1) < 0) {\n+\t\t\t\tif (errno == EINTR)\n+\t\t\t\t\tcontinue;\n+\t\t\t\tgoto finish;\n+\t\t\t}\n+\t\t\tif (pfd[0].revents & POLLOUT) {\n+\t\t\t\tssize_t r = xwrite(proc.in, buf + buf_off, buf_len);\n+\t\t\t\tif (r <= 0)\n+\t\t\t\t\tgoto finish;\n+\t\t\t\tbuf_len -= r;\n+\t\t\t\tbuf_off += r;\n+\t\t\t}\n+\t\t\tif (pfd[1].revents & POLLIN) {\n+\t\t\t\tssize_t r = xread(proc.err, msgs, sizeof(msgs));\n+\t\t\t\tif (r <= 0)\n+\t\t\t\t\tgoto finish;\n+\t\t\t\tshow_hook_output(msgs, r);\n+\t\t\t}\n+\t\t} while (buf_len > 0);\n+\t}\n+\n+finish:\n+\tclose(proc.in);\n+\tproc.close_in = 0;\n+\tfor (;;) {\n+\t\tssize_t r = xread(proc.err, msgs, sizeof(msgs));\n+\t\tif (r <= 0)\n+\t\t\tbreak;\n+\t\tshow_hook_output(msgs, r);\n \t}\n \treturn hook_status(finish_command(&proc), hook_name);\n }\n \n+static int run_noinput_hook(const char **argv)\n+{\n+\tstatic char msgs[128];\n+\tstruct child_process proc;\n+\tint code;\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\tproc.argv = argv;\n+\tproc.no_stdin = 1;\n+\tproc.stdout_to_stderr = 1;\n+\tproc.err = -1;\n+\n+\tcode = start_command(&proc);\n+\tif (code)\n+\t\treturn code;\n+\tfor (;;) {\n+\t\tssize_t sz = xread(proc.err, msgs, sizeof(msgs));\n+\t\tif (sz <= 0)\n+\t\t\tbreak;\n+\t\tshow_hook_output(msgs, sz);\n+\t}\n+\treturn finish_command(&proc);\n+}\n+\n static int run_update_hook(struct command *cmd)\n {\n \tstatic const char update_hook[] = \"hooks/update\";\n-\tstruct child_process proc;\n \tconst char *argv[5];\n \n \tif (access(update_hook, X_OK) < 0)\n@@ -152,12 +221,7 @@ static int run_update_hook(struct command *cmd)\n \targv[3] = sha1_to_hex(cmd->new_sha1);\n \targv[4] = NULL;\n \n-\tmemset(&proc, 0, sizeof(proc));\n-\tproc.argv = argv;\n-\tproc.no_stdin = 1;\n-\tproc.stdout_to_stderr = 1;\n-\n-\treturn hook_status(run_command(&proc), update_hook);\n+\treturn hook_status(run_noinput_hook(argv), update_hook);\n }\n \n static const char *update(struct command *cmd)\n@@ -264,8 +328,7 @@ static void run_update_post_hook(struct command *cmd)\n \t\targc++;\n \t}\n \targv[argc] = NULL;\n-\trun_command_v_opt(argv, RUN_COMMAND_NO_STDIN\n-\t\t| RUN_COMMAND_STDOUT_TO_STDERR);\n+\trun_noinput_hook(argv);\n }\n \n static void execute_commands(const char *unpacker_error)\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex 9a12024..7e77a7d 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -117,19 +117,19 @@ test_expect_success 'send-pack produced no output' '\n '\n \n cat <<EOF >expect\n-STDOUT pre-receive\n-STDERR pre-receive\n-STDOUT update refs/heads/master\n-STDERR update refs/heads/master\n-STDOUT update refs/heads/tofail\n-STDERR update refs/heads/tofail\n-STDOUT post-receive\n-STDERR post-receive\n-STDOUT post-update\n-STDERR post-update\n+remote: STDOUT pre-receive\u001b[K\n+remote: STDERR pre-receive\u001b[K\n+remote: STDOUT update refs/heads/master\u001b[K\n+remote: STDERR update refs/heads/master\u001b[K\n+remote: STDOUT update refs/heads/tofail\u001b[K\n+remote: STDERR update refs/heads/tofail\u001b[K\n+remote: STDOUT post-receive\u001b[K\n+remote: STDERR post-receive\u001b[K\n+remote: STDOUT post-update\u001b[K\n+remote: STDERR post-update\u001b[K\n EOF\n test_expect_success 'send-pack stderr contains hook messages' '\n-\tgrep ^STD send.err >actual &&\n+\tgrep ^remote: send.err >actual &&\n \tgit diff - actual <expect\n '\n \n-- \n1.5.4.1.1309.g833c2\n"}]}