[PATCH v3] midx-write: skip writes with no object entries
- From
- Pia Park <pia@pierre.co>
- Date
- Oct 29, 2026, 08:26 UTC
- Message-ID
- <97eca05600afc5828e3f27c6fcfd3f7872ca326b.1793260625.git.pia@pierre.co>
- In-Reply-To
- <eef33827000cf106544174ed000129c2989af1cd.1788851232.git.pia@pierre.co>
A MIDX write can find packs without finding any objects, either because the packs are empty or because an incremental write finds only objects already indexed by an earlier layer. The existing early exit checks only the number of packs, so these cases publish a zero-object MIDX without a reverse index.
Empty MIDXs were already published by 662148c435 (midx: write object offsets, 2018-07-12); the failure mode appeared in 27afc272c4 (midx: implement writing incremental MIDX bitmaps, 2025-03-20), when incremental bitmap writes began loading reverse indexes from prior layers.
Reproducible on master as of b8242b093d (The 23rd batch, 2026-09-07), starting with a non-incremental write:
git init --bare --object-format=sha1 empty.git &&
(
cd empty.git &&
git pack-objects objects/pack/pack </dev/null &&
git multi-pack-index write &&
git multi-pack-index write --incremental --bitmap
)The first write succeeds but publishes an empty MIDX, which also fails verification with "the midx contains no oid". The second write exits with status 255, reporting "could not load reverse index for MIDX". Starting with an incremental write has the same problem.
When the computed entry list is empty, return success through the existing cleanup path instead of publishing an empty MIDX. Apply this to both non-incremental and incremental writes. The check follows pack/drop validation and precedes lock acquisition and temporary file creation, leaving existing MIDX files untouched. Preserve the existing error when there are no pack files to index.
Empty-object writes already return success, including when --bitmap emits a warning. Preserve that exit status, but omit the warning and empty MIDX.
Test empty packs in a bare repository, empty incremental layers, and packs containing only objects indexed by an earlier layer. Check that writes succeed without creating a MIDX or changing an existing chain. Update the existing bitmap test to expect no MIDX.
Signed-off-by: Pia Park <pia@pierre.co> --- The implementation is unchanged from v2. I held off on the preparatory patch to return success when no packs are present because it raises the question of what to do with an existing MIDX, discussed below.
Changes since v2:
* Simplify the tests as Taylor suggested: drop directory listings, output checks, and subsequent writes already covered elsewhere. * Rename the initial-layer test to make its empty pack explicit, and distinguish the intermediate-layer tests from it. * Add cleanup for the bare repository created by the new t5319 test. * Update the commit message's description of the tests.
The initial-layer test does contain a pack: "git pack-objects" with empty input writes a pack containing zero objects.
There is also a related question about existing MIDX files. With a non-incremental --stdin-packs write selecting an empty pack, this patch returns success and preserves the old MIDX. If its packs were deleted, verification still reports those missing packs. I reproduced this on the patched build. An empty --stdin-packs list also preserves the old MIDX, but continues to report the existing no-pack-files error.
If we make the no-packs case return success too, should an empty non-incremental replacement remove the existing MIDX and its associated files? That cleanup question also applies to the empty-pack case above. Incremental writes would still need to preserve their existing chain. I've left both the no-packs status change and removal of existing MIDX files out of this version; would you prefer to address them separately?
All 885 tests across the eight MIDX, bitmap, and repack scripts pass, along with test-lint and whitespace checks.
midx-write.c | 5 +-- t/t5319-multi-pack-index.sh | 15 +++++++ t/t5326-multi-pack-bitmaps.sh | 9 ++--- t/t5334-incremental-multi-pack-index.sh | 52 +++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/midx-write.c b/midx-write.c index 8537102254..3038bbfad2 100644 --- a/midx-write.c +++ b/midx-write.c @@ -1617,9 +1617,8 @@ static int write_midx_internal(struct write_midx_opts *opts) } if (!ctx.entries_nr) { - if (opts->flags & MIDX_WRITE_BITMAP) - warning(_("refusing to write multi-pack .bitmap without any objects")); - opts->flags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP); + result = 0; + goto cleanup; } if (ctx.incremental) { diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 68143cb5b7..3d65fafe62 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -54,6 +54,21 @@ test_expect_success "don't write midx with no packs" ' test_path_is_missing pack/multi-pack-index ' +test_expect_success 'skip non-incremental MIDX with no objects' ' + test_when_finished "rm -rf empty.git" && + git init --bare empty.git && + ( + cd empty.git && + git pack-objects objects/pack/pack </dev/null && + + for opt in "" --bitmap + do + git multi-pack-index write $opt && + test_path_is_missing objects/pack/multi-pack-index || return 1 + done + ) +' + test_expect_success SHA1 'warn if a midx contains no oid' ' cp "$TEST_DIRECTORY"/t5319/no-objects.midx $objdir/pack/multi-pack-index && test_must_fail git multi-pack-index verify && diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 86beab1dae..854b59d641 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -305,7 +305,7 @@ test_midx_bitmap_cases () { ) ' - test_expect_success 'no .bitmap is written without any objects' ' + test_expect_success 'no MIDX or .bitmap is written without any objects' ' rm -fr repo && git init repo && test_when_finished "rm -fr repo" && @@ -319,12 +319,9 @@ test_midx_bitmap_cases () { EOF git multi-pack-index write --bitmap --stdin-packs \ - <packs 2>err && + <packs && - test_grep "bitmap without any objects" err && - - test_path_is_file $midx && - test_path_is_missing $midx-$(midx_checksum $objdir).bitmap + test_path_is_missing $midx ) ' diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh index f0b82b5f65..25b7be92ff 100755 --- a/t/t5334-incremental-multi-pack-index.sh +++ b/t/t5334-incremental-multi-pack-index.sh @@ -195,4 +195,56 @@ test_expect_success 'non-incremental write with existing incremental chain' ' ) ' +test_expect_success 'skip initial MIDX layer with empty pack' ' + git init empty && + ( + cd empty && + git config maintenance.auto false && + git pack-objects $packdir/pack </dev/null && + + for bitmap in --bitmap --no-bitmap + do + git multi-pack-index write --incremental "$bitmap" && + test_dir_is_empty "$midxdir" || return 1 + done + ) +' + +test_expect_success 'skip intermediate MIDX layer with empty pack' ' + git init empty-pack && + ( + cd empty-pack && + git config maintenance.auto false && + write_midx_layer && + + git pack-objects $packdir/pack </dev/null && + cp "$midx_chain" chain.expect && + + for bitmap in --bitmap --no-bitmap + do + git multi-pack-index write --incremental "$bitmap" && + test_cmp chain.expect "$midx_chain" || return 1 + done + ) +' + +test_expect_success 'skip intermediate MIDX layer with duplicate pack' ' + git init duplicate-pack && + ( + cd duplicate-pack && + git config maintenance.auto false && + write_midx_layer && + + git rev-parse HEAD^{tree} >in && + git pack-objects $packdir/pack <in && + cp "$midx_chain" chain.expect && + + for bitmap in --bitmap --no-bitmap + do + git multi-pack-index write --incremental "$bitmap" && + test_cmp chain.expect "$midx_chain" || return 1 + done + ) +' + test_done base-commit: b8242b093d9e941a34460d715e3ce616a34ac3fe
-- 2.55.0