[PATCH v2 8/8] repack: include required packs in incremental MIDX writes

[Taylor Blau <ttaylorr@openai.com>]

The append plan introduced in 06733a50eee (repack: allow
`--write-midx=incremental` without `--geometric`, 2026-05-19) adds only
newly written packs to the existing MIDX chain. The bitmap writer can
use objects from the new layer and all retained base layers, but the
plan omits preexisting packs outside the chain. Bitmap generation fails
if a selected commit reaches an object absent from the resulting chain.

The geometric plan from 1da62fb5c86 (repack: implement incremental MIDX
repacking, 2026-05-19) can omit kept and cruft packs, since neither
necessarily participates in the geometric repack. Such packs can also be
lost when replacing a tip layer that contains them. Neither plan
consults `midx_included_packs()`, so the rules for retaining cruft in
ordinary MIDX writes do not protect incremental writes.

Use that selection logic to add missing packs to each plan's write step.
Skip packs in retained base layers, but include required packs from a
replaced tip. Count added objects when choosing which layers to compact,
without changing the preferred pack.

Write and verify bitmaps in the existing append test: its existing
checks do not detect the omitted pack containing the first commit. Cover
the no-new-pack case separately with a reachable blob in a cruft pack.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 Documentation/git-repack.adoc      |  5 +-
 repack-midx.c                      | 83 ++++++++++++++++++++++++------
 t/t7705-repack-incremental-midx.sh | 63 ++++++++++++++++++-----
 3 files changed, 122 insertions(+), 29 deletions(-)

diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc
index a1f9e64f668..ee59b76580a 100644
--- a/Documentation/git-repack.adoc
+++ b/Documentation/git-repack.adoc
@@ -303,8 +303,9 @@ linkgit:git-multi-pack-index[1]).
 		flat MIDX.
 +
 Without `--geometric`, a new MIDX layer is appended to the existing
-chain (or a new chain is started) containing whatever packs were written
-by the repack. Existing layers are preserved as-is.
+chain (or a new chain is started) containing newly written packs and any
+other required packs not already in the chain. Existing layers are
+preserved as-is.
 +
 When combined with `--geometric`, the incremental mode maintains a chain
 of MIDX layers that is compacted over time using a geometric merging
diff --git a/repack-midx.c b/repack-midx.c
index 06eadb9df82..58776c44691 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -7,6 +7,7 @@
 #include "odb.h"
 #include "oidset.h"
 #include "pack-bitmap.h"
+#include "packfile.h"
 #include "path.h"
 #include "refs.h"
 #include "run-command.h"
@@ -73,7 +74,8 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)
 
 static int midx_has_unknown_packs(struct string_list *include,
 				  struct pack_geometry *geometry,
-				  struct existing_packs *existing)
+				  struct existing_packs *existing,
+				  struct multi_pack_index *base)
 {
 	struct string_list_item *item;
 
@@ -91,6 +93,8 @@ static int midx_has_unknown_packs(struct string_list *include,
 		 *    MIDX. Note this function is called before the include
 		 *    list is populated with any cruft pack(s).
 		 *
+		 *  - In a MIDX layer retained as part of the new chain's base.
+		 *
 		 *  - Below the geometric split line (if using pack geometry),
 		 *    indicating that the pack won't be included in the new
 		 *    MIDX, but its contents were rolled up as part of the
@@ -99,7 +103,8 @@ static int midx_has_unknown_packs(struct string_list *include,
 		 *  - In the existing non-kept packs list (if not using pack
 		 *    geometry), and marked as non-deleted.
 		 */
-		if (string_list_has_string(include, pack_name)) {
+		if (string_list_has_string(include, pack_name) ||
+		    midx_contains_pack(base, pack_name)) {
 			continue;
 		} else if (geometry) {
 			struct strbuf buf = STRBUF_INIT;
@@ -141,7 +146,8 @@ static int midx_has_unknown_packs(struct string_list *include,
 }
 
 static void midx_included_packs(struct string_list *include,
-				struct repack_write_midx_opts *opts)
+				struct repack_write_midx_opts *opts,
+				struct multi_pack_index *base)
 {
 	struct existing_packs *existing = opts->existing;
 	struct pack_geometry *geometry = opts->geometry;
@@ -198,7 +204,7 @@ static void midx_included_packs(struct string_list *include,
 
 	if (opts->midx_must_contain_cruft ||
 	    (!geometry->split_factor && existing->kept_packs.nr) ||
-	    midx_has_unknown_packs(include, geometry, existing)) {
+	    midx_has_unknown_packs(include, geometry, existing, base)) {
 		/*
 		 * If there are one or more unknown pack(s) present (see
 		 * midx_has_unknown_packs() for what makes a pack
@@ -336,7 +342,7 @@ static int write_midx_included_packs(struct repack_write_midx_opts *opts)
 	struct packed_git *preferred = pack_geometry_preferred_pack(opts->geometry);
 	int ret = 0;
 
-	midx_included_packs(&include, opts);
+	midx_included_packs(&include, opts, NULL);
 	if (!include.nr)
 		goto done;
 
@@ -547,9 +553,50 @@ static void midx_compaction_step_release(struct midx_compaction_step *step)
 	free(step->csum);
 }
 
+static int midx_compaction_step_include_packs(struct midx_compaction_step *step,
+					      struct repack_write_midx_opts *opts,
+					      struct multi_pack_index *base)
+{
+	struct odb_source_files *files = odb_source_files_downcast(opts->existing->source);
+	struct string_list include = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
+	struct strbuf path = STRBUF_INIT;
+	int ret = 0;
+
+	midx_included_packs(&include, opts, base);
+	string_list_sort(&step->u.write);
+
+	for_each_string_list_item(item, &include) {
+		struct packed_git *p;
+
+		if (string_list_has_string(&step->u.write, item->string) ||
+		    midx_contains_pack(base, item->string))
+			continue;
+
+		strbuf_reset(&path);
+		strbuf_addf(&path, "%s/%s", opts->packdir, item->string);
+		p = packfile_store_load_pack(files->packed, path.buf, 1);
+		if (!p || open_pack_index(p)) {
+			ret = error(_("cannot open index for %s"), path.buf);
+			goto out;
+		}
+		if (unsigned_add_overflows(step->objects_nr, p->num_objects)) {
+			ret = error(_("too many objects in MIDX compaction step"));
+			goto out;
+		}
+		step->objects_nr += p->num_objects;
+		string_list_insert(&step->u.write, item->string);
+	}
+
+out:
+	strbuf_release(&path);
+	string_list_clear(&include, 0);
+	return ret;
+}
+
 /*
- * Build an append-only MIDX plan: a single WRITE step for the freshly
- * written packs, plus COPY steps for every existing layer.  No
+ * Build an append-only MIDX plan: a single WRITE step for packs not
+ * already in the chain, plus COPY steps for every existing layer. No
  * compaction or merging is performed.
  */
 static void repack_make_midx_append_plan(struct repack_write_midx_opts *opts,
@@ -557,17 +604,20 @@ static void repack_make_midx_append_plan(struct repack_write_midx_opts *opts,
 					 size_t *steps_nr_p)
 {
 	struct odb_source_files *files = odb_source_files_downcast(opts->existing->source);
+	struct string_list include = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
 	struct multi_pack_index *m;
 	struct midx_compaction_step *steps = NULL;
 	struct midx_compaction_step *step = NULL;
-	struct strbuf buf = STRBUF_INIT;
 	size_t steps_nr = 0, steps_alloc = 0;
-	uint32_t i;
 
 	odb_reprepare(opts->existing->repo->objects);
 	m = get_multi_pack_index(files->packed);
 
-	for (i = 0; i < opts->names->nr; i++) {
+	midx_included_packs(&include, opts, m);
+	for_each_string_list_item(item, &include) {
+		if (midx_contains_pack(m, item->string))
+			continue;
 		if (!step) {
 			ALLOC_GROW(steps, st_add(steps_nr, 1), steps_alloc);
 			step = &steps[steps_nr++];
@@ -575,12 +625,9 @@ static void repack_make_midx_append_plan(struct repack_write_midx_opts *opts,
 			step->type = MIDX_COMPACTION_STEP_WRITE;
 			string_list_init_dup(&step->u.write);
 		}
-		strbuf_reset(&buf);
-		strbuf_addf(&buf, "pack-%s.idx",
-			    opts->names->items[i].string);
-		string_list_append(&step->u.write, buf.buf);
+		string_list_append(&step->u.write, item->string);
 	}
-	strbuf_release(&buf);
+	string_list_clear(&include, 0);
 
 	for (; m; m = m->base_midx) {
 		ALLOC_GROW(steps, st_add(steps_nr, 1), steps_alloc);
@@ -729,6 +776,12 @@ static int repack_make_midx_compaction_plan(struct repack_write_midx_opts *opts,
 	if (opts->geometry->midx_tip_rewritten)
 		m = m->base_midx;
 
+	if (midx_compaction_step_include_packs(&step, opts, m) < 0) {
+		midx_compaction_step_release(&step);
+		ret = -1;
+		goto out;
+	}
+
 	trace2_data_string("repack", opts->existing->repo, "midx:rewrote-tip",
 			   opts->geometry->midx_tip_rewritten ? "true" : "false");
 
diff --git a/t/t7705-repack-incremental-midx.sh b/t/t7705-repack-incremental-midx.sh
index 25a8c40e8ee..4760c920a50 100755
--- a/t/t7705-repack-incremental-midx.sh
+++ b/t/t7705-repack-incremental-midx.sh
@@ -74,7 +74,7 @@ test_expect_success '--write-midx=incremental without --geometric' '
 		git repack -d &&
 
 		test_commit second &&
-		git repack --write-midx=incremental &&
+		git repack --write-midx=incremental --write-bitmap-index &&
 
 		git multi-pack-index verify &&
 		test_line_count = 1 $midx_chain &&
@@ -83,7 +83,7 @@ test_expect_success '--write-midx=incremental without --geometric' '
 		# A second repack appends a new layer without
 		# disturbing the existing one.
 		test_commit third &&
-		git repack --write-midx=incremental &&
+		git repack --write-midx=incremental --write-bitmap-index &&
 
 		git multi-pack-index verify &&
 		test_line_count = 2 $midx_chain &&
@@ -91,10 +91,50 @@ test_expect_success '--write-midx=incremental without --geometric' '
 		head -n 1 $midx_chain >actual &&
 		test_cmp expect actual &&
 
+		git rev-list --test-bitmap HEAD &&
 		git fsck
 	)
 '
 
+test_expect_success 'incremental MIDX includes cruft without a new pack' '
+	git init incremental-cruft &&
+	(
+		cd incremental-cruft &&
+		git config repack.midxMustContainCruft false &&
+
+		test_commit base &&
+		echo cruft | git hash-object -w --stdin &&
+		git repack --cruft -d &&
+		test_commit cruft &&
+		git repack -d &&
+
+		# All objects are packed, but the new MIDX still needs cruft.
+		git repack --write-midx=incremental --write-bitmap-index &&
+		git rev-list --test-bitmap HEAD
+	)
+'
+
+test_expect_success 'geometric incremental MIDX retains cruft when replacing its tip' '
+	git init geometric-incremental-cruft &&
+	(
+		cd geometric-incremental-cruft &&
+		git config repack.midxNewLayerThreshold 1 &&
+
+		test_commit base &&
+		echo cruft | git hash-object -w --stdin &&
+		git repack --cruft -d &&
+		git multi-pack-index write --incremental --bitmap &&
+		test_commit cruft &&
+
+		# Pack the new commit and tree, leaving the blob in cruft.
+		git repack -d &&
+		git repack --geometric=2 --write-midx=incremental \
+			--write-bitmap-index &&
+		test_line_count = 1 $midx_chain &&
+		git rev-list --test-bitmap HEAD
+	)
+'
+
 test_expect_success 'below layer threshold, tip packs excluded' '
 	git init below-layer-threshold-tip-packs-excluded &&
 	(
@@ -338,7 +378,7 @@ test_expect_success 'geometric rollup with surviving tip packs' '
 	)
 '
 
-test_expect_success 'kept packs are excluded from repack' '
+test_expect_success 'kept packs are excluded from repack but included in MIDX' '
 	git init kept-packs-excluded-from-repack &&
 	(
 		cd kept-packs-excluded-from-repack &&
@@ -353,21 +393,20 @@ test_expect_success 'kept packs are excluded from repack' '
 			test_commit "$i" && git repack -d || return 1
 		done &&
 
-		keep=$(ls $packdir/pack-*.idx | head -n 1) &&
-		touch "${keep%.idx}.keep" &&
+		keep=$(test-tool find-pack A) &&
+		touch "${keep%.pack}.keep" &&
 
-		# The kept pack is excluded as a repacking candidate
-		# entirely, so no rollup occurs as there is only one
-		# non-kept pack. A new MIDX layer is written containing
-		# that pack.
-		git repack --geometric=2 -d --write-midx=incremental &&
+		# Neither pack is repacked, but both are needed for the
+		# bitmap of B, which reaches objects in the kept pack.
+		git repack --geometric=2 -d --write-midx=incremental \
+			--write-bitmap-index &&
 
 		test-tool read-midx $objdir >actual &&
 		grep "^pack-.*\.idx$" actual >actual.packs &&
-		test_line_count = 1 actual.packs &&
-		test_grep ! "$keep" actual.packs &&
+		test_line_count = 2 actual.packs &&
 
 		git multi-pack-index verify &&
+		git rev-list --test-bitmap HEAD &&
 
 		# All objects (from both kept and non-kept packs)
 		# must still be accessible.
-- 
2.56.0.8.ga42f775cbe2