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
