Re: [PATCH v2 1/8] packfile: introduce 'find_kept_pack_entry()'

On Wed, Feb 03, 2021 at 10:58:50PM -0500, Taylor Blau wrote:

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).

In particular, an new 'git repack' mode which ensures the resulting

Nit (not worth re-rolling): s/an new/a new/

There is a gotcha when looking up objects that are duplicated in kept
and non-kept packs, particularly when the MIDX stores the non-kept
version and the caller asked for kept objects only. This could be
resolved by teaching the MIDX to resolve duplicates by always favoring
the kept pack (if one exists), but this breaks an assumption in existing
MIDXs, and so it would require a format change.

I don't think this would be possible without a major rethink of how
midxs work. The "keep" property of a pack is not set in stone when the
midx is created. You could add a ".keep" file to one of its packs later,
or even mark one as an in-core keep on the fly. But the duplicate
resolution happens at creation.

So maybe your "breaks an assumption" is the notion that we do not store
duplicate information at all in the midx. If so, then I agree. :) But
I'd also call fixing that more than just a format change.

(None of which changes your point, which isn't that it isn't worth
pursuing that direction).

-Peff