Re: [PATCH] allow hooks to ignore their standard input stream

From: Clemens Buchacher <clemens.buchacher@intel.com>
Date: Nov 11, 2015, 14:42 UTC
Message-ID: <20151111144222.GA24717@musxeris015.imu.intel.com>
In-Reply-To: <20151111143920.GA30409@musxeris015.imu.intel.com>

On Wed, Nov 11, 2015 at 03:39:20PM +0100, Clemens Buchacher wrote:

> +		if (write_in_full(proc.in, buf.buf, buf.len) < 0) {
> +			/* We do not mind if a hook does not read all refs. */
> +			if (errno != EPIPE)
> +				ret = -1;

I can reproduce the pipe error reliably with the test below. I did not include it in the patch since I am in doubt if we should add an optional sleep to the code.

-->o--

diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
index 6f9916a..8cfe59a 100755
--- a/t/t5571-pre-push-hook.sh
+++ b/t/t5571-pre-push-hook.sh
@@ -109,23 +109,13 @@ test_expect_success 'push to URL' '
        diff expected actual
 '

-# Test that filling pipe buffers doesn't cause failure
-# Too slow to leave enabled for general use
-if false
-then
-       printf 'parent1\nrepo1\n' >expected
-       nr=1000
-       while test $nr -lt 2000
-       do
-               nr=$(( $nr + 1 ))
-               git branch b/$nr $COMMIT3
-               echo "refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40" >>expected
-       done
-
-       test_expect_success 'push many refs' '
-               git push parent1 "refs/heads/b/*:refs/heads/b/*" &&
-               diff expected actual
-       '
-fi
+write_script "$HOOK" <<\EOF
+exit 0
+EOF
+
+test_expect_success 'hook does not consume input' '
+    git branch noinput &&
+    GIT_TEST_SIGPIPE=t git push parent1 noinput
+'

 test_done
diff --git a/transport.c b/transport.c
index 23b2ed6..d83ef1c 100644
--- a/transport.c
+++ b/transport.c
@@ -1129,6 +1129,8 @@ static int run_pre_push_hook(struct transport *transport,

        strbuf_init(&buf, 256);

+       if (getenv("GIT_TEST_SIGPIPE"))
+               sleep_millisec(10);
        for (r = remote_refs; r; r = r->next) {
                if (!r->peer_ref) continue;
                if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;