Re: [RFC PATCH] upload-pack: fix filter options scope

[Taylor Blau <me@ttaylorr.com>]

On Thu, May 07, 2020 at 10:47:10AM -0400, Jeff King wrote:
> On Thu, May 07, 2020 at 02:32:39PM +0200, Christian Couder wrote:
>
> > > A client who does this is stupid and wrong (and I didn't check the
> > > protocol spec, but it could very well be violating the spec). So I don't
> > > think it's _that_ big a deal. But it would be nice if all of those
> > > globals got moved into upload_pack_data, and both v1 and v2 just used it
> > > consistently.
> >
> > Unfortunately as I discuss a bit above 'struct upload_pack_data' is
> > not used by v1, only by v2. I think making v1 use upload_pack_data
> > might be nice, but it's a separate issue. So I am tempted to just
> > improve the commit message, adding some information from you and from
> > this discussion, and then re-sending.
> >
> > Another intermediate solution would be to add filter_options to
> > 'struct upload_pack_data' for v2 and to use filter_options directly
> > for v1.
>
> I think we do want the v1 path to use upload_pack_data in the long run,
> just to keep things less confusing.
>
> But if you don't want to go that far now, I'd strongly prefer the second
> option, pushing v2 towards how we ultimately want it to look, and
> plumbing a local variable through v1 paths as necessary. Or perhaps just
> renaming the global to filter_options_v1 or something, so that v2 code
> paths aren't tempted to use it.

I would be very much in favor of that direction. Thanks for CC-ing me on
this thread, since any changes here will obviously create merge
conflicts on my end when I send the "configure allowed object filters"
patches.

> -Peff

Thanks,
Taylor