Thread (7 messages) 7 messages, 3 authors, 19h ago

[PATCH v3] midx-write: skip writes with no object entries

flat view
HOTtoday

From: Pia Park <hidden>
Date: 2026-10-29 08:27:08
Subsystem: the rest · Maintainer: Linus Torvalds

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 <redacted>
---
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help