All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] midx-write: skip empty incremental layers
@ 2026-09-08  1:46 Pia Park
  2026-09-08  4:09 ` Taylor Blau
  0 siblings, 1 reply; 4+ messages in thread
From: Pia Park @ 2026-09-08  1:46 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau, Derrick Stolee

An incremental MIDX write can find new packs without finding any new
objects, either because the packs are empty or because their objects
are already indexed by an earlier layer.

The existing early exit checks the number of packs, so these cases
publish a zero-object layer without a reverse index. A subsequent
incremental write with --bitmap fails when loading that reverse index.

Reproducible on master as of
b8242b093d9e941a34460d715e3ce616a34ac3fe (2026-09-07), using:

    git init --bare --object-format=sha1 empty.git &&
    (
        cd empty.git &&
        git config midx.version 2 &&
        git pack-objects objects/pack/pack </dev/null &&
        git multi-pack-index write --incremental --bitmap &&
        git multi-pack-index write --incremental --bitmap
    )

The first write succeeds but publishes the empty layer
3c8853aad425100c5ee2ee22209bb0bb3df9ca37. The second exits with status
255, reporting "could not load reverse index for MIDX".

Return success through the existing cleanup path when
compute_sorted_entries() finds no entries for a non-compacting
incremental write. This prevents publishing an empty layer that causes
subsequent incremental writes with --bitmap to fail with exit status
255. Exit before acquiring a lock or creating a temporary MIDX file,
leaving the existing chain untouched.

Add regression tests for empty initial layers, empty packs, and packs
containing only duplicate objects. Check silent success, preservation
of the existing chain and file list, and subsequent writes that add
new objects.

Signed-off-by: Pia Park <pia@pierre.co>
---
Built with DEVELOPER=1 and ran these scripts before and after:

  t5319-multi-pack-index.sh
  t5326-multi-pack-bitmaps.sh
  t5327-multi-pack-bitmaps-rev.sh
  t5332-multi-pack-reuse.sh
  t5334-incremental-multi-pack-index.sh
  t5335-compact-multi-pack-index.sh
  t7700-repack.sh
  t7705-repack-incremental-midx.sh

All 881 baseline tests and all 884 tests with the patch passed. The
three new regressions fail on the unpatched build. Test lint and
whitespace checks passed.

Both empty and duplicate-only pack reproductions also fail on a fresh,
unmodified build of upstream master at the base commit shown below.

 midx-write.c                            |  5 +++
 t/t5334-incremental-multi-pack-index.sh | 58 +++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/midx-write.c b/midx-write.c
index 8537102254..cdb2ef0474 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1518,6 +1518,11 @@ static int write_midx_internal(struct write_midx_opts *opts)
 
 	compute_sorted_entries(&ctx, start_pack);
 
+	if (ctx.incremental && !ctx.compact && !ctx.entries_nr) {
+		result = 0;
+		goto cleanup;
+	}
+
 	ctx.large_offsets_needed = 0;
 	for (size_t i = 0; i < ctx.entries_nr; i++) {
 		if (ctx.entries[i].offset > 0x7fffffff)
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index f0b82b5f65..4bdfa61d38 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -195,4 +195,62 @@ test_expect_success 'non-incremental write with existing incremental chain' '
 	)
 '
 
+test_expect_success 'skip initial MIDX layer with no objects' '
+	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" >out 2>err &&
+			test_must_be_empty out &&
+			test_must_be_empty err &&
+			test_dir_is_empty "$midxdir" || return 1
+		done &&
+
+		write_midx_layer &&
+		test_line_count = 1 "$midx_chain" &&
+		git multi-pack-index verify
+	)
+'
+
+for pack in empty duplicate
+do
+	test_expect_success "skip MIDX layer with $pack pack" '
+		git init "$pack-pack" &&
+		(
+			cd "$pack-pack" &&
+			git config maintenance.auto false &&
+			write_midx_layer &&
+
+			if test "$pack" = duplicate
+			then
+				git rev-parse HEAD^{tree} >in
+			else
+				>in
+			fi &&
+			git pack-objects $packdir/pack <in &&
+			cp "$midx_chain" chain.expect &&
+			ls "$packdir" "$midxdir" >files.expect &&
+
+			for bitmap in --bitmap --no-bitmap
+			do
+				git multi-pack-index write --incremental "$bitmap" >out 2>err &&
+				test_must_be_empty out &&
+				test_must_be_empty err &&
+				test_cmp chain.expect "$midx_chain" &&
+				ls "$packdir" "$midxdir" >files.actual &&
+				test_cmp files.expect files.actual || return 1
+			done &&
+
+			write_midx_layer &&
+			test_line_count = 2 "$midx_chain" &&
+			git multi-pack-index verify &&
+			git rev-list --test-bitmap 2.2
+		)
+	'
+done
+
 test_done

base-commit: b8242b093d9e941a34460d715e3ce616a34ac3fe
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-08 15:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08  1:46 [PATCH] midx-write: skip empty incremental layers Pia Park
2026-09-08  4:09 ` Taylor Blau
2026-09-08  7:10   ` [PATCH v2] midx-write: skip writes with no object entries Pia Park
2026-09-08 15:06     ` Taylor Blau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.