On Wed, Jul 17, 2024 at 05:12:47PM -0400, Taylor Blau wrote: > Two years ago, commit ff1e653c8e2 (midx: respect > 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP', 2021-08-31) introduced a new > environment variable which caused the test suite to write MIDX bitmaps > after any 'git repack' invocation. > > At the time, this was done to help flush out any bugs with MIDX bitmaps > that weren't explicitly covered in the t5326-multi-pack-bitmap.sh > script. > > Two years later, that flag has served us well and is no longer providing > meaningful coverage, as the script in t5326 has matured substantially > and covers many more interesting cases than it did back when ff1e653c8e2 > was originally written. I do think it could be providing some value still, just because other scripts may create unusual setups that will exercise the code in different ways. That said... > Remove the 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' environment variable > as it is no longer serving a useful purpose. More importantly, removing > this variable clears the way for us to introduce a new one to help > similarly flush out bugs related to incremental MIDX chains. > > Because these incremental MIDX chains are (for now) incompatible with > MIDX bitmaps, we cannot have both. ...if it is one or the other, I think it is better to test the new code. And I do think that midx bitmap code is less likely to be exercised in interesting ways by random parts of the test suite (versus something like GIT_TEST_DEFAULT_HASH, whose effects are pervasive). So I think this is a good tradeoff. > builtin/repack.c | 12 ++---------- > ci/run-build-and-tests.sh | 1 - > midx.h | 2 -- > t/README | 4 ---- > t/t0410-partial-clone.sh | 2 -- > t/t5310-pack-bitmaps.sh | 4 ---- > t/t5319-multi-pack-index.sh | 3 +-- > t/t5326-multi-pack-bitmaps.sh | 3 +-- > t/t5327-multi-pack-bitmaps-rev.sh | 5 ++--- > t/t7700-repack.sh | 21 +++++++-------------- > 10 files changed, 13 insertions(+), 44 deletions(-) Patch looks good. -Peff
