Re: [RFC] allowing hooks to ignore input?

[Johannes Sixt <j6t@kdbg.org>]

Am 13.09.2014 um 00:48 schrieb Junio C Hamano:
> The pre-receive and post-receive hooks were designed to be an
> improvement over old style update and post-update hooks that used to
> take the update information on the command line and were limited by
> the command line length limit.  They take the same information from
> their standard input.  It has been mandatory for these new style
> hooks to consume the update information fully from the standard
> input stream.  Otherwise, they would risk killing the receive-pack
> process via SIGPIPE.
> 
> This is easy, and it has already been done by existing hooks that
> are written correctly, to work around, if a hook does not want to
> look at all the information, by sending its standard input to
> /dev/null (perhaps a niche use of hook might need to know only the
> fact that a push was made, without having to know what objects have
> been pushed to update which refs).
> 
> However, because there is no good way to consistently fail hooks
> that do not consume the input fully, it can lead to a hard to
> diagnose "once in a blue moon" phantom failure.
> 
> I wonder if this "you must consume the input fully", which is a
> mandate that is not enforced strictly, is not helping us to catch
> mistakes in hooks more than it is hurting us.  Perhaps we can do
> something like the attached patch to make it optional for them to
> read the input we feed?
> 
> I dunno.
> 
>  builtin/receive-pack.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 30560a7..9d9d16d 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -17,6 +17,7 @@
>  #include "version.h"
>  #include "tag.h"
>  #include "gpg-interface.h"
> +#include "sigchain.h"
>  
>  static const char receive_pack_usage[] = "git receive-pack <git-dir>";
>  
> @@ -500,6 +501,8 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
>  		return code;
>  	}
>  
> +	sigchain_push(SIGPIPE, SIG_IGN);
> +
>  	while (1) {
>  		const char *buf;
>  		size_t n;
> @@ -511,6 +514,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
>  	close(proc.in);
>  	if (use_sideband)
>  		finish_async(&muxer);
> +
> +	sigchain_pop(SIGPIPE);
> +
>  	return finish_command(&proc);
>  }

I think this is a good move. Hooks are written by users, who sometimes
are not clueful enough.

But what do our writers do when they fail with EPIPE? Die? If so, this
patch alone is not sufficient.

-- Hannes