Re: [PATCH 3/4] pack-bitmap.c: support 'tree:0' filtering
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 5, 2020, 05:25 UTC
- Message-ID
- <xmqq7dxrc5r9.fsf@gitster.c.googlers.com>
- In-Reply-To
- <87b21d72bb588f7366d928544aeaf4de68b027a7.1588633810.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 11 quoted lines
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 3693c9e62f..195ee8cad0 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -749,7 +749,7 @@ static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git,
> eword_t mask;
> uint32_t i;
>
> - if (type != OBJ_BLOB)
> + if (type != OBJ_BLOB && type != OBJ_TREE)
> BUG("filter_bitmap_exclude_type: unsupported type '%d'", type);OK. This is the same as the previous step, but why would we even need this guard? find_tip_objects() is equipped to find tips of any object type, iterating on the bitmap for "type", or flipping the bits in the to_filter bitmap, does not have any limitation to the blob type in the previous step, and there is no limitation to the blob or tree types after this step, either, no?
Show 11 quoted lines
> @@ -867,6 +867,20 @@ static void filter_bitmap_blob_limit(struct bitmap_index *bitmap_git,
> bitmap_free(tips);
> }
>
> +static void filter_bitmap_tree_depth(struct bitmap_index *bitmap_git,
> + struct object_list *tip_objects,
> + struct bitmap *to_filter,
> + unsigned long limit)
> +{
> + if (limit)
> + BUG("filter_bitmap_tree_depth given non-zero limit");This one does make sense, because the code to exclude all trees and all blobs we have below won't be able to cull only trees at a given depth or deeper.
> + filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, > + OBJ_TREE); > + filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, > + OBJ_BLOB);
And these two are quite straight-forward.
Show 15 quoted lines
> +}
> +
> static int filter_bitmap(struct bitmap_index *bitmap_git,
> struct object_list *tip_objects,
> struct bitmap *to_filter,
> @@ -890,6 +904,15 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
> return 0;
> }
>
> + if (filter->choice == LOFC_TREE_DEPTH &&
> + filter->tree_exclude_depth == 0) {
> + if (bitmap_git)
> + filter_bitmap_tree_depth(bitmap_git, tip_objects,
> + to_filter,
> + filter->tree_exclude_depth);I briefly wondered if it is cleaner to read if we hardcode 0 as the last argument. But if the helper function ever learns how to filter by tree with non-zero depth, we can only tweak the if() condition without changing the call, so the way you wrote it is the right way.
Thanks.