On Tue, May 09, 2017 at 09:44:50AM +0900, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > On Mon, May 08, 2017 at 01:56:27PM +0900, Junio C Hamano wrote: > > > >> Surely, even if we need to exclude some objects from an existing > >> packfile due to these selection options, we should be able to reuse > >> the non-excluded part, no? The end result may involve having to > >> pick and reuse more and smaller slices from existing packfiles, > >> which may be much less efficient, but it is no immediately obvious > >> to me if it leads to "need to disable". I would understand it if it > >> were "it becomes much less efficient and we are better off not using > >> the bitmap code at all", though. > > > > Yes, it's this last bit. The main win of the packfile reuse code is that > > it builds on the bitmaps to avoid doing as much per-object work as > > possible. So the objects don't even get added to the list of "struct > > object_entry", and we never consider them for the "should they be in the > > result" checks beyond the have/want computation done by the bitmaps. > > > > We could add those checks in, but what's the point? The idea of the > > reuse code is to be a fast path for serving vanilla clones. Searching > > through all of the packfiles for a .keep entry is the antithesis of > > that. > > Ah, OK, and now I understand why you called this a "bug" (which is > older and do not need to be addressed as part of 2.13) in the > original message. The new tests check requests that ought to > produce an empty packfile as the result actually do, but with the > current code, the reuse code does not work with --local and friends > and ends up including what was requested to be excluded.
Right. Did you want me to try re-wording the commit message, or does it make sense now? -Peff
