From: Taylor Blau <me@ttaylorr.com>
Date: Wed, 15 Oct 2025 22:28:59 +0000 (-0400)
Subject: repack: keep track of MIDX pack names using existing_packs
X-Git-Tag: v2.52.0-rc0~16^2~15
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2fee63a71ae8113fd91d8e5924ae4a5619ad0cd3;p=thirdparty%2Fgit.git

repack: keep track of MIDX pack names using existing_packs

Instead of storing the list of MIDX pack names separately, let's inline
it into the existing_packs struct, further reducing the number of
parameters we have to pass around.

This amounts to adding a new string_list to the existing_packs struct,
and populating it via `existing_packs_collect()`. This is fairly
straightforward to do, since we are already looping over all packs, all
we need to do is:

    if (p->multi_pack_index)
        string_list_append(&existing->midx_packs, pack_basename(p));

Note, however, that this check *must* come before other conditions where
we discard and do not keep track of a pack, including the condition "if
(!p->pack_local)" immediately below. This is because the existing
routine which collects MIDX pack names does so blindly, and does not
discard, for example, non-local packs.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

diff --git a/builtin/repack.c b/builtin/repack.c
index dda533f171..a57a14ef60 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -118,8 +118,7 @@ struct repack_write_midx_opts {
 	int midx_must_contain_cruft;
 };
 
-static int midx_has_unknown_packs(struct string_list *midx_pack_names,
-				  struct string_list *include,
+static int midx_has_unknown_packs(struct string_list *include,
 				  struct pack_geometry *geometry,
 				  struct existing_packs *existing)
 {
@@ -127,7 +126,7 @@ static int midx_has_unknown_packs(struct string_list *midx_pack_names,
 
 	string_list_sort(include);
 
-	for_each_string_list_item(item, midx_pack_names) {
+	for_each_string_list_item(item, &existing->midx_packs) {
 		const char *pack_name = item->string;
 
 		/*
@@ -190,7 +189,6 @@ static int midx_has_unknown_packs(struct string_list *midx_pack_names,
 
 static void midx_included_packs(struct string_list *include,
 				struct existing_packs *existing,
-				struct string_list *midx_pack_names,
 				struct string_list *names,
 				struct pack_geometry *geometry)
 {
@@ -245,8 +243,7 @@ static void midx_included_packs(struct string_list *include,
 	}
 
 	if (midx_must_contain_cruft ||
-	    midx_has_unknown_packs(midx_pack_names, include, geometry,
-				   existing)) {
+	    midx_has_unknown_packs(include, geometry, existing)) {
 		/*
 		 * If there are one or more unknown pack(s) present (see
 		 * midx_has_unknown_packs() for what makes a pack
@@ -604,7 +601,6 @@ int cmd_repack(int argc,
 	struct child_process cmd = CHILD_PROCESS_INIT;
 	struct string_list_item *item;
 	struct string_list names = STRING_LIST_INIT_DUP;
-	struct string_list midx_pack_names = STRING_LIST_INIT_DUP;
 	struct existing_packs existing = EXISTING_PACKS_INIT;
 	struct pack_geometry geometry = { 0 };
 	struct tempfile *refs_snapshot = NULL;
@@ -978,18 +974,6 @@ int cmd_repack(int argc,
 
 	string_list_sort(&names);
 
-	if (get_multi_pack_index(repo->objects->sources)) {
-		struct multi_pack_index *m =
-			get_multi_pack_index(repo->objects->sources);
-
-		for (; m; m = m->base_midx) {
-			for (uint32_t i = 0; i < m->num_packs; i++) {
-				string_list_append(&midx_pack_names,
-						   m->pack_names[i]);
-			}
-		}
-	}
-
 	close_object_store(repo->objects);
 
 	/*
@@ -1015,8 +999,7 @@ int cmd_repack(int argc,
 			.write_bitmaps = write_bitmaps > 0,
 			.midx_must_contain_cruft = midx_must_contain_cruft
 		};
-		midx_included_packs(&include, &existing, &midx_pack_names,
-				    &names, &geometry);
+		midx_included_packs(&include, &existing, &names, &geometry);
 
 		ret = write_midx_included_packs(&opts);
 
@@ -1063,7 +1046,6 @@ int cmd_repack(int argc,
 cleanup:
 	string_list_clear(&keep_pack_list, 0);
 	string_list_clear(&names, 1);
-	string_list_clear(&midx_pack_names, 0);
 	existing_packs_release(&existing);
 	pack_geometry_release(&geometry);
 	pack_objects_args_release(&po_args);
diff --git a/repack.c b/repack.c
index d8afdd352d..1d485e0112 100644
--- a/repack.c
+++ b/repack.c
@@ -80,6 +80,9 @@ void existing_packs_collect(struct existing_packs *existing,
 		size_t i;
 		const char *base;
 
+		if (p->multi_pack_index)
+			string_list_append(&existing->midx_packs,
+					    pack_basename(p));
 		if (!p->pack_local)
 			continue;
 
@@ -104,6 +107,7 @@ void existing_packs_collect(struct existing_packs *existing,
 	string_list_sort(&existing->kept_packs);
 	string_list_sort(&existing->non_kept_packs);
 	string_list_sort(&existing->cruft_packs);
+	string_list_sort(&existing->midx_packs);
 	strbuf_release(&buf);
 }
 
@@ -220,6 +224,7 @@ void existing_packs_release(struct existing_packs *existing)
 	string_list_clear(&existing->kept_packs, 0);
 	string_list_clear(&existing->non_kept_packs, 0);
 	string_list_clear(&existing->cruft_packs, 0);
+	string_list_clear(&existing->midx_packs, 0);
 }
 
 static struct {
diff --git a/repack.h b/repack.h
index 803e129224..6aa5b4e0f0 100644
--- a/repack.h
+++ b/repack.h
@@ -40,6 +40,7 @@ struct existing_packs {
 	struct string_list kept_packs;
 	struct string_list non_kept_packs;
 	struct string_list cruft_packs;
+	struct string_list midx_packs;
 };
 
 #define EXISTING_PACKS_INIT { \