On Tue, Nov 19, 2024 at 05:07:38PM -0500, Taylor Blau wrote:
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 1dddb242434..02864a0e1f7 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -2564,13 +2584,57 @@ static void test_show_commit(struct commit *commit, void *data)
> display_progress(tdata->prg, ++tdata->seen);
> }
>
> +static uint32_t bitmap_total_entry_count(struct bitmap_index *bitmap_git)
> +{
> + uint32_t total = 0;
> + do {
> + total = st_add(total, bitmap_git->entry_count);
> + bitmap_git = bitmap_git->base;
> + } while (bitmap_git);
> +
> + return total;
> +}
> +
> +static void prepare_bitmap_test_data(struct bitmap_test_data *tdata,
> + struct bitmap_index *bitmap_git)
Nit: according to our style guide this should be called
`bitmap_test_data_prepare()`.
> +{
> + memset(tdata, 0, sizeof(struct bitmap_test_data));
> +
> + tdata->bitmap_git = bitmap_git;
> + tdata->base = bitmap_new();
> + tdata->commits = ewah_to_bitmap(bitmap_git->commits);
> + tdata->trees = ewah_to_bitmap(bitmap_git->trees);
> + tdata->blobs = ewah_to_bitmap(bitmap_git->blobs);
> + tdata->tags = ewah_to_bitmap(bitmap_git->tags);
> +
> + if (bitmap_git->base) {
> + CALLOC_ARRAY(tdata->base_tdata, 1);
> + prepare_bitmap_test_data(tdata->base_tdata, bitmap_git->base);
> + }
> +}
> +
> +static void free_bitmap_test_data(struct bitmap_test_data *tdata)
Same nit here, this should be called `bitmap_test_data_free()`. In fact,
it should be called `bitmap_test_data_release()`, because we don't free
`tadata` itself.
> @@ -2579,17 +2643,26 @@ void test_bitmap_walk(struct rev_info *revs)
> if (revs->pending.nr != 1)
> die(_("you must specify exactly one commit to test"));
>
> - fprintf_ln(stderr, "Bitmap v%d test (%d entries%s)",
> + fprintf_ln(stderr, "Bitmap v%d test (%d entries%s, %d total)",
> bitmap_git->version,
> bitmap_git->entry_count,
> - bitmap_git->table_lookup ? "" : " loaded");
> + bitmap_git->table_lookup ? "" : " loaded",
> + bitmap_total_entry_count(bitmap_git));
>
> root = revs->pending.objects[0].item;
> - bm = bitmap_for_commit(bitmap_git, (struct commit *)root);
> + bm = find_bitmap_for_commit(bitmap_git, (struct commit *)root, &found);
>
> if (bm) {
> fprintf_ln(stderr, "Found bitmap for '%s'. %d bits / %08x checksum",
> - oid_to_hex(&root->oid), (int)bm->bit_size, ewah_checksum(bm));
> + oid_to_hex(&root->oid),
> + (int)bm->bit_size, ewah_checksum(bm));
> +
> + if (bitmap_is_midx(found))
> + fprintf_ln(stderr, "Located via MIDX '%s'.",
> + hash_to_hex(get_midx_checksum(found->midx)));
> + else
> + fprintf_ln(stderr, "Located via pack '%s'.",
> + hash_to_hex(found->pack->hash));
>
> result = ewah_to_bitmap(bm);
> }
I'm a bit surprised that this doesn't result in any changes required in
our tests.
Patrick
