Git development
 help / color / mirror / Atom feed
From: Taylor Blau <ttaylorr@openai.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Ted Nyman <tnyman@openai.com>, Elijah Newren <newren@github.com>
Subject: [PATCH 4/4] repack: retain cruft packs in MIDXs containing kept packs
Date: Tue, 29 Sep 2026 20:28:58 -0500	[thread overview]
Message-ID: <e942c256334e4de31ec0a1cb2d5f8c7465d8696f.1790731662.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1790731662.git.me@ttaylorr.com>

When performing a geometric repack with 'repack.midxMustContainCruft'
set to "false", Git uses '--stdin-packs=follow' to copy (once-cruft)
objects needed for reachability closure out of cruft packs. .keep packs
do not need to participate in that walk, though they *are* included in
the resulting MIDX.

A .keep pack can contain a commit that reaches an object whose only copy
is in a cruft pack. When there is no previous MIDX and the repack writes
a new pack, neither `midx_has_unknown_packs()` nor the `!names.nr`
fallback require that cruft pack to be included. If the kept commit (or
a descendant of it) is selected for bitmap coverage, the bitmap writer
fails because the MIDX does not contain all of its reachable objects.

Include cruft packs whenever the MIDX contains kept packs. This also
retains cruft when the kept packs happen to have full closure, or when
'--pack-kept-objects' lets the repack walk them. It avoids having to
establish their closure before deciding which packs the MIDX needs.

Add a test that packs the tip commit and its tree into a kept pack,
leaving its parent in the cruft pack. The new commit's blob remains
loose, making the geometric repack write a new pack and bypass the
no-new-packs fallback. Verify that the repack succeeds and that we are
able to successfully write a bitmap.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 repack-midx.c           |  5 +++++
 t/t7704-repack-cruft.sh | 20 ++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/repack-midx.c b/repack-midx.c
index 64c7f8d0f42..622c3c9d236 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -197,6 +197,7 @@ static void midx_included_packs(struct string_list *include,
 	}
 
 	if (opts->midx_must_contain_cruft ||
+	    existing->kept_packs.nr ||
 	    midx_has_unknown_packs(include, geometry, existing)) {
 		/*
 		 * If there are one or more unknown pack(s) present (see
@@ -209,6 +210,10 @@ static void midx_included_packs(struct string_list *include,
 		 * reachability closure if the MIDX is bitmapped and one
 		 * or more of the bitmap's selected commits reaches a
 		 * once-cruft object that was later made reachable.
+		 *
+		 * Kept packs may also depend on cruft objects, since
+		 * they are included above without necessarily being
+		 * traversed by the repack.
 		 */
 		for_each_string_list_item(item, &existing->cruft_packs) {
 			/*
diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh
index f7f83e70ffe..02db2a06d9e 100755
--- a/t/t7704-repack-cruft.sh
+++ b/t/t7704-repack-cruft.sh
@@ -798,6 +798,26 @@ test_expect_success 'incremental repack includes cruft for MIDX bitmaps' '
 	)
 '
 
+test_expect_success 'geometric repack includes cruft for kept packs' '
+	setup_cruft_exclude_tests kept-cruft &&
+	(
+		cd kept-cruft &&
+
+		# Keep HEAD and its tree outside the geometric repack. Its
+		# parent is reachable again, but still in the cruft pack.
+		git rev-parse HEAD HEAD^{tree} >objects &&
+		pack=$(git pack-objects $packdir/pack <objects) &&
+		touch $packdir/pack-$pack.keep &&
+		git prune-packed &&
+
+		# The new blob is still loose, so this writes a pack instead
+		# of taking the no-new-packs fallback.
+		GIT_TEST_MULTI_PACK_INDEX=0 \
+		git repack -d --geometric=2 --write-midx --write-bitmap-index &&
+		git rev-list --test-bitmap HEAD
+	)
+'
+
 test_expect_success 'repack --write-midx includes cruft when instructed' '
 	setup_cruft_exclude_tests exclude-cruft-when-instructed &&
 	(
-- 
2.56.0.4.gbee41d2fc68

  parent reply	other threads:[~2026-09-30  1:29 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  1:28 [PATCH 0/4] repack: various corner cases for cruft-less MIDXs Taylor Blau
2026-09-30  1:28 ` [PATCH 1/4] pack-objects: introduce `stdin_packs_context` struct Taylor Blau
2026-09-30 17:42   ` Junio C Hamano
2026-10-01  3:13     ` Taylor Blau
2026-09-30  1:28 ` [PATCH 2/4] pack-objects: ensure tree/tag closure with '--stdin-packs=follow' Taylor Blau
2026-09-30 17:51   ` Junio C Hamano
2026-09-30 18:16   ` Derrick Stolee
2026-10-01  3:14     ` Taylor Blau
2026-10-01 23:22     ` Elijah Newren
2026-10-02  0:51       ` Taylor Blau
2026-10-02 23:02         ` Jeff King
2026-09-30 20:31   ` Jeff King
2026-10-01  3:18     ` Taylor Blau
2026-09-30  1:28 ` [PATCH 3/4] repack: retain cruft packs in MIDXs after incremental repacks Taylor Blau
2026-09-30 20:45   ` Jeff King
2026-10-01  3:21     ` Taylor Blau
2026-09-30  1:28 ` Taylor Blau [this message]
2026-09-30 20:53   ` [PATCH 4/4] repack: retain cruft packs in MIDXs containing kept packs Jeff King
2026-10-01  3:35     ` Taylor Blau
2026-09-30 20:55 ` [PATCH 0/4] repack: various corner cases for cruft-less MIDXs Jeff King
2026-10-01  3:37   ` Taylor Blau
2026-10-01  4:11 ` [PATCH v2 0/8] " Taylor Blau
2026-10-01  4:11   ` [PATCH v2 1/8] pack-objects: introduce `stdin_packs_context` struct Taylor Blau
2026-10-01  4:11   ` [PATCH v2 2/8] pack-objects: ensure tree/tag closure with '--stdin-packs=follow' Taylor Blau
2026-10-02 23:13     ` Jeff King
2026-10-03  0:55       ` Taylor Blau
2026-10-03  1:06         ` Jeff King
2026-10-01  4:11   ` [PATCH v2 3/8] repack: retain cruft packs in MIDXs after incremental repacks Taylor Blau
2026-10-01  4:11   ` [PATCH v2 4/8] repack: use a sorted list for explicitly kept packs Taylor Blau
2026-10-02 23:16     ` Jeff King
2026-10-01  4:11   ` [PATCH v2 5/8] repack: follow kept packs when omitting cruft from the MIDX Taylor Blau
2026-10-02 23:25     ` Jeff King
2026-10-03  0:50       ` Taylor Blau
2026-10-01  4:11   ` [PATCH v2 6/8] repack: track the preferred pack explicitly in MIDX write steps Taylor Blau
2026-10-02 23:28     ` Jeff King
2026-10-03  1:00       ` Taylor Blau
2026-10-03  1:07         ` Jeff King
2026-10-01  4:12   ` [PATCH v2 7/8] repack: defer allocating the append plan's write step Taylor Blau
2026-10-01  4:12   ` [PATCH v2 8/8] repack: include required packs in incremental MIDX writes Taylor Blau
2026-10-02 23:41     ` Jeff King
2026-10-03  1:01       ` Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e942c256334e4de31ec0a1cb2d5f8c7465d8696f.1790731662.git.me@ttaylorr.com \
    --to=ttaylorr@openai.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@github.com \
    --cc=peff@peff.net \
    --cc=tnyman@openai.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox