{"thread":{"id":"7179","subject":"[PATCH 8/8] Change {pre,post}-receive hooks to use stdin","startedAt":"2007-03-10T08:28:16Z","lastAt":"2007-03-10T08:28:16Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"36737","messageId":"20070310082816.GH4133@spearce.org","threadId":"7179","inReplyTo":null,"subject":"[PATCH 8/8] Change {pre,post}-receive hooks to use stdin","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-03-10T08:28:16Z","receivedAt":"2007-03-10T08:28:16Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sergey Vlasov, Andy Parkins and Alex Riesen all pointed out that it\nis possible for a single invocation of receive-pack to be given more\nrefs than the OS might allow us to pass as command line parameters\nto a single hook invocation.\n\nWe don't want to break these up into multiple invocations (like\nxargs might do) as that makes it impossible for the pre-receive\nhook to verify multiple related ref updates occur at the same time,\nand it makes it harder for post-receive to send out a single batch\nnotification.\n\nInstead we pass the reference data on a pipe connected to the\nhook's stdin, supplying one ref per line to the hook.  This way a\nsingle hook invocation can obtain an infinite amount of ref data,\nwithout bumping into any operating system limits.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n Documentation/git-receive-pack.txt |   32 ++++++++++----------\n receive-pack.c                     |   58 ++++++++++++++++++-----------------\n t/t5401-update-hooks.sh            |   30 ++++++++++--------\n 3 files changed, 62 insertions(+), 58 deletions(-)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 3cf5511..6914aa5 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -40,13 +40,13 @@ OPTIONS\n pre-receive Hook\n ----------------\n Before any ref is updated, if $GIT_DIR/hooks/pre-receive file exists\n-and is executable, it will be invoked once, with three parameters\n-per ref to be updated:\n+and is executable, it will be invoked once with no parameters.  The\n+standard input of the hook will be one line per ref to be updated:\n \n-       $GIT_DIR/hooks/pre-receive (refname sha1-old sha1-new)+\n+       sha1-old SP sha1-new SP refname LF\n \n-The refname parameter is relative to $GIT_DIR; e.g. for the master\n-head this is \"refs/heads/master\".  The two sha1 arguments after\n+The refname value is relative to $GIT_DIR; e.g. for the master\n+head this is \"refs/heads/master\".  The two sha1 values before\n each refname are the object names for the refname before and after\n the update.  Refs to be created will have sha1-old equal to 0{40},\n while refs to be deleted will have sha1-new equal to 0{40}, otherwise\n@@ -86,13 +86,14 @@ post-receive Hook\n -----------------\n After all refs were updated (or attempted to be updated), if any\n ref update was successful, and if $GIT_DIR/hooks/post-receive\n-file exists and is executable, it will be invoke once with three\n-parameters for each successfully updated ref:\n+file exists and is executable, it will be invoke once with no\n+parameters.  The standard input of the hook will be one line\n+for each successfully updated ref:\n \n-       $GIT_DIR/hooks/post-receive (refname sha1-old sha1-new)+\n+       sha1-old SP sha1-new SP refname LF\n \n-The refname parameter is relative to $GIT_DIR; e.g. for the master\n-head this is \"refs/heads/master\".  The two sha1 arguments after\n+The refname value is relative to $GIT_DIR; e.g. for the master\n+head this is \"refs/heads/master\".  The two sha1 values before\n each refname are the object names for the refname before and after\n the update.  Refs that were created will have sha1-old equal to\n 0{40}, while refs that were deleted will have sha1-new equal to\n@@ -105,18 +106,17 @@ ref listing the commits pushed to the repository:\n \n \t#!/bin/sh\n \t# mail out commit update information.\n-\twhile test $# -gt 0\n+\twhile read oval nval ref\n \tdo\n-\t\tif expr \"$2\" : '0*$' >/dev/null\n+\t\tif expr \"$oval\" : '0*$' >/dev/null\n \t\tthen\n \t\t\techo \"Created a new ref, with the following commits:\"\n-\t\t\tgit-rev-list --pretty \"$2\"\n+\t\t\tgit-rev-list --pretty \"$nval\"\n \t\telse\n \t\t\techo \"New commits:\"\n-\t\t\tgit-rev-list --pretty \"$3\" \"^$2\"\n+\t\t\tgit-rev-list --pretty \"$nval\" \"^$oval\"\n \t\tfi |\n-\t\tmail -s \"Changes to ref $1\" commit-list@mydomain\n-\t\tshift; shift; shift; # discard this ref's args\n+\t\tmail -s \"Changes to ref $ref\" commit-list@mydomain\n \tdone\n \texit 0\n \ndiff --git a/receive-pack.c b/receive-pack.c\nindex 4d7170f..7cf5878 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -79,6 +79,8 @@ static int hook_status(int code, const char *hook_name)\n \t\treturn error(\"hook fork failed\");\n \tcase -ERR_RUN_COMMAND_EXEC:\n \t\treturn error(\"hook execute failed\");\n+\tcase -ERR_RUN_COMMAND_PIPE:\n+\t\treturn error(\"hook pipe failed\");\n \tcase -ERR_RUN_COMMAND_WAITPID:\n \t\treturn error(\"waitpid failed\");\n \tcase -ERR_RUN_COMMAND_WAITPID_WRONG_PID:\n@@ -93,44 +95,44 @@ static int hook_status(int code, const char *hook_name)\n \t}\n }\n \n-static int run_hook(const char *hook_name,\n-\tstruct command *first_cmd,\n-\tint single)\n+static int run_hook(const char *hook_name)\n {\n+\tstatic char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];\n \tstruct command *cmd;\n-\tint argc, code;\n-\tconst char **argv;\n+\tstruct child_process proc;\n+\tconst char *argv[2];\n+\tint have_input = 0, code;\n \n-\tfor (argc = 0, cmd = first_cmd; cmd; cmd = cmd->next) {\n+\tfor (cmd = commands; !have_input && cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string)\n-\t\t\targc += 3;\n-\t\tif (single)\n-\t\t\tbreak;\n+\t\t\thave_input = 1;\n \t}\n \n-\tif (!argc || access(hook_name, X_OK) < 0)\n+\tif (!have_input || access(hook_name, X_OK) < 0)\n \t\treturn 0;\n \n-\targv = xmalloc(sizeof(*argv) * (2 + argc));\n \targv[0] = hook_name;\n-\tfor (argc = 1, cmd = first_cmd; cmd; cmd = cmd->next) {\n+\targv[1] = NULL;\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\tproc.argv = argv;\n+\tproc.in = -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+\tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string) {\n-\t\t\targv[argc++] = xstrdup(cmd->ref_name);\n-\t\t\targv[argc++] = xstrdup(sha1_to_hex(cmd->old_sha1));\n-\t\t\targv[argc++] = xstrdup(sha1_to_hex(cmd->new_sha1));\n+\t\t\tsize_t n = 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\tif (single)\n-\t\t\tbreak;\n \t}\n-\targv[argc] = NULL;\n-\n-\tcode = run_command_v_opt(argv,\n-\t\tRUN_COMMAND_NO_STDIN | RUN_COMMAND_STDOUT_TO_STDERR);\n-\twhile (--argc > 0)\n-\t\tfree((char*)argv[argc]);\n-\tfree(argv);\n-\n-\treturn hook_status(code, hook_name);\n+\treturn hook_status(finish_command(&proc), hook_name);\n }\n \n static int run_update_hook(struct command *cmd)\n@@ -265,7 +267,7 @@ static void execute_commands(const char *unpacker_error)\n \t\treturn;\n \t}\n \n-\tif (run_hook(pre_receive_hook, commands, 0)) {\n+\tif (run_hook(pre_receive_hook)) {\n \t\twhile (cmd) {\n \t\t\tcmd->error_string = \"pre-receive hook declined\";\n \t\t\tcmd = cmd->next;\n@@ -520,7 +522,7 @@ int main(int argc, char **argv)\n \t\t\tunlink(pack_lockfile);\n \t\tif (report_status)\n \t\t\treport(unpack_status);\n-\t\trun_hook(post_receive_hook, commands, 0);\n+\t\trun_hook(post_receive_hook);\n \t\trun_update_post_hook(commands);\n \t}\n \treturn 0;\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex c10721b..2d4b481 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -25,8 +25,8 @@ test_expect_success setup '\n \n cat >victim/.git/hooks/pre-receive <<'EOF'\n #!/bin/sh\n-echo \"$@\" >>$GIT_DIR/pre-receive.args\n-read x; printf \"$x\" >$GIT_DIR/pre-receive.stdin\n+printf \"$@\" >>$GIT_DIR/pre-receive.args\n+cat - >$GIT_DIR/pre-receive.stdin\n echo STDOUT pre-receive\n echo STDERR pre-receive >&2\n EOF\n@@ -44,8 +44,8 @@ chmod u+x victim/.git/hooks/update\n \n cat >victim/.git/hooks/post-receive <<'EOF'\n #!/bin/sh\n-echo \"$@\" >>$GIT_DIR/post-receive.args\n-read x; printf \"$x\" >$GIT_DIR/post-receive.stdin\n+printf \"$@\" >>$GIT_DIR/post-receive.args\n+cat - >$GIT_DIR/post-receive.stdin\n echo STDOUT post-receive\n echo STDERR post-receive >&2\n EOF\n@@ -80,11 +80,10 @@ test_expect_success 'hooks ran' '\n \ttest -f victim/.git/post-update.stdin\n '\n \n-test_expect_success 'pre-receive hook arguments' '\n-\techo \\\n-\t refs/heads/master $commit0 $commit1 \\\n-\t refs/heads/tofail $commit1 $commit0 \\\n-\t| diff - victim/.git/pre-receive.args\n+test_expect_success 'pre-receive hook input' '\n+\t(echo $commit0 $commit1 refs/heads/master;\n+\t echo $commit1 $commit0 refs/heads/tofail\n+\t) | diff - victim/.git/pre-receive.stdin\n '\n \n test_expect_success 'update hook arguments' '\n@@ -93,9 +92,9 @@ test_expect_success 'update hook arguments' '\n \t) | diff - victim/.git/update.args\n '\n \n-test_expect_success 'post-receive hook arguments' '\n-\techo refs/heads/master $commit0 $commit1 |\n-\tgit diff - victim/.git/post-receive.args\n+test_expect_success 'post-receive hook input' '\n+\techo $commit0 $commit1 refs/heads/master |\n+\tgit diff - victim/.git/post-receive.stdin\n '\n \n test_expect_success 'post-update hook arguments' '\n@@ -104,12 +103,15 @@ test_expect_success 'post-update hook arguments' '\n '\n \n test_expect_success 'all hook stdin is /dev/null' '\n-\t! test -s victim/.git/pre-receive.stdin &&\n \t! test -s victim/.git/update.stdin &&\n-\t! test -s victim/.git/post-receive.stdin &&\n \t! test -s victim/.git/post-update.stdin\n '\n \n+test_expect_success 'all *-receive hook args are empty' '\n+\t! test -s victim/.git/pre-receive.args &&\n+\t! test -s victim/.git/post-receive.args\n+'\n+\n test_expect_failure 'send-pack produced no output' '\n \ttest -s send.out\n '\n-- \n1.5.0.3.942.g299f\n"}]}