[v3,0/9] midx: prevent bitmap corruption when permuting pack order

Message

Taylor Blau Jan. 4, 2022, 6:15 p.m. UTC

Here is a small reroll of my series which fixes a serious problem with MIDX
bitmaps by which they can become corrupt when permuting their pack order.

This reroll cleans up some of the tests by removing a couple of places where we
explicitly set GIT_TRACE2_EVENT_NESTING in the environment, which is no longer
needed after 62329d336f (Merge branch 'ds/trace2-regions-in-tests', 2021-12-15).

It also adds one new patch on top, which ensure that we gracefully fall back
when MIDX bitmaps cannot be read in certain cases.

It is prepared on the tip of master (which is dcc0cd074f at the time of
writing).

Taylor Blau (9):
  t5326: demonstrate bitmap corruption after permutation
  midx.c: make changing the preferred pack safe
  pack-revindex.c: instrument loading on-disk reverse index
  t5326: drop unnecessary setup
  t5326: extract `test_rev_exists`
  t5326: move tests to t/lib-bitmap.sh
  t/lib-bitmap.sh: parameterize tests over reverse index source
  midx: read `RIDX` chunk when present
  pack-bitmap.c: gracefully fallback after opening pack/MIDX

 Documentation/technical/multi-pack-index.txt |   1 +
 Documentation/technical/pack-format.txt      |  13 +-
 midx.c                                       |  31 +++-
 midx.h                                       |   1 +
 pack-bitmap.c                                |   4 +
 pack-revindex.c                              |  20 ++
 t/lib-bitmap.sh                              | 185 +++++++++++++++++++
 t/t5310-pack-bitmaps.sh                      |  28 +++
 t/t5326-multi-pack-bitmaps.sh                | 164 +++-------------
 t/t5327-multi-pack-bitmaps-rev.sh            |  23 +++
 t/t7700-repack.sh                            |   4 -
 11 files changed, 322 insertions(+), 152 deletions(-)
 create mode 100755 t/t5327-multi-pack-bitmaps-rev.sh

Range-diff against v2:
 1:  dfbac4bc60 =  1:  babce7d29a t5326: demonstrate bitmap corruption after permutation
 2:  4ea52e66dd !  2:  7d20c13f8b midx.c: make changing the preferred pack safe
    @@ Commit message
     
      ## Documentation/technical/multi-pack-index.txt ##
     @@ Documentation/technical/multi-pack-index.txt: and their offsets into multiple packfiles. It contains:
    -   - An offset within the jth packfile for the object.
    - - If large offsets are required, we use another list of large
    + ** An offset within the jth packfile for the object.
    + * If large offsets are required, we use another list of large
        offsets similar to version 2 pack-indexes.
     +- An optional list of objects in pseudo-pack order (used with MIDX bitmaps).
      
 3:  b630fea149 =  3:  3279e2eb9b pack-revindex.c: instrument loading on-disk reverse index
 4:  f430b6f2e9 =  4:  5818621ea8 t5326: drop unnecessary setup
 5:  73faab9f42 !  5:  33502d6a17 t5326: extract `test_rev_exists`
    @@ t/t5326-multi-pack-bitmaps.sh: midx_pack_source () {
     +	commit="$1"
     +
     +	test_expect_success 'reverse index exists' '
    -+		GIT_TRACE2_EVENT_NESTING=10 \
     +		GIT_TRACE2_EVENT=$(pwd)/event.trace \
     +			git rev-list --test-bitmap "$commit" &&
     +
 6:  bf42b116e1 !  6:  76e23cae0f t5326: move tests to t/lib-bitmap.sh
    @@ t/lib-bitmap.sh: have_delta () {
     +	commit="$1"
     +
     +	test_expect_success 'reverse index exists' '
    -+		GIT_TRACE2_EVENT_NESTING=10 \
     +		GIT_TRACE2_EVENT=$(pwd)/event.trace \
     +			git rev-list --test-bitmap "$commit" &&
     +
    @@ t/t5326-multi-pack-bitmaps.sh: test_description='exercise basic multi-pack bitma
     -	commit="$1"
     -
     -	test_expect_success 'reverse index exists' '
    --		GIT_TRACE2_EVENT_NESTING=10 \
     -		GIT_TRACE2_EVENT=$(pwd)/event.trace \
     -			git rev-list --test-bitmap "$commit" &&
     -
 7:  fa91631024 !  7:  7ce3dc60f9 t/lib-bitmap.sh: parameterize tests over reverse index source
    @@ t/lib-bitmap.sh: midx_pack_source () {
     +	kind="$2"
      
      	test_expect_success 'reverse index exists' '
    - 		GIT_TRACE2_EVENT_NESTING=10 \
      		GIT_TRACE2_EVENT=$(pwd)/event.trace \
      			git rev-list --test-bitmap "$commit" &&
      
 8:  993bfa8dd8 !  8:  55aa69de12 midx: read `RIDX` chunk when present
    @@ t/t5326-multi-pack-bitmaps.sh: test_description='exercise basic multi-pack bitma
      GIT_TEST_MULTI_PACK_INDEX=0
      GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
      
    -+GIT_TEST_MIDX_WRITE_REV=0
    -+GIT_TEST_MIDX_READ_RIDX=1
    -+export GIT_TEST_MIDX_WRITE_REV
    -+export GIT_TEST_MIDX_READ_RIDX
    ++# This test exercise multi-pack bitmap functionality where the object order is
    ++# stored and read from a special chunk within the MIDX, so use the default
    ++# behavior here.
    ++sane_unset GIT_TEST_MIDX_WRITE_REV
    ++sane_unset GIT_TEST_MIDX_READ_RIDX
     +
      midx_bitmap_core
      
 -:  ---------- >  9:  9707d5ea44 pack-bitmap.c: gracefully fallback after opening pack/MIDX