Re: [PATCH v3 00/16] midx: implement a multi-pack reverse index

On Mon, Mar 29, 2021 at 09:05:33AM -0400, Jeff King wrote:

On Thu, Mar 11, 2021 at 12:04:31PM -0500, Taylor Blau wrote:

quoted

Here is another reroll of my series to implement a reverse index in
preparation for multi-pack reachability bitmaps. The previous version
was based on 'ds/chunked-file-api', but that topic has since been merged
to 'master'. This series is now built directly on top of 'master'.

I gave the whole thing another careful read. Most of what I found were
small nits, but enough that I think one more re-roll is worth it.

Thanks. I agree that another re-roll is worth it. I have one prepared
locally, and I just had one outstanding question in:

    https://lore.kernel.org/git/YGI6ySogGoYZi66A@nand.local/ (local)

that I'll wait on your reply to before sending a reroll.

The biggest question is what we want to happen next. As you note, the
concept of a midx .rev file is useless until we have the matching
.bitmap file. So we _could_ let this sit in next while the dependent
bitmap topic is reviewed, and then merge them down together. But I'm
inclined to treat this as an independent topic that can get merged to
master on its own, since the early cleanups are valuable on their own,
and the .rev parts at the end, even if dead, won't hurt anything.

That matches what I was hoping for. I think the clean-ups are worth it
on their own, but I also think it's a good idea to take the whole
series, since it means there's one less long-running branch in flight
while we review the MIDX bitmaps topic.

(FWIW, I can also see an argument in the other direction along the lines
of "we may discover something later on that requires us to change the
way multi-pack .rev files work". I think that such an outcome is fairly
unlikely, but worth considering anyway).

Thanks,
Taylor