From: Taylor Blau Date: Mon, 22 Aug 2022 19:50:24 GMT Subject: [PATCH v2 0/7] midx: permit changing the preferred pack when reusing the MIDX Message-ID: In-Reply-To: Here is a small reroll of my series that resolves a bug that was reported[1] by Johannes, and investigated by him, Abhradeep, and Stolee in that same sub-thread. As before: the crux of the issue is that a MIDX bitmap can enter a corrupt state when changing the preferred pack from its value in an existing MIDX in certain circumstances as described in the first and final patches. This version incorporates some cosmetic changes suggested by Stolee, and adds a new patch on top which avoids adding objects from the MIDX that were represented by the (new) preferred pack, since we know we'll end up discarding those objects anyways. For convenience, a range-diff against v1 is included below. Thanks again for your review! [1]: https://lore.kernel.org/git/p3r70610-8n52-s8q0-n641-onp4ps01330n@tzk.qr/ Taylor Blau (7): t5326: demonstrate potential bitmap corruption t/lib-bitmap.sh: avoid silencing stderr midx.c: extract `struct midx_fanout` midx.c: extract `midx_fanout_add_midx_fanout()` midx.c: extract `midx_fanout_add_pack_fanout()` midx.c: include preferred pack correctly with existing MIDX midx.c: avoid adding preferred objects twice midx.c | 139 +++++++++++++++++++++++----------- t/lib-bitmap.sh | 2 +- t/t5326-multi-pack-bitmaps.sh | 44 +++++++++++ 3 files changed, 139 insertions(+), 46 deletions(-) Range-diff against v1: 1: 3e30ab1a19 ! 1: 6b38bfcd2c t5326: demonstrate potential bitmap corruption @@ t/t5326-multi-pack-bitmaps.sh: test_expect_success 'graceful fallback when missi ' +test_expect_success 'preferred pack change with existing MIDX bitmap' ' -+ rm -fr repo && -+ git init repo && -+ test_when_finished "rm -fr repo" && ++ git init preferred-pack-with-existing && + ( -+ cd repo && ++ cd preferred-pack-with-existing && + + test_commit base && + test_commit other && @@ t/t5326-multi-pack-bitmaps.sh: test_expect_success 'graceful fallback when missi + p2="$(git pack-objects "$objdir/pack/pack" \ + --delta-base-offset @@ midx.c: static struct pack_midx_entry *get_sorted_entries(struct multi_pack_inde ## t/t5326-multi-pack-bitmaps.sh ## @@ t/t5326-multi-pack-bitmaps.sh: test_expect_success 'preferred pack change with existing MIDX bitmap' ' + git pack-objects --all --unpacked $objdir/pack/pack0 && + + # Generate a new MIDX which changes the preferred pack +- # to a pack contained in the existing MIDX, such that +- # not all objects from p2 that appear in the MIDX had +- # their copy selected from p2. ++ # to a pack contained in the existing MIDX. + git multi-pack-index write --bitmap \ + --preferred-pack="pack-$p2.pack" && test_path_is_file $midx && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && +- # When the above circumstances are met, an existing bug +- # in the MIDX machinery will cause the reverse index to +- # be read incorrectly, resulting in failed clones (among +- # other things). - test_must_fail git clone --no-local . clone2 ++ # When the above circumstances are met, the preferred ++ # pack should change appropriately and clones should ++ # (still) succeed. + git clone --no-local . clone2 ) ' -: ---------- > 7: 887ab9485f midx.c: avoid adding preferred objects twice -- 2.37.0.1.g1379af2e9d