Re: [PATCH v3 3/9] pack-revindex.c: instrument loading on-disk reverse index

> On Thu, Jan 20, 2022 at 10:15:45AM -0800, Jonathan Tan wrote:
> > Taylor Blau <me@ttaylorr.com> writes:
> > > In a subsequent commit, we'll use the MIDX's new 'RIDX' chunk as a
> > > source for the reverse index's data. But it will be useful for tests to
> > > be able to determine whether the reverse index was loaded from the
> > > separate .rev file, or from a chunk within the MIDX.
> >
> > As for this patch and all subsequent patches, we (at $DAYJOB) discussed
> > during our Review Club the idea of not supporting .rev files altogether.
> > Firstly, because of this bug, we cannot fully trust .rev files anymore.
> > And even if we decided to trust it (or to trust it after some
> > verification step that I can't think of), that file would only be useful
> > for a short time until a regularly scheduled maintenance step
> > regenerates the bitmaps anyway.
> 
> I assume that you're talking about MIDX .rev files here, not the usual
> packfile ones which are only related in the sense that they follow the
> same file format, but are otherwise distinct.

> We could drop support for the MIDX .rev files, but I do not think it is
> a very good idea. There are tagged versions of Git which have .rev files
> out in the wild, so suddenly dropping support for them would mean that
> existing repositories would no longer be able to read their multi-pack
> bitmaps after upgrading to a version of Git which doesn't include
> support for separate .rev files.
> 
> So there are some backwards compatibility concerns there, and I think
> that that alone means we can't do it.
> 
> In order to maintain good test coverage of the soon-to-be deprecated
> external MIDX .rev files, this patch series adds some additional testing
> knobs to make sure that we're thoroughly exercising both cases.
> 
> It's somewhat unfortunate that we are stuck with on-disk .rev files for
> MIDXs, since it opened the door for this bug to manifest itself. But I
> am not comfortable getting rid of them at this point.