Re: [PATCH v2 2/3] push: Add support for pre-push hooks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 15, 2013, 00:36 UTC
- Message-ID
- <7vip6z7056.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1358054224-7710-3-git-send-email-aaron@schrab.com>
Aaron Schrab <aaron@schrab.com> writes:
Show 19 quoted lines
> t/t5571-pre-push-hook.sh | 129 +++++++++++++++++++++++++++++++++++++++++++++ > diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh > new file mode 100755 > index 0000000..d68fed7 > --- /dev/null > +++ b/t/t5571-pre-push-hook.sh > @@ -0,0 +1,129 @@ > +#!/bin/sh > + > +test_description='check pre-push hooks' > +. ./test-lib.sh > + > +# Setup hook that always succeeds > +HOOKDIR="$(git rev-parse --git-dir)/hooks" > +HOOK="$HOOKDIR/pre-push" > +mkdir -p "$HOOKDIR" > +write_script "$HOOK" <<EOF > +exit 0 > +EOF
As this script is expected to read from the pipe, if this exits before the parent has a chance to write to the pipe, the parent can be killed with sigpipe.
At least the attached patch is necessary.
In the longer term, we may want to discuss what should happen when the hook exited without even reading what we fed. My gut feeling is that we can still trust its exit status (a hook that was badly coded so it wanted to read from us and use that information to decide but somehow died before fully reading from us is not likely to exit with zero status, so we wouldn't diagnosing breakage as a success), but there may be downsides for being that lax.
If we decide we want to be lax, then the call site of this hook and the pre-receive hook (is there any other "take info from the standard input" hook?) need to be modified so that they ignore sigpipe, I think.
There was a related discussion around this issue about a year ago.
http://thread.gmane.org/gmane.comp.version-control.git/180346/focus=186291
t/t5571-pre-push-hook.sh | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh index d68fed7..577d252 100755 --- a/t/t5571-pre-push-hook.sh +++ b/t/t5571-pre-push-hook.sh @@ -8,6 +8,7 @@ HOOKDIR="$(git rev-parse --git-dir)/hooks" HOOK="$HOOKDIR/pre-push" mkdir -p "$HOOKDIR" write_script "$HOOK" <<EOF +cat >/dev/null exit 0 EOF @@ -19,6 +20,7 @@ test_expect_success 'setup' ' git push parent1 HEAD:foreign ' write_script "$HOOK" <<EOF +cat >/dev/null exit 1 EOF @@ -38,6 +40,7 @@ COMMIT2="$(git rev-parse HEAD)" export COMMIT2 write_script "$HOOK" <<'EOF' +cat >/dev/null echo "$1" >actual echo "$2" >>actual cat >>actual