Re: [PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips

> We have two locations that iterate over the preferred bitmap tips as
> configured by the user via "pack.preferBitmapTips". Both of these
> callsites are subtly wrong and can lead to a `BUG()`, which we'll fix in
> a subsequent commit.
>
> Prepare for this fix by unifying the two callsites into a new
> `for_each_preferred_bitmap_tip()` function.
>
> This removes the last callsite of `bitmap_preferred_tips()` outside of
> "pack-bitmap.c". As such, convert the function to be local to that file
> only.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/pack-objects.c | 19 ++-----------------
>  pack-bitmap.c          | 18 +++++++++++++++++-
>  pack-bitmap.h          |  9 ++++++++-
>  repack-midx.c          | 14 +++-----------
>  4 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 5846b6a293..979470e402 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -4554,22 +4554,6 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU
>  	return 0;
>  }
>
> -static void mark_bitmap_preferred_tips(void)
> -{
> -	struct string_list_item *item;
> -	const struct string_list *preferred_tips;
> -
> -	preferred_tips = bitmap_preferred_tips(the_repository);
> -	if (!preferred_tips)
> -		return;
> -
> -	for_each_string_list_item(item, preferred_tips) {
> -		refs_for_each_ref_in(get_main_ref_store(the_repository),
> -				     item->string, mark_bitmap_preferred_tip,
> -				     NULL);
> -	}
> -}
> -
>  static inline int is_oid_uninteresting(struct repository *repo,
>  				       struct object_id *oid)
>  {
> @@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
>  		load_delta_islands(the_repository, progress);
>
>  	if (write_bitmap_index)
> -		mark_bitmap_preferred_tips();
> +		for_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,
> +					      NULL);
>
>  	if (!fn_show_object)
>  		fn_show_object = show_object;
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 972203f12b..2f5cb34009 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)
>  	return !!bitmap_git->midx;
>  }
>
> -const struct string_list *bitmap_preferred_tips(struct repository *r)
> +static const struct string_list *bitmap_preferred_tips(struct repository *r)
>  {
>  	const struct string_list *dest;
>
> @@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)
>  	return NULL;
>  }
>
> +void for_each_preferred_bitmap_tip(struct repository *repo,
> +				   each_ref_fn cb, void *cb_data)
> +{
> +	struct string_list_item *item;
> +	const struct string_list *preferred_tips;
> +
> +	preferred_tips = bitmap_preferred_tips(repo);
> +	if (!preferred_tips)
> +		return;
> +

> +	for_each_string_list_item(item, preferred_tips) {
> +		refs_for_each_ref_in(get_main_ref_store(repo),
> +				     item->string, cb, cb_data);
> +	}
> +}
> +
>  int bitmap_is_preferred_refname(struct repository *r, const char *refname)
>  {
>  	const struct string_list *preferred_tips = bitmap_preferred_tips(r);
> diff --git a/pack-bitmap.h b/pack-bitmap.h
> index 1bd7a791e2..d0611d0481 100644
> --- a/pack-bitmap.h
> +++ b/pack-bitmap.h
> @@ -5,6 +5,7 @@
>  #include "khash.h"
>  #include "pack.h"
>  #include "pack-objects.h"
> +#include "refs.h"
>  #include "string-list.h"
>
>  struct commit;
> @@ -99,6 +100,13 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  			      show_reachable_fn show_reach,
>  			      void *payload);
>
> +/*
> + * Iterate over all references that are configured as preferred bitmap tips via
> + * "pack.preferBitmapTips" and invoke the callback on each function.
> + */
> +void for_each_preferred_bitmap_tip(struct repository *repo,
> +				   each_ref_fn cb, void *cb_data);
> +
>  #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \
>  	"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL"
>
> @@ -182,7 +190,6 @@ char *pack_bitmap_filename(struct packed_git *p);
>
>  int bitmap_is_midx(struct bitmap_index *bitmap_git);
>
> -const struct string_list *bitmap_preferred_tips(struct repository *r);
>  int bitmap_is_preferred_refname(struct repository *r, const char *refname);
>
>  int verify_bitmap_files(struct repository *r);
> diff --git a/repack-midx.c b/repack-midx.c
> index 74bdfa3a6e..0682b80c42 100644
> --- a/repack-midx.c
> +++ b/repack-midx.c
> @@ -40,7 +40,6 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data)
>  void midx_snapshot_refs(struct repository *repo, struct tempfile *f)
>  {
>  	struct midx_snapshot_ref_data data;
> -	const struct string_list *preferred = bitmap_preferred_tips(repo);
>
>  	data.repo = repo;
>  	data.f = f;
> @@ -51,16 +50,9 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)
>  		 die(_("could not open tempfile %s for writing"),
>  		     get_tempfile_path(f));
>
> -	if (preferred) {
> -		struct string_list_item *item;
> -
> -		data.preferred = 1;
> -		for_each_string_list_item(item, preferred)
> -			refs_for_each_ref_in(get_main_ref_store(repo),
> -					     item->string,
> -					     midx_snapshot_ref_one, &data);
> -		data.preferred = 0;
> -	}
> +	data.preferred = 1;
> +	for_each_preferred_bitmap_tip(repo, midx_snapshot_ref_one, &data);
> +	data.preferred = 0;
>

>  	refs_for_each_ref(get_main_ref_store(repo),
>  			  midx_snapshot_ref_one, &data);
>
> --
> 2.53.0.rc2.206.g60c1bca835.dirty