Subject: Re: [PATCH 01/10] packfile: introduce 'find_kept_pack_entry()'

From: Junio C Hamano <[email-masked]>
Date: Thu, 28 Jan 2021 18:33:10 -0800

Taylor Blau <[email-masked]> writes:

> Future callers will want a function to fill a 'struct pack_entry' for a
> given object id but _only_ from its position in any kept pack(s). They
> could accomplish this by calling 'find_pack_entry()' and checking
> whether the found pack is kept or not, but this is insufficient, since
> there may be duplicate objects (and the mru cache makes it unpredictable
> which variant we'll get).

I wonder if we eventually need a callback interface to walk _all_
pack entries for a given object, so that "I am only interested in
instances in kept packs" will be under total control of the callers.
As it stands, it is "just grab any one that is in a kept pack, any
one of them is fine", which is almost just of as narrow utility as
the original's "just grab the first one---any one of them is fine",
the latter of which is "insufficient" as the log message says.

But this (in the context of the remainder of the series) might be
sufficient, at least for now.

> Teach this new function to treat the two different kinds of kept packs
> (on disk ones with .keep files, as well as in-core ones which are set by
> manually poking the 'pack_keep_in_core' bit) separately. This will
> become important for callers that only want to respect a certain kind of
> kept pack.

Or maybe not ;-)

If there are notable relationship between on-disk and in-core kept
packs (e.g. "the set of on-disk kept packs is a subset of in-core
kept packs", "usually on-disk kept packs get in-core kept bit upon
their packed_git instances are populated, but we can drop the bit at
runtime, so on-disk and in-core are pretty much independent and
there is no notable relationship"), it must be explained upfront to
help the reader form a sensible world view.

> Introduce 'find_kept_pack_entry()' which behaves like
> 'find_pack_entry()', except that it skips over packs which are not
> marked kept. Callers will be added in subsequent patches.
>
> Co-authored-by: Jeff King <[email-masked]>
> Signed-off-by: Jeff King <[email-masked]>
> Signed-off-by: Taylor Blau <[email-masked]>

Fun.

Thanks.