repack: demote .baddeltas packs into geometric rollup

A pack-aggregate run rolls many small packs up into one aggregator
output and writes a sibling `.baddeltas` marker, since the new pack
reuses raw delta-chains from its inputs without any cross-input delta
search.  That marker tells future `pack-objects` runs over the pack to
treat its existing deltas as stale and recompute new ones.

The aggregator deliberately does not install a `.keep` marker on its
output: we want the next opportunity to re-delta its contents to roll up
the pack like any other.  But a `git repack --geometric=N` pass walks
the packs sorted ascending by object count and only rolls up packs below
the geometric split; anything already at the heavy end of the
progression sits above the split and is kept as-is.  Aggregated packs
could potentially be large enough to land there, which may leave a
`.baddeltas` pack above the split for many subsequent geometric passes
resulting in the pack not getting repacked and keeping its stale deltas.

Teach `pack_geometry_split()` to do a post-pass demotion of any
`.baddeltas` packs from the kept region into the rollup region.  The
demotion preserves the ascending sort order of the kept region so that
`pack_geometry_preferred_pack()` continues to return the largest local
kept pack.  Demotion is unconditional: a `.baddeltas` marker is an
explicit "these deltas need redoing" tag from a previous repack step,
and the next geometric pass is the cheapest place to honor it.

`.keep`-marked packs are still excluded from the rollup by the existing
`pack_kept_objects` / `remove_redundant_packs` paths, so a user who
really wants to pin a `.baddeltas` pack in place has that escape hatch.

Add two tests to t5336: one that confirms a `.baddeltas` pack above the
natural split is rolled up and its marker disappears from the resulting
pack, and a control test that confirms a non-`.baddeltas` pack above the
split is still preserved.

Assisted-by: Claude Opus 4.7
Signed-off-by: Elijah Newren <newren@gmail.com>

Author: newren

repack-geometry.c

@@ -187,13 +187,66 @@ static uint32_t compute_pack_geometry_split(struct packed_git **pack, size_t pac
 	return split;
 }
 
+/*
+ * Move any packs marked with a `.baddeltas` sidecar from the kept
+ * region of `pack[]` into the rollup region.  A `.baddeltas` marker
+ * declares that the pack's existing deltas are stale (e.g. because
+ * the pack was assembled by `git pack-aggregate`, which concatenates
+ * input packs without performing a fresh delta search).  We do not
+ * want such packs to sit on the geometric ladder indefinitely:
+ * rolling them up forces the next `pack-objects` invocation to
+ * recompute deltas across their objects, while leaving any
+ * `.keep`-protected pack alone (those are filtered out by
+ * `pack_geometry_init()`).
+ *
+ * Demotion preserves the ascending sort order of the kept region so
+ * that `pack_geometry_preferred_pack()` continues to return the
+ * largest local kept pack.
+ */
+static void demote_bad_delta_packs(struct pack_geometry *geometry)
+{
+	struct packed_git **demoted, **retained;
+	uint32_t d_nr = 0, r_nr = 0, kept_nr;
+
+	if (geometry->split == geometry->pack_nr)
+		return;
+
+	kept_nr = geometry->pack_nr - geometry->split;
+	for (uint32_t k = geometry->split; k < geometry->pack_nr; k++) {
+		if (geometry->pack[k]->has_bad_deltas)
+			d_nr++;
+	}
+	if (!d_nr)
+		return;
+
+	ALLOC_ARRAY(demoted, d_nr);
+	ALLOC_ARRAY(retained, kept_nr - d_nr);
+	d_nr = 0;
+	for (uint32_t k = geometry->split; k < geometry->pack_nr; k++) {
+		if (geometry->pack[k]->has_bad_deltas)
+			demoted[d_nr++] = geometry->pack[k];
+		else
+			retained[r_nr++] = geometry->pack[k];
+	}
+
+	memcpy(&geometry->pack[geometry->split], demoted,
+	       d_nr * sizeof(*geometry->pack));
+	memcpy(&geometry->pack[geometry->split + d_nr], retained,
+	       r_nr * sizeof(*geometry->pack));
+	geometry->split += d_nr;
+
+	free(demoted);
+	free(retained);
+}
+
 void pack_geometry_split(struct pack_geometry *geometry)
 {
 	geometry->split = compute_pack_geometry_split(geometry->pack, geometry->pack_nr,
 						      geometry->split_factor);
 	geometry->promisor_split = compute_pack_geometry_split(geometry->promisor_pack,
 							       geometry->promisor_pack_nr,
 							       geometry->split_factor);
+	demote_bad_delta_packs(geometry);
 	for (uint32_t i = 0; i < geometry->split; i++) {
 		struct packed_git *p = geometry->pack[i];
 		/*

t/t5336-pack-aggregate.sh

@@ -406,12 +406,17 @@ test_expect_success 'startup cleans up stale .keep markers from dead pids' '
 	)
 '
 
-test_expect_success 'startup leaves live-pid .keep markers alone' '
+test_expect_success !MINGW 'startup leaves live-pid .keep markers alone' '
 	test_when_finished "rm -fr work" &&
 	cp -R repo work &&
 	# Use the test-runner shell pid, which is reliably alive
 	# for the duration of this test.  PID 1 (init) would also
 	# work since our code treats EPERM as "alive".
+	#
+	# !MINGW: on Windows, bash $$ is a virtualized MSYS pid that
+	# does not correspond to a process kill(pid, 0) can see, so
+	# our liveness check would treat it as dead and the marker
+	# would be removed.  Skip this test there.
 	live_pid=$$ &&
 	(
 		cd work &&
@@ -430,4 +435,74 @@ test_expect_success 'startup leaves live-pid .keep markers alone' '
 	)
 '
 
+# ---- geometric .baddeltas demotion ----
+
+# Helper for the geometric tests: pack a single unique blob into its
+# own packfile.  The `tag` parameter must differ between callers so
+# that the blob contents do not collide with one another (otherwise
+# `git pack-objects --stdin-packs` would see the "rollup" objects
+# already present in the kept pack and emit nothing).
+make_unique_pack () {
+	tag=$1 &&
+	blob=$(echo "unique-${tag}-$$" | git hash-object -w --stdin) &&
+	echo "$blob" |
+	git pack-objects --window=0 .git/objects/pack/pack >/dev/null &&
+	git prune-packed
+}
+
+test_expect_success 'geometric repack demotes .baddeltas packs into rollup' '
+	test_when_finished "rm -fr work" &&
+	cp -R repo work &&
+	(
+		cd work &&
+		# Build several small packs and consolidate them into
+		# one larger pack that would normally sit above the
+		# geometric split.
+		build_n_packs 8 >/dev/null &&
+		git repack -d --geometric=2 &&
+		test 1 -eq "$(count_packs)" &&
+		big=$(ls .git/objects/pack/pack-*.pack) &&
+		# Drop a .baddeltas marker on it (simulating an
+		# aggregator output).
+		>"${big%.pack}.baddeltas" &&
+		test 1 -eq "$(count_baddeltas)" &&
+		# Add small packs that are individually far smaller
+		# than the big one; ordinarily the big pack would be
+		# kept above the geometric split.
+		make_unique_pack a &&
+		make_unique_pack b &&
+		make_unique_pack c &&
+		test 4 -eq "$(count_packs)" &&
+		# With the .baddeltas demotion, the big pack is rolled
+		# up despite being above the natural split, and the
+		# resulting pack carries no .baddeltas marker.
+		git repack -d --geometric=2 &&
+		test 1 -eq "$(count_packs)" &&
+		test 0 -eq "$(count_baddeltas)" &&
+		git fsck
+	)
+'
+
+test_expect_success 'geometric repack leaves non-baddeltas packs above the split alone' '
+	test_when_finished "rm -fr work" &&
+	cp -R repo work &&
+	(
+		cd work &&
+		build_n_packs 8 >/dev/null &&
+		git repack -d --geometric=2 &&
+		test 1 -eq "$(count_packs)" &&
+		# Add small packs; without any .baddeltas marker the
+		# big pack should be preserved above the split.
+		make_unique_pack a &&
+		make_unique_pack b &&
+		make_unique_pack c &&
+		test 4 -eq "$(count_packs)" &&
+		git repack -d --geometric=2 &&
+		# 1 kept big pack + 1 newly-aggregated rollup = 2.
+		test 2 -eq "$(count_packs)" &&
+		test 0 -eq "$(count_baddeltas)" &&
+		git fsck
+	)
+'
+
 test_done