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

From: Jeff King <[email-masked]>
Date: Wed, 15 Oct 2025 06:26:35 -0400

On Sun, Sep 28, 2025 at 06:09:40PM -0400, Taylor Blau wrote:

> 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 which 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.

OK. I thought at first this was doing something complicated, but it
really is just: move the call to midx_included_packs() down into the
function which uses it, rather than doing it in the caller.

Without another caller we can't judge this, but I'll take your word that
it helps eventually.

The patch itself looks like it should be a noop with respect to
behavior.

-Peff