Re: [PATCH 3/4] pack-bitmap.c: support 'tree:0' filtering

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> 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?

> @@ -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.

> +}
> +
>  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.