pack-bitmap: iterate object sources when opening bitmaps

When opening a bitmap for a repository we perform two steps:

  - We first look for a multi-pack index bitmap in any of the object
    sources connected to the repository.

  - We then look for a packfile bitmap in any of the packfiles of any of
    the object sources.

Both of these steps thus iterate through object sources themselves, one
via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This
layout makes it hard to introduce a way to open the bitmap of one
specific object source, which is functionality that we'll require in a
subsequent commit.

Reverse the loop so that we instead loop through all sources in the
outer loop, and then for each source we try to load its bitmap via
either the multi-pack index or via a packfile.

Note that this changes the precedence of bitmaps in one specific edge
case: when an earlier object source only has a packfile bitmap, but a
later source has a multi-pack index bitmap, we now pick the packfile
bitmap of the earlier source. Previously, a multi-pack index bitmap from
any source would have taken precedence over all packfile bitmaps. Given
that object sources are ordered such that the local source comes first,
this arguably is an improvement, as we now prefer local bitmaps over
bitmaps in alternates. Furthermore, we already warn about repositories
that have multiple bitmaps, so this setup is broken and thus arguably
not worth worrying about too much.

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

diff --git a/pack-bitmap.c b/pack-bitmap.c
index eda38a54337395e392df34c7df5ab951b76c3deb..e32795a595b8a7d5d799e21958a821a40923df71 100644 (file)
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
        return 0;
 }
 
-static int open_pack_bitmap(struct repository *r,
-                           struct bitmap_index *bitmap_git)
+static int open_bitmap_for_source(struct odb_source_packed *source,
+                                 struct bitmap_index *bitmap_git)
 {
-       struct packed_git *p;
-       int ret = -1;
+       struct multi_pack_index *midx = get_multi_pack_index(source);
+       struct packfile_list_entry *e;
+       bool found = false;
 
-       repo_for_each_pack(r, p) {
-               if (open_pack_bitmap_1(bitmap_git, p) == 0) {
-                       ret = 0;
-                       /*
-                        * The only reason to keep looking is to report
-                        * duplicates.
-                        */
-                       if (!trace2_is_enabled())
-                               break;
-               }
+       if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+               found = true;
+
+       for (e = packfile_store_get_packs(source); e; e = e->next) {
+               /*
+                * When tracing is enabled we want to keep looking to report
+                * duplicates even if we have already found a bitmap.
+                */
+               if (found && !trace2_is_enabled())
+                       break;
+
+               if (!open_pack_bitmap_1(bitmap_git, e->pack))
+                       found = true;
        }
 
-       return ret;
+       return found ? 0 : -1;
 }
 
-static int open_midx_bitmap(struct repository *r,
-                           struct bitmap_index *bitmap_git)
+static int open_bitmap(struct repository *r,
+                      struct bitmap_index *bitmap_git)
 {
        struct odb_source *source;
-       int ret = -1;
+       bool found = false;
 
        assert(!bitmap_git->map);
 
        odb_prepare_alternates(r->objects);
        for (source = r->objects->sources; source; source = source->next) {
                struct odb_source_files *files = odb_source_files_downcast(source);
-               struct multi_pack_index *midx = get_multi_pack_index(files->packed);
-               if (midx && !open_midx_bitmap_1(bitmap_git, midx))
-                       ret = 0;
-       }
-       return ret;
-}
-
-static int open_bitmap(struct repository *r,
-                      struct bitmap_index *bitmap_git)
-{
-       int found;
 
-       assert(!bitmap_git->map);
+               if (!open_bitmap_for_source(files->packed, bitmap_git))
+                       found = true;
 
-       found = !open_midx_bitmap(r, bitmap_git);
-
-       /*
-        * these will all be skipped if we opened a midx bitmap; but run it
-        * anyway if tracing is enabled to report the duplicates
-        */
-       if (!found || trace2_is_enabled())
-               found |= !open_pack_bitmap(r, bitmap_git);
+               /*
+                * The only reason to keep looking after having found a bitmap
+                * is to report duplicates.
+                */
+               if (found && !trace2_is_enabled())
+                       break;
+       }
 
        return found ? 0 : -1;
 }