On Tue, Sep 16, 2014 at 03:27:12PM -0700, Junio C Hamano wrote: > Johannes Sixt <j6t@xxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html