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: when the preferred bitmap tips contain an
exact refname match, then we will hit a `BUG()`.

Prepare for the 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. Note that the function is still used by a second caller, so we
cannot just inline it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5846b6a293a27b0841d54213cc71e616855e59f8..979470e402aa7056bf0930980f746ed242240139 100644 (file)
--- 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 972203f12b6d9b9e18a08a5b7d81d284f162c567..2f5cb3400970a3f0b38d5ada2cb1084e923a8396 100644 (file)
--- 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 1bd7a791e2a0d0794dc235b3f71293b5cc142a0b..d0611d0481301014b805c1c1c714a06e4036ac87 100644 (file)
--- 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 74bdfa3a6e913f65679076689f723d64e351584d..0682b80c4278d4071eac94f2adda714ce531b6c2 100644 (file)
--- 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);