Re: [PATCH 2/4] upload-pack.c: allow banning certain object filter(s)

From: Jeff King <peff@peff.net>
Date: Jul 20, 2020, 20:21 UTC
Message-ID: <20200720202140.GA2059161@coredump.intra.peff.net>
In-Reply-To: <20200720200739.GB91942@syl.lan>

On Mon, Jul 20, 2020 at 04:07:39PM -0400, Taylor Blau wrote:

> Heh. I was hoping to be rid of those errors with Christian's patches,
> but it sounds like the problem is coming from outside of 'upload-pack'
> and instead in 'clone'.

Yes, it's definitely on the client side.

Show 14 quoted lines

> That reasoning seems sound to me, but I'd rather not touch clone in this
> patch series if I don't have to. What I'd rather do is something like:
> 
>   - Introduce this patch series with the 'test_must_fail ok=sigpipe',
>     and no error checking.
> 
>   - Modify clone to swallow these errors and eat a packet or two.
> 
>   - Then, drop the 'ok=sigpipe' from t5616 after 'git clone' is a little
>     bit smarter here.
> 
> Maybe more steps than is strictly necessary, but I think it keeps the
> scope of the review on this series reasonable, which is a tradeoff that
> I'm willing to make.

I think that's reasonable.

It doesn't look like the "pump the read side after EPIPE" patch is even on the "seen" branch yet, and you'd need to further extend it to clone. It's definitely a big enough change that it's better not to lump it in with this series.

-Peff