{"thread":{"id":"40773","subject":"[PATCH] allow hooks to ignore their standard input stream","startedAt":"2015-11-11T14:39:20Z","lastAt":"2015-11-16T13:59:05Z","messageCount":8,"participants":["Clemens Buchacher","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"273192","messageId":"20151111143920.GA30409@musxeris015.imu.intel.com","threadId":"40773","inReplyTo":null,"subject":"[PATCH] allow hooks to ignore their standard input stream","fromName":"Clemens Buchacher","fromEmail":"clemens.buchacher@intel.com","sentAt":"2015-11-11T14:39:20Z","receivedAt":"2015-11-11T14:39:20Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Since ec7dbd145 (receive-pack: allow hooks to ignore its standard input\nstream) the pre-receive and post-receive hooks ignore SIGPIPE. Do the\nsame for the remaining hooks pre-push and post-rewrite, which read from\nstandard input. The same arguments for ignoring SIGPIPE apply.\n\nSigned-off-by: Clemens Buchacher <clemens.buchacher@intel.com>\n---\n builtin/commit.c |  3 +++\n transport.c      | 11 +++++++++--\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex dca09e2..f2a8b78 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -32,6 +32,7 @@\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n #include \"mailmap.h\"\n+#include \"sigchain.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [<options>] [--] <pathspec>...\"),\n@@ -1537,8 +1538,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,\n \t\treturn code;\n \tn = snprintf(buf, sizeof(buf), \"%s %s\\n\",\n \t\t     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n \twrite_in_full(proc.in, buf, n);\n \tclose(proc.in);\n+\tsigchain_pop(SIGPIPE);\n \treturn finish_command(&proc);\n }\n \ndiff --git a/transport.c b/transport.c\nindex 23b2ed6..e34ab92 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -15,6 +15,7 @@\n #include \"submodule.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n+#include \"sigchain.h\"\n \n /* rsync support */\n \n@@ -1127,6 +1128,8 @@ static int run_pre_push_hook(struct transport *transport,\n \t\treturn -1;\n \t}\n \n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n \tstrbuf_init(&buf, 256);\n \n \tfor (r = remote_refs; r; r = r->next) {\n@@ -1140,8 +1143,10 @@ static int run_pre_push_hook(struct transport *transport,\n \t\t\t r->peer_ref->name, sha1_to_hex(r->new_sha1),\n \t\t\t r->name, sha1_to_hex(r->old_sha1));\n \n-\t\tif (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {\n-\t\t\tret = -1;\n+\t\tif (write_in_full(proc.in, buf.buf, buf.len) < 0) {\n+\t\t\t/* We do not mind if a hook does not read all refs. */\n+\t\t\tif (errno != EPIPE)\n+\t\t\t\tret = -1;\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1152,6 +1157,8 @@ static int run_pre_push_hook(struct transport *transport,\n \tif (!ret)\n \t\tret = x;\n \n+\tsigchain_pop(SIGPIPE);\n+\n \tx = finish_command(&proc);\n \tif (!ret)\n \t\tret = x;\n-- \n1.9.4\n"},{"id":"273193","messageId":"20151111144222.GA24717@musxeris015.imu.intel.com","threadId":"40773","inReplyTo":"20151111143920.GA30409@musxeris015.imu.intel.com","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Clemens Buchacher","fromEmail":"clemens.buchacher@intel.com","sentAt":"2015-11-11T14:42:22Z","receivedAt":"2015-11-11T14:42:22Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Wed, Nov 11, 2015 at 03:39:20PM +0100, Clemens Buchacher wrote:\n> +\t\tif (write_in_full(proc.in, buf.buf, buf.len) < 0) {\n> +\t\t\t/* We do not mind if a hook does not read all refs. */\n> +\t\t\tif (errno != EPIPE)\n> +\t\t\t\tret = -1;\n\nI can reproduce the pipe error reliably with the test below. I did not\ninclude it in the patch since I am in doubt if we should add an optional\nsleep to the code.\n\n-->o--\ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nindex 6f9916a..8cfe59a 100755\n--- a/t/t5571-pre-push-hook.sh\n+++ b/t/t5571-pre-push-hook.sh\n@@ -109,23 +109,13 @@ test_expect_success 'push to URL' '\n        diff expected actual\n '\n\n-# Test that filling pipe buffers doesn't cause failure\n-# Too slow to leave enabled for general use\n-if false\n-then\n-       printf 'parent1\\nrepo1\\n' >expected\n-       nr=1000\n-       while test $nr -lt 2000\n-       do\n-               nr=$(( $nr + 1 ))\n-               git branch b/$nr $COMMIT3\n-               echo \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\" >>expected\n-       done\n-\n-       test_expect_success 'push many refs' '\n-               git push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n-               diff expected actual\n-       '\n-fi\n+write_script \"$HOOK\" <<\\EOF\n+exit 0\n+EOF\n+\n+test_expect_success 'hook does not consume input' '\n+    git branch noinput &&\n+    GIT_TEST_SIGPIPE=t git push parent1 noinput\n+'\n\n test_done\ndiff --git a/transport.c b/transport.c\nindex 23b2ed6..d83ef1c 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1129,6 +1129,8 @@ static int run_pre_push_hook(struct transport *transport,\n\n        strbuf_init(&buf, 256);\n\n+       if (getenv(\"GIT_TEST_SIGPIPE\"))\n+               sleep_millisec(10);\n        for (r = remote_refs; r; r = r->next) {\n                if (!r->peer_ref) continue;\n                if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;\n"},{"id":"273268","messageId":"20151113061729.GC32157@sigill.intra.peff.net","threadId":"40773","inReplyTo":"20151111144222.GA24717@musxeris015.imu.intel.com","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-13T06:17:29Z","receivedAt":"2015-11-13T06:17:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 11, 2015 at 03:42:22PM +0100, Clemens Buchacher wrote:\n\n> On Wed, Nov 11, 2015 at 03:39:20PM +0100, Clemens Buchacher wrote:\n> > +\t\tif (write_in_full(proc.in, buf.buf, buf.len) < 0) {\n> > +\t\t\t/* We do not mind if a hook does not read all refs. */\n> > +\t\t\tif (errno != EPIPE)\n> > +\t\t\t\tret = -1;\n> \n> I can reproduce the pipe error reliably with the test below. I did not\n> include it in the patch since I am in doubt if we should add an optional\n> sleep to the code.\n\nYeah, we definitely do not want to do so. The reliable way to get a\nSIGPIPE is:\n\n  1. Have one process write more than a pipe buffer's worth of data.\n\n  2. Have the other process exit without reading anything.\n\nThen even if the writer process goes first, it will block, and\neventually get EPIPE when the other side closes the descriptor.\n\nUnfortunately, the pipe buffer size varies from system to system. It's\n128K by default on modern Linux platforms. Using that is probably\nenough. The worst case is that the system has a larger buffer, and the\ntest will succeed even without the fix. But that is OK as long as\nsomebody somewhere is running the test with a smaller buffer and would\ncatch a regression.\n\nOf course, convincing git to generate 128K worth of hook input is often\nquite inefficient. The existing test in t5571 is quite painful because\nit calls \"git branch\" in a loop. We can do much better than that.\nThe test below reliably fails without your patch and passes with it, and\nseems to run reasonably quickly for me:\n\ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nindex 6f9916a..61df2f9 100755\n--- a/t/t5571-pre-push-hook.sh\n+++ b/t/t5571-pre-push-hook.sh\n@@ -109,23 +109,28 @@ test_expect_success 'push to URL' '\n \tdiff expected actual\n '\n \n-# Test that filling pipe buffers doesn't cause failure\n-# Too slow to leave enabled for general use\n-if false\n-then\n-\tprintf 'parent1\\nrepo1\\n' >expected\n-\tnr=1000\n-\twhile test $nr -lt 2000\n-\tdo\n-\t\tnr=$(( $nr + 1 ))\n-\t\tgit branch b/$nr $COMMIT3\n-\t\techo \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\" >>expected\n-\tdone\n-\n-\ttest_expect_success 'push many refs' '\n-\t\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n-\t\tdiff expected actual\n-\t'\n-fi\n+test_expect_success 'set up many-ref tests' '\n+\t{\n+\t\techo >&3 parent1 &&\n+\t\techo >&3 repo1 &&\n+\t\tnr=1000\n+\t\twhile test $nr -lt 2000\n+\t\tdo\n+\t\t\tnr=$(( $nr + 1 ))\n+\t\t\techo \"create refs/heads/b/$nr $COMMIT3\"\n+\t\t\techo >&3 \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\"\n+\t\tdone\n+\t} 3>expected | git update-ref --stdin\n+'\n+\n+test_expect_success 'filling pipe buffer does not cause failure' '\n+\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'sigpipe does not cause pre-push hook failure' '\n+\techo \"exit 0\" | write_script \"$HOOK\" &&\n+\tgit push parent1 \"refs/heads/b/*:refs/heads/c/*\"\n+'\n \n test_done\n"},{"id":"273269","messageId":"20151113061847.GD32157@sigill.intra.peff.net","threadId":"40773","inReplyTo":"20151111143920.GA30409@musxeris015.imu.intel.com","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-13T06:18:48Z","receivedAt":"2015-11-13T06:18:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 11, 2015 at 03:39:20PM +0100, Clemens Buchacher wrote:\n\n> Since ec7dbd145 (receive-pack: allow hooks to ignore its standard input\n> stream) the pre-receive and post-receive hooks ignore SIGPIPE. Do the\n> same for the remaining hooks pre-push and post-rewrite, which read from\n> standard input. The same arguments for ignoring SIGPIPE apply.\n\nThe reasoning and the patch itself look good to me. I posted some\nthoughts on tests in response to your other message.\n\n-Peff\n"},{"id":"273273","messageId":"20151113093303.GA4111@musxeris015.imu.intel.com","threadId":"40773","inReplyTo":"20151113061729.GC32157@sigill.intra.peff.net","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Clemens Buchacher","fromEmail":"clemens.buchacher@intel.com","sentAt":"2015-11-13T09:33:03Z","receivedAt":"2015-11-13T09:33:03Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Fri, Nov 13, 2015 at 01:17:29AM -0500, Jeff King wrote:\n> \n> The test below reliably fails without your patch and passes with it, and\n> seems to run reasonably quickly for me:\n\nThank you. I confirm the same behavior on my system. Below I have added\nyour change to the patch.\n\n-->o--\nSince ec7dbd145 (receive-pack: allow hooks to ignore its standard input stream)\nthe pre-receive and post-receive hooks ignore SIGPIPE. Do the same for the\nremaining hooks pre-push and post-rewrite, which read from standard input. The\nsame arguments for ignoring SIGPIPE apply.\n\nPerformance improvements which allow us to enable the test by\ndefault by Jeff King.\n\nSigned-off-by: Clemens Buchacher <clemens.buchacher@intel.com>\n---\n builtin/commit.c         |  3 +++\n t/t5571-pre-push-hook.sh | 41 +++++++++++++++++++++++------------------\n transport.c              | 11 +++++++++--\n 3 files changed, 35 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex dca09e2..f2a8b78 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -32,6 +32,7 @@\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n #include \"mailmap.h\"\n+#include \"sigchain.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [<options>] [--] <pathspec>...\"),\n@@ -1537,8 +1538,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,\n \t\treturn code;\n \tn = snprintf(buf, sizeof(buf), \"%s %s\\n\",\n \t\t     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n \twrite_in_full(proc.in, buf, n);\n \tclose(proc.in);\n+\tsigchain_pop(SIGPIPE);\n \treturn finish_command(&proc);\n }\n \ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nindex 6f9916a..61df2f9 100755\n--- a/t/t5571-pre-push-hook.sh\n+++ b/t/t5571-pre-push-hook.sh\n@@ -109,23 +109,28 @@ test_expect_success 'push to URL' '\n \tdiff expected actual\n '\n \n-# Test that filling pipe buffers doesn't cause failure\n-# Too slow to leave enabled for general use\n-if false\n-then\n-\tprintf 'parent1\\nrepo1\\n' >expected\n-\tnr=1000\n-\twhile test $nr -lt 2000\n-\tdo\n-\t\tnr=$(( $nr + 1 ))\n-\t\tgit branch b/$nr $COMMIT3\n-\t\techo \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\" >>expected\n-\tdone\n-\n-\ttest_expect_success 'push many refs' '\n-\t\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n-\t\tdiff expected actual\n-\t'\n-fi\n+test_expect_success 'set up many-ref tests' '\n+\t{\n+\t\techo >&3 parent1 &&\n+\t\techo >&3 repo1 &&\n+\t\tnr=1000\n+\t\twhile test $nr -lt 2000\n+\t\tdo\n+\t\t\tnr=$(( $nr + 1 ))\n+\t\t\techo \"create refs/heads/b/$nr $COMMIT3\"\n+\t\t\techo >&3 \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\"\n+\t\tdone\n+\t} 3>expected | git update-ref --stdin\n+'\n+\n+test_expect_success 'filling pipe buffer does not cause failure' '\n+\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'sigpipe does not cause pre-push hook failure' '\n+\techo \"exit 0\" | write_script \"$HOOK\" &&\n+\tgit push parent1 \"refs/heads/b/*:refs/heads/c/*\"\n+'\n \n test_done\ndiff --git a/transport.c b/transport.c\nindex 23b2ed6..e34ab92 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -15,6 +15,7 @@\n #include \"submodule.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n+#include \"sigchain.h\"\n \n /* rsync support */\n \n@@ -1127,6 +1128,8 @@ static int run_pre_push_hook(struct transport *transport,\n \t\treturn -1;\n \t}\n \n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n \tstrbuf_init(&buf, 256);\n \n \tfor (r = remote_refs; r; r = r->next) {\n@@ -1140,8 +1143,10 @@ static int run_pre_push_hook(struct transport *transport,\n \t\t\t r->peer_ref->name, sha1_to_hex(r->new_sha1),\n \t\t\t r->name, sha1_to_hex(r->old_sha1));\n \n-\t\tif (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {\n-\t\t\tret = -1;\n+\t\tif (write_in_full(proc.in, buf.buf, buf.len) < 0) {\n+\t\t\t/* We do not mind if a hook does not read all refs. */\n+\t\t\tif (errno != EPIPE)\n+\t\t\t\tret = -1;\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1152,6 +1157,8 @@ static int run_pre_push_hook(struct transport *transport,\n \tif (!ret)\n \t\tret = x;\n \n+\tsigchain_pop(SIGPIPE);\n+\n \tx = finish_command(&proc);\n \tif (!ret)\n \t\tret = x;\n-- \n1.9.4\n"},{"id":"273299","messageId":"20151113232320.GB16173@sigill.intra.peff.net","threadId":"40773","inReplyTo":"20151113093303.GA4111@musxeris015.imu.intel.com","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-13T23:23:20Z","receivedAt":"2015-11-13T23:23:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 13, 2015 at 10:33:03AM +0100, Clemens Buchacher wrote:\n\n> Since ec7dbd145 (receive-pack: allow hooks to ignore its standard input stream)\n> the pre-receive and post-receive hooks ignore SIGPIPE. Do the same for the\n> remaining hooks pre-push and post-rewrite, which read from standard input. The\n> same arguments for ignoring SIGPIPE apply.\n> \n> Performance improvements which allow us to enable the test by\n> default by Jeff King.\n\nI actually did add a new test. The existing one was basically this:\n\n> +test_expect_success 'filling pipe buffer does not cause failure' '\n> +\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n> +\ttest_cmp expected actual\n> +'\n\nIt actually _does_ read all of the input, but I guess is making sure we\ncall write() in a loop. I don't know if this is even worth keeping.\n\nCan you think of a good reason that it is checking something\ninteresting?\n\n> +test_expect_success 'sigpipe does not cause pre-push hook failure' '\n> +\techo \"exit 0\" | write_script \"$HOOK\" &&\n> +\tgit push parent1 \"refs/heads/b/*:refs/heads/c/*\"\n> +'\n\nThis is the new one which checks your code.\n\n-Peff\n"},{"id":"273355","messageId":"20151116080558.GA23851@musxeris015.imu.intel.com","threadId":"40773","inReplyTo":"20151113232320.GB16173@sigill.intra.peff.net","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Clemens Buchacher","fromEmail":"clemens.buchacher@intel.com","sentAt":"2015-11-16T08:05:58Z","receivedAt":"2015-11-16T08:05:58Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi Junio,\n\nI believe we have finalized the discussion on this patch. Please apply\n\nOn Fri, Nov 13, 2015 at 06:23:20PM -0500, Jeff King wrote:\n> \n> > +test_expect_success 'filling pipe buffer does not cause failure' '\n> > +\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n> > +\ttest_cmp expected actual\n> > +'\n> \n> It actually _does_ read all of the input, but I guess is making sure we\n> call write() in a loop. I don't know if this is even worth keeping.\n> \n> Can you think of a good reason that it is checking something\n> interesting?\n\nNo, I also cannot think of a good reason to keep it. Here is the patch\nwith the test above removed.\n\nBest regards,\nClemens\n\n--o<--\nSince ec7dbd145 (receive-pack: allow hooks to ignore its standard input stream)\nthe pre-receive and post-receive hooks ignore SIGPIPE. Do the same for the\nremaining hooks pre-push and post-rewrite, which read from standard input. The\nsame arguments for ignoring SIGPIPE apply.\n\nInclude test by Jeff King which checks that SIGPIPE does not cause\npre-push hook failure. With the use of git update-ref --stdin it is fast\nenough to be enabled by default.\n\nSigned-off-by: Clemens Buchacher <clemens.buchacher@intel.com>\n---\n builtin/commit.c         |  3 +++\n t/t5571-pre-push-hook.sh | 33 +++++++++++++++------------------\n transport.c              | 11 +++++++++--\n 3 files changed, 27 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex dca09e2..f2a8b78 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -32,6 +32,7 @@\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n #include \"mailmap.h\"\n+#include \"sigchain.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [<options>] [--] <pathspec>...\"),\n@@ -1537,8 +1538,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,\n \t\treturn code;\n \tn = snprintf(buf, sizeof(buf), \"%s %s\\n\",\n \t\t     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n \twrite_in_full(proc.in, buf, n);\n \tclose(proc.in);\n+\tsigchain_pop(SIGPIPE);\n \treturn finish_command(&proc);\n }\n \ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nindex 6f9916a..ba975bb 100755\n--- a/t/t5571-pre-push-hook.sh\n+++ b/t/t5571-pre-push-hook.sh\n@@ -109,23 +109,20 @@ test_expect_success 'push to URL' '\n \tdiff expected actual\n '\n \n-# Test that filling pipe buffers doesn't cause failure\n-# Too slow to leave enabled for general use\n-if false\n-then\n-\tprintf 'parent1\\nrepo1\\n' >expected\n-\tnr=1000\n-\twhile test $nr -lt 2000\n-\tdo\n-\t\tnr=$(( $nr + 1 ))\n-\t\tgit branch b/$nr $COMMIT3\n-\t\techo \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\" >>expected\n-\tdone\n-\n-\ttest_expect_success 'push many refs' '\n-\t\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n-\t\tdiff expected actual\n-\t'\n-fi\n+test_expect_success 'set up many-ref tests' '\n+\t{\n+\t\tnr=1000\n+\t\twhile test $nr -lt 2000\n+\t\tdo\n+\t\t\tnr=$(( $nr + 1 ))\n+\t\t\techo \"create refs/heads/b/$nr $COMMIT3\"\n+\t\tdone\n+\t} | git update-ref --stdin\n+'\n+\n+test_expect_success 'sigpipe does not cause pre-push hook failure' '\n+\techo \"exit 0\" | write_script \"$HOOK\" &&\n+\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\"\n+'\n \n test_done\ndiff --git a/transport.c b/transport.c\nindex 23b2ed6..e34ab92 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -15,6 +15,7 @@\n #include \"submodule.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n+#include \"sigchain.h\"\n \n /* rsync support */\n \n@@ -1127,6 +1128,8 @@ static int run_pre_push_hook(struct transport *transport,\n \t\treturn -1;\n \t}\n \n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n \tstrbuf_init(&buf, 256);\n \n \tfor (r = remote_refs; r; r = r->next) {\n@@ -1140,8 +1143,10 @@ static int run_pre_push_hook(struct transport *transport,\n \t\t\t r->peer_ref->name, sha1_to_hex(r->new_sha1),\n \t\t\t r->name, sha1_to_hex(r->old_sha1));\n \n-\t\tif (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {\n-\t\t\tret = -1;\n+\t\tif (write_in_full(proc.in, buf.buf, buf.len) < 0) {\n+\t\t\t/* We do not mind if a hook does not read all refs. */\n+\t\t\tif (errno != EPIPE)\n+\t\t\t\tret = -1;\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1152,6 +1157,8 @@ static int run_pre_push_hook(struct transport *transport,\n \tif (!ret)\n \t\tret = x;\n \n+\tsigchain_pop(SIGPIPE);\n+\n \tx = finish_command(&proc);\n \tif (!ret)\n \t\tret = x;\n-- \n1.9.4\n"},{"id":"273366","messageId":"20151116135905.GB13471@sigill.intra.peff.net","threadId":"40773","inReplyTo":"20151116080558.GA23851@musxeris015.imu.intel.com","subject":"Re: [PATCH] allow hooks to ignore their standard input stream","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-16T13:59:05Z","receivedAt":"2015-11-16T13:59:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 16, 2015 at 09:05:58AM +0100, Clemens Buchacher wrote:\n\n> Hi Junio,\n> \n> I believe we have finalized the discussion on this patch. Please apply\n\nJunio is out for a bit, and I'm acting as maintainer. I've picked up\nyour patch but haven't pushed anything out yet.\n\n> No, I also cannot think of a good reason to keep it. Here is the patch\n> with the test above removed.\n\nThanks, this looks good to me.\n\n-Peff\n"}]}