tb/midx-bitmap-corruption-fix (was: Re: What's cooking in git.git (Jan 2022, #03; Thu, 13))

[Taylor Blau <me@ttaylorr.com>]

On Thu, Jan 13, 2022 at 04:48:59PM -0800, Junio C Hamano wrote:
> * tb/midx-bitmap-corruption-fix (2022-01-04) 9 commits
>  - pack-bitmap.c: gracefully fallback after opening pack/MIDX
>  - midx: read `RIDX` chunk when present
>  - t/lib-bitmap.sh: parameterize tests over reverse index source
>  - t5326: move tests to t/lib-bitmap.sh
>  - t5326: extract `test_rev_exists`
>  - t5326: drop unnecessary setup
>  - pack-revindex.c: instrument loading on-disk reverse index
>  - midx.c: make changing the preferred pack safe
>  - t5326: demonstrate bitmap corruption after permutation
>
>  A bug that made multi-pack bitmap and the object order out-of-sync
>  (hence the .midx data gets corrupted) has been fixed.
>
>  Waiting for a hopefully final review.
>  cf. <Ydceeo33Yt4N%2FbrN@nand.local>
>  source: <cover.1641320129.git.me@ttaylorr.com>

I would really like to get this into 2.35 since it's fixing an important
source of repository corruption, but I think it should have a careful
round of review before merging. And it is pretty late into the cycle
anyway, so we may be too late to merge the whole thing.

But the first two patches:

  - midx.c: make changing the preferred pack safe
  - t5326: demonstrate bitmap corruption after permutation

could be applied as-is and the rest of the series left for later, which
should be a safer approach (and would be sufficient to resolve the bug
at the expense of some redundant bytes on disk[1]).

I think Stolee is probably the most familiar with this topic, but he is
off currently and I'm not sure whether or not he'll be back with enough
time to get this merged before 2.35.

On the other hand, the bug is pretty difficult to trigger, is affecting
a very new feature, shouldn't ever cause permanent damage, and can be
recovered from fairly easily (by dropping existing bitmaps). So perhaps
it's OK to let it sit out for another release like this...

Thanks,
Taylor

[1]: More or less storing the contents of the multi-pack-index-$HASH.rev
     file twice: once in the .rev file itself, and again as an optional
     write-only chunk in the MIDX.