Thread (158 messages) 158 messages, 3 authors, 2025-10-16

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

From: Jeff King <hidden>
Date: 2025-10-15 10:33:21

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help