Re: [PATCH v3 03/13] pack-bitmap.c: open and store incremental bitmap layers
From: Patrick Steinhardt <hidden>
Date: 2025-02-28 10:01:19
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 &/
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
@@ -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