On Tue, Nov 19, 2024 at 05:07:26PM -0500, Taylor Blau wrote:
> Prepare the pack-bitmap machinery to work with incremental MIDXs by
> adding a new "base" field to keep track of the bitmap index associated
> with the previous MIDX layer.
>
> The changes in this commit are mostly boilerplate to open the correct
> bitmap(s), add them to the chain bitmap layers along the "base" pointer,
s/bitmap layers/of &/
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index bba9c6a905a..41675a69f68 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -54,6 +54,13 @@ struct bitmap_index {
> struct packed_git *pack;
> struct multi_pack_index *midx;
>
> + /*
> + * If using a multi-pack index chain, 'base' points to the
> + * bitmap index corresponding to this bitmap's midx->base_midx.
> + */
> + struct bitmap_index *base;
> + uint32_t base_nr;
> +
It would be nice to point out that `base_nr` is not 0-indexed, but
1-indexed, which is rather uncommon. Is there any particular reason why
you made it 1-indexed?
> @@ -377,8 +384,13 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
> char *midx_bitmap_filename(struct multi_pack_index *midx)
> {
> struct strbuf buf = STRBUF_INIT;
> - get_midx_filename_ext(&buf, midx->object_dir, get_midx_checksum(midx),
> - MIDX_EXT_BITMAP);
> + if (midx->has_chain)
> + get_split_midx_filename_ext(&buf, midx->object_dir,
> + get_midx_checksum(midx),
> + MIDX_EXT_BITMAP);
> + else
> + get_midx_filename_ext(&buf, midx->object_dir,
> + get_midx_checksum(midx), MIDX_EXT_BITMAP);
>
> return strbuf_detach(&buf, NULL);
> }
Okay, this is mostly the same change as in the preceding commit, but for
bitmaps instead of reverse indices.
> @@ -397,10 +409,17 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
> {
> struct stat st;
> char *bitmap_name = midx_bitmap_filename(midx);
> - int fd = git_open(bitmap_name);
> + int fd;
> uint32_t i, preferred_pack;
> struct packed_git *preferred;
>
> + fd = git_open(bitmap_name);
> + if (fd < 0 && errno == ENOENT) {
> + FREE_AND_NULL(bitmap_name);
> + bitmap_name = midx_bitmap_filename(midx);
> + fd = git_open(bitmap_name);
> + }
> +
Wait, this looks weird to me. `bitmap_name` already contains the result
of `midx_bitmap_filename()`, so you're essentially retrying the exact
same operation as before?
Patrick
