Subject: Re: [RFC] allowing hooks to ignore input?

From: Jeff King <[email-masked]>
Date: Thu, 18 Sep 2014 07:14:12 -0400

On Tue, Sep 16, 2014 at 03:27:12PM -0700, Junio C Hamano wrote:

> Johannes Sixt <[email-masked]> writes:
> 
> > I think this is a good move. Hooks are written by users, who sometimes
> > are not clueful enough.
> 
> Thanks for a sanity check.  I do not think it is about cluefulness
> in this particular case.  A rule that is not meaningfully enforced
> by reliably failing offenders is a rule that is hard to follow if a
> clueful user wanted to.

This looks good to me. Here's a real-world example of somebody getting
bitten by this (I _thought_ we had dealt with it then, but apparently
not):

  http://article.gmane.org/gmane.comp.version-control.git/186287

> This round comes with a test, but depending on the size of your pipe
> buffer and context switching race, an unpatched Git may pass it
> (reducing the test_seq number down to say 199 would certainly make
> it pass without the fix most of the time).

I took a look at your test. Having worked on racy sigpipe tests
recently, I think the only way to guarantee failure is to make sure you
fill up the pipe buffer. My experiments showed this was typically 64K on
a variety of Linux systems, but I think can be bumped up at runtime.

When working on the sanitized_signals() test, we decided to just pick an
arbitrarily high number like 1MB and use that (ideally you would send
infinite data until you get SIGPIPE, but the failure mode for your test
is then that it doesn't finish :-/).

You have things much harder and much easier here. Harder, in that you
can only convince git to send ~100 bytes per ref, so you would need a
lot of refs (and thus it's expensive to over-compensate). But easier, in
that you do not need to _reliably_ catch SIGPIPE. You only need to catch
it often enough that somebody notices if the rest is broken, not catch
it every time to ensure success.

So I think it is OK as-is. You should be generating ~91K of ref data
(refname + two sha1s + spaces and newline), and I can't imagine many
systems have a pipe buffer bigger than 64K. If they do, the only
downside is that those systems might only intermittently catch a
regression.

-Peff