Re: [PATCH 03/11] pack-objects: use rev.filter when possible

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 4, 2022, 22:25 UTC
Message-ID: <xmqqbkylz77a.fsf@gitster.g>
In-Reply-To: <d5edb193229731edbac9f861ad569885ec2b4405.1645638911.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines

> From: Derrick Stolee <derrickstolee@github.com>
>
> In builtin/pack-objects.c, we use a 'filter_options' global to populate
> the --filter=<X> argument. The previous change created a pointer to a
> filter option in 'struct rev_info', so we can use that pointer here as a
> start to simplifying some usage of object filters.

Hmph, it is very unfortunate that we cannot really get rid of the file-scope static filter_options easily and replace it with an instance of "struct list_objects_filter_options" embedded in rev_info, because cmd_pack_objects(), where the place filter-options are parsed, does not have an instance of "struct rev_info" to use yet, and it takes some code restructuring to get there.

And that is why ...

Show 23 quoted lines

> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  builtin/pack-objects.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index ba2006f2212..256d9b1798f 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3651,7 +3651,7 @@ static int pack_options_allow_reuse(void)
>  
>  static int get_object_list_from_bitmap(struct rev_info *revs)
>  {
> -	if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
> +	if (!(bitmap_git = prepare_bitmap_walk(revs, revs->filter, 0)))
>  		return -1;
>  
>  	if (pack_options_allow_reuse() &&
> @@ -3727,6 +3727,7 @@ static void get_object_list(int ac, const char **av)
>  	repo_init_revisions(the_repository, &revs, NULL);
>  	save_commit_buffer = 0;
>  	setup_revisions(ac, av, &revs, &s_r_opt);
> +	revs.filter = &filter_options;

... we need something like this.

Nothing wrong per-se, feels somewhat unsatisfactory, but it would work OK, which counts ;-)

Show 10 quoted lines

>  	/* make sure shallows are read */
>  	is_repository_shallow(the_repository);
> @@ -3777,7 +3778,7 @@ static void get_object_list(int ac, const char **av)
>  
>  	if (!fn_show_object)
>  		fn_show_object = show_object;
> -	traverse_commit_list_filtered(&filter_options, &revs,
> +	traverse_commit_list_filtered(revs.filter, &revs,
>  				      show_commit, fn_show_object, NULL,
>  				      NULL);