From: Jeff King <peff@peff.net>
Date: Sat, 17 Aug 2024 10:31:55 GMT
Subject: Re: [PATCH 1/8] pack-bitmap: initialize `bitmap_writer_init()` with packing_data
Message-ID: <20240817103155.GA551779@coredump.intra.peff.net>
In-Reply-To: <9d322fc5399c453913d08f35eee907b5c909ad6b.1723743050.git.me@ttaylorr.com>

On Thu, Aug 15, 2024 at 01:31:00PM -0400, Taylor Blau wrote:

> In order to determine its object order, the pack-bitmap machinery keeps
> a 'struct packing_data' corresponding to the pack or pseudo-pack (when
> writing a MIDX bitmap) being written.
> 
> The to_pack field is provided to the bitmap machinery by callers of
> bitmap_writer_build() and assigned to the bitmap_writer struct at that
> point.
> 
> But a subsequent commit will want to have access to that data earlier on
> during commit selection. Prepare for that by adding a 'to_pack' argument
> to 'bitmap_writer_init()', and initializing the field during that
> function.
> 
> Subsequent commits will clean up other functions which take
> now-redundant arguments (like nr_objects, which is equivalent to
> pdata->objects_nr, or pdata itself).

This (and the next few follow-on commits) seem like a good change to me.
It simplifies many of the function calls, and I think it expresses the
domain logic in the API: there is a single set of objects being mapped
to bits, and many parts of the process will rely on it.

Even the midx code, which is not generating a pack, uses a "fake"
packing_data as the way to express that (because inherently the bit
ordering is all coming from the pack-index nature). If we likewise ever
wrote code to generate bitmaps from an existing pack, it would probably
use packing_data, too. :)

-Peff