Re: [PATCH v3 1/2] pack-bitmap.c: avoid exposing absolute paths

[Taylor Blau <me@ttaylorr.com>]

On Fri, Nov 04, 2022 at 11:17:09AM +0800, Teng Long wrote:
> @@ -354,8 +354,9 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
>  	if (bitmap_git->pack || bitmap_git->midx) {
>  		struct strbuf buf = STRBUF_INIT;
>  		get_midx_filename(&buf, midx->object_dir);
> -		/* ignore extra bitmap file; we can only handle one */
> -		warning(_("ignoring extra bitmap file: '%s'"), buf.buf);
> +		/* ignore extra midx bitmap files; we can only handle one */
> +		trace2_data_string("bitmap", the_repository,
> +				   "ignoring extra midx bitmap file", buf.buf);

This is looking good, though I think we *could* drop the comment here.
It's as redundant in the new version as it was in the old (i.e., we say
that we ignore extra bitmap files in a comment, and then issue a warning
with basically the same message).

But I don't feel strongly about it, so I think this patch is fine with
or without that comment.

> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index 6d693eef82..0b422c8a63 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -428,8 +428,9 @@ test_bitmap_cases () {
>  			test_line_count = 2 packs &&
>  			test_line_count = 2 bitmaps &&
>
> -			git rev-list --use-bitmap-index HEAD 2>err &&
> -			grep "ignoring extra bitmap file" err
> +			GIT_TRACE2_PERF=$(pwd)/trace2.txt git rev-list --use-bitmap-index HEAD &&

Do we need to clean up an existing trace2.txt here? Looks like not, so
OK. Is there a reason we need GIT_TRACE2_PERF and not GIT_TRACE2_EVENT
like the rest of the tests in t5310?

> +			grep "opened bitmap" trace2.txt &&
> +			grep "ignoring extra bitmap" trace2.txt

These two look good.

Thanks,
Taylor