On Sat, Dec 15, 2018 at 09:31:15AM +0900, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > I actually started by doing that, but "struct serve_options" is not > > currently known by ls-refs.c, upload-pack.c, etc. So they'd have to > > start including "serve.h". I don't think that's the end of the world, > > but it felt like a funny mutual circular to me (my mental model now is > > that ls-refs, upload-pack, etc are low-level commands, tied together by > > the "serve" stuff). > > That matches my mental model, too. I think the difference between > us is what *_opt struct is. I viewed that it was like diff_options > struct where the driving machinery keeps state of the ongoing > operation performed by lower level routines to fulfill the request > by the API caller, i.e. holding both wish from the caller, and > scratchpad data for the mchinery and the lower level routine to > communicate with each other. > > And the new field feels like the last "scratchpad used by the > machinery to tell lower-level ls-refs helper what context it is > operting in".
Yeah, I agree that such a "pass this through" struct full of options and context would make sense. I just wouldn't tie it to the "serve" machinery. Did you read the side-thread between me and Jonathan? Another option here is to just have ls-refs assume that the client will tell it the context (and assume uploadpack for now, since that's all that v2 currently does). That would make this patch go away entirely. :) -Peff
