On Fri, Apr 17, 2020 at 8:06 PM Jeff King <peff@xxxxxxxx> wrote:
>
> On Fri, Apr 17, 2020 at 11:40:30AM -0600, Taylor Blau wrote:
>
> > > What do you think about something like:
> > >
> > > [promisorFilter "noBlobs"]
> > > type = blob:none
> > > uploadpack = true # maybe "allow" could also mean "true" here
> > > ...
> > > ?
> >
> > I'm not sure about introducing a layer of indirection here with
> > "noBlobs". It's nice that it could perhaps be enabled/disabled for
> > different builtins (e.g., by adding 'revList = false', say), but I'm not
> > convinced that this is improving all of those cases, either.
>
> Yeah, I don't like forcing the user to invent a subsection name. My
> first thought was to suggest:
>
> [promisorFilter "blob:none"]
> uploadpack = true
>
> but your tree example shows why that gets awkward: there are more keys
> than just "allow this".
I like your first thought better than something that starts with
"uploadPack". And I think if we let people find a subsection name (as
what I suggest) they might indeed end up with something like:
[promisorFilter "blob:none"]
type = blob:none
uploadpack = true
as they might lack inspiration. As filters are becoming more and more
complex though, people might find it much simpler to use the
subsection name in commands if we let them do that. For example we
already allow:
git rev-list --filter=combine:<filter1>+<filter2>+...<filterN> ...
which could be simplified to:
git rev-list --filter=combinedFilter ...
(where "combinedFilter" is defined in the config with
"type=combine:<filter1>+<filter2>+...<filterN>".)
[...]
> > We're not using them, but without them we suffer from a problem that if
> > we can get a SIGPIPE when writing the "sorry, I don't support that
> > filter" message back to the client, then they won't receive it.
> >
> > Szeder's patches help address that issue by catching the SIGPIPE and
> > popping off enough from the client buffer so that we can write the
> > message out before dying.
>
> I definitely think we should pursue that patch, but it really can be
> done orthogonally. It's an existing bug that affects other instances
> where upload-pack returns an error. The tests can work around it with
> "test_must_fail ok=sigpipe" in the meantime.
Ok, maybe I will take a look a this one then.
Thanks,
Christian.
