[PATCH v2 38/49] builtin/repack.c: inline packs within `write_midx_included_packs()`

Taylor Blau
2025-10-15T22:29:13Z

To write a MIDX at the end of a repack operation, 'git repack' presently
computes the set of packs to write into the MIDX, before invoking
`write_midx_included_packs()` with a `string_list` containing those
packs.

The logic for computing which packs are supposed to appear in the
resulting MIDX is within `midx_included_packs()`, where it is aware of
details like which cruft pack(s) were written/combined, if/how we did a
geometric repack, etc.

Computing this list ourselves before providing it to the sole function
to make use of that list `write_midx_included_packs()` is somewhat
awkward. In the future, repack will learn how to write incremental
MIDXs, which will use a very different pack selection routine.

Instead of doing something like:

struct string_list included_packs = STRING_LIST_INIT_DUP;
if (incremental) {
midx_incremental_included_packs(&included_packs, ...):
write_midx_incremental_included_packs(&included_packs, ...);
} else {
midx_included_packs(&included_packs, ...):
write_midx_included_packs(&included_packs, ...);
}

in the future, let's have each function that writes a MIDX be
responsible for itself computing the list of included packs. Inline the
declaration and initialization of `included_packs` into the
`write_midx_included_packs()` function itself, and repeat that pattern
in the future when we introduce new ways to write MIDXs.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index a518a2b2f3..fad10be42a 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -109,7 +109,6 @@ static int repack_config(const char *var, const char *value,

struct repack_write_midx_opts {
struct existing_packs *existing;
- struct string_list *include;
struct pack_geometry *geometry;
struct string_list *names;
const char *refs_snapshot;
@@ -330,12 +329,14 @@ static void remove_redundant_bitmaps(struct string_list *include,
static int write_midx_included_packs(struct repack_write_midx_opts *opts)
{
struct child_process cmd = CHILD_PROCESS_INIT;
+ struct string_list include = STRING_LIST_INIT_DUP;
struct string_list_item *item;
struct packed_git *preferred = pack_geometry_preferred_pack(opts->geometry);
FILE *in;
int ret = 0;

- if (!opts->include->nr)
+ midx_included_packs(&include, opts);
+ if (!include.nr)
goto done;

cmd.in = -1;
@@ -397,14 +398,17 @@ static int write_midx_included_packs(struct repack_write_midx_opts *opts)
goto done;

in = xfdopen(cmd.in, "w");
- for_each_string_list_item(item, opts->include)
+ for_each_string_list_item(item, &include)
fprintf(in, "%s\n", item->string);
fclose(in);

ret = finish_command(&cmd);
done:
if (!ret && opts->write_bitmaps)
- remove_redundant_bitmaps(opts->include, opts->packdir);
+ remove_redundant_bitmaps(&include, opts->packdir);
+
+ string_list_clear(&include, 0);
+
return ret;
}

@@ -994,10 +998,8 @@ int cmd_repack(int argc,
existing_packs_mark_for_deletion(&existing, &names);

if (write_midx) {
- struct string_list include = STRING_LIST_INIT_DUP;
struct repack_write_midx_opts opts = {
.existing = &existing,
- .include = &include,
.geometry = &geometry,
.names = &names,
.refs_snapshot = refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL,
@@ -1006,12 +1008,9 @@ int cmd_repack(int argc,
.write_bitmaps = write_bitmaps > 0,
.midx_must_contain_cruft = midx_must_contain_cruft
};
- midx_included_packs(&include, &opts);

ret = write_midx_included_packs(&opts);

- string_list_clear(&include, 0);
-
if (ret)
goto cleanup;
}
--
2.51.0.540.ga7423965ad8