Re: [PATCH v2 3/8] midx-write.c: pass `start_pack` to `compute_sorted_entries()`

From: Jeff King <peff@peff.net>
Date: May 30, 2024, 06:59 UTC
Message-ID: <20240530065943.GA1949834@coredump.intra.peff.net>
In-Reply-To: <e81296f8ccf8b47758d0ec4fbb04f0f2b356b490.1717023301.git.me@ttaylorr.com>

On Wed, May 29, 2024 at 06:55:28PM -0400, Taylor Blau wrote:

Show 13 quoted lines

> The function `compute_sorted_entries()` is broadly responsible for
> building an array of the objects to be written into a MIDX based on the
> provided list of packs.
> 
> If we have loaded an existing MIDX, however, we may not use all of its
> packs, despite loading them into the ctx->info array.
> 
> The existing implementation simply skips past the first
> ctx->m->num_packs (if ctx->m is non-NULL, indicating that we loaded an
> existing MIDX). This is because we read objects in packs from an
> existing MIDX via the MIDX itself, rather than from the pack-level
> fanout to guarantee a de-duplicated result (see: a40498a1265 (midx: use
> existing midx when writing new one, 2018-07-12)).

Hmm, if the last patch had not removed the separate array/nr pair for packs, then you could just pass (array + start_pack, nr - start_pack). :)

But I think it is probably reasonable to keep the notion of "here are all the packs" and "you are skipping some of them" clear through the call stack anyway. Especially if the notion of start_pack gets more complicated:

> Future changes (outside the scope of this patch series) to the MIDX code
> will require us to skip *at most* that number[^1].

So this patch seems fine to me.

-Peff