I was motivated to start looking closely at midx-write.c due to multiple users reporting Git crashes in their background maintenance, specifically during git multi-pack-index repack calls. I was eventually able to reproduce it in git multi-pack-index expire as well. Patch 1 is the only change we need to fix this bug. It includes a test case that will fail under --stress with SANITIZE=address. It requires creating many packfiles (50 was not enough, but 100 is enough). As far as I can tell, this bug has existed since Git 2.47.0 in October 2024, but I started hearing reports of this from users in July 2025 (and took a while to get a dump/repro). The remaining patches are cleanups based on my careful rereading of midx-write.c. There are some issues about error handling that needed some cleanup as well as a removal of the DISABLE_SIGN_COMPARE_WARNINGS macro. Updates in V3 ============= * Use test_grep over grep. * Translate an error message. * Clarify a commit message. Updates in V2 ============= * A stale comment to an unsubmitted version of the test is removed. * More cases needing open_pack_index() are patched. * Typos fixed. * A new patch assumes error and sets result to zero only on the few successful paths. Thanks, -Stolee Derrick Stolee (6): midx-write: only load initialized packs midx-write: put failing response value back midx-write: use cleanup when incremental midx fails midx-write: use uint32_t for preferred_pack_idx midx-write: reenable signed comparison errors midx-write: simplify error cases midx-write.c | 134 +++++++++++++++++------------------- t/t5319-multi-pack-index.sh | 22 +++++- 2 files changed, 86 insertions(+), 70 deletions(-) base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1965%2Fderrickstolee%2Fmidx-write-cleanup-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1965/derrickstolee/midx-write-cleanup-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/1965 Range-diff vs v2: 1: e02a444315 = 1: e02a444315 midx-write: only load initialized packs 2: a1dd3ed874 ! 2: 1e5f43a417 midx-write: put failing response value back @@ t/t5319-multi-pack-index.sh: test_expect_success 'load reverse index when missin + git cat-file --batch-check="%(objectsize:disk)" <tip && + + test_must_fail git multi-pack-index write 2>err && -+ grep "could not load pack" err ++ test_grep "could not load pack" err ) ' 3: c4f75cca09 = 3: 414ae51024 midx-write: use cleanup when incremental midx fails 4: 2290e27ded ! 4: b113b3f012 midx-write: use uint32_t for preferred_pack_idx @@ Commit message For now, replace the use of -1 with a 'NO_PREFERRED_PACK' macro and an equality check. The macro stores the max value of a uint32_t, so we cannot store a preferred pack that appears last in a list of 2^32 total - packs, but that's expected to be unreasonable already. This improves the - range from 2^31 already. + packs, but that's expected to be unreasonable already. Furthermore, with + this change we end up extending the range from 2^31 possible packs to + 2^32-1. There are some careful things to worry about with initializing the preferred pack in the struct and using that value when searching for a 5: 35302f5228 ! 5: 7c68f2535c midx-write: reenable signed comparison errors @@ midx-write.c: static int write_midx_internal(struct repository *r, const char *o */ + if (ctx.num_multi_pack_indexes_before == UINT32_MAX) -+ die("too many multi-pack-indexes"); ++ die(_("too many multi-pack-indexes")); + CALLOC_ARRAY(keep_hashes, ctx.num_multi_pack_indexes_before + 1); 6: 7be25cf534 = 6: 224be4ee5c midx-write: simplify error cases -- gitgitgadget
