midx: traverse the local MIDX first

When a repository has an alternate object directory configured, callers
can traverse through each alternate's MIDX by walking the '->next'
pointer.

But, when 'prepare_multi_pack_index_one()' loads multiple MIDXs, it
places the new ones at the front of this pointer chain, not at the end.
This can be confusing for callers such as 'git repack -ad', causing test
failures like in t7700.6 with 'GIT_TEST_MULTI_PACK_INDEX=1'.

The occurs when dropping a pack known to the local MIDX with alternates
configured that have their own MIDX. Since the alternate's MIDX is
returned via 'get_multi_pack_index()', 'midx_contains_pack()' returns
true (which is correct, since it traverses through the '->next' pointer
to find the MIDX in the chain that does contain the requested object).
But, we call 'clear_midx_file()' on 'the_repository', which drops the
MIDX at the path of the first MIDX in the chain, which (in the case of
t7700.6 is the one in the alternate).

This patch addresses that by:

  - placing the local MIDX first in the chain when calling
    'prepare_multi_pack_index_one()', and

  - introducing a new 'get_local_multi_pack_index()', which explicitly
    returns the repository-local MIDX, if any.

Don't impose an additional order on the MIDX's '->next' pointer beyond
that the first item in the chain must be local if one exists so that we
avoid a quadratic insertion.

Likewise, use 'get_local_multi_pack_index()' in
'remove_redundant_pack()' to fix the formerly broken t7700.6 when run
with 'GIT_TEST_MULTI_PACK_INDEX=1'.

Finally, note that the MIDX ordering invariant is only preserved by the
insertion order in 'prepare_packed_git()', which traverses through the
ODB's '->next' pointer, meaning we visit the local object store first.
This fragility makes this an undesirable long-term solution if more
callers are added, but it is acceptable for now since this is the only
caller.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/repack.c b/builtin/repack.c
index f10f52779cd92f66b7539879a18d631cf47cea6b..60cb1969565b15de26463f6ed17e15c01c162b65 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -133,7 +133,7 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list,
 static void remove_redundant_pack(const char *dir_name, const char *base_name)
 {
        struct strbuf buf = STRBUF_INIT;
-       struct multi_pack_index *m = get_multi_pack_index(the_repository);
+       struct multi_pack_index *m = get_local_multi_pack_index(the_repository);
        strbuf_addf(&buf, "%s.pack", base_name);
        if (m && midx_contains_pack(m, buf.buf))
                clear_midx_file(the_repository);

diff --git a/midx.c b/midx.c
index 6d1584ca51d313343bbb65f9baa7c794259c7bf0..d956af6b1f80063caa167ffb4a74f37d8d584fe3 100644 (file)
--- a/midx.c
+++ b/midx.c
@@ -402,8 +402,12 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i
        m = load_multi_pack_index(object_dir, local);
 
        if (m) {
-               m->next = r->objects->multi_pack_index;
-               r->objects->multi_pack_index = m;
+               struct multi_pack_index *mp = r->objects->multi_pack_index;
+               if (mp) {
+                       m->next = mp->next;
+                       mp->next = m;
+               } else
+                       r->objects->multi_pack_index = m;
                return 1;
        }
 

diff --git a/packfile.c b/packfile.c
index 6ab5233613e2417f8ee9ce0991ae532726b59b20..9ef27508f2ed6d2685e5d85cd92667ac735140d5 100644 (file)
--- a/packfile.c
+++ b/packfile.c
@@ -1027,6 +1027,17 @@ struct multi_pack_index *get_multi_pack_index(struct repository *r)
        return r->objects->multi_pack_index;
 }
 
+struct multi_pack_index *get_local_multi_pack_index(struct repository *r)
+{
+       struct multi_pack_index *m = get_multi_pack_index(r);
+
+       /* no need to iterate; we always put the local one first (if any) */
+       if (m && m->local)
+               return m;
+
+       return NULL;
+}
+
 struct packed_git *get_all_packs(struct repository *r)
 {
        struct multi_pack_index *m;

diff --git a/packfile.h b/packfile.h
index 240aa73b95a64912fb2cde5be049f57d91e53b84..a58fc738e06319624b666c80e2636217400cdda9 100644 (file)
--- a/packfile.h
+++ b/packfile.h
@@ -57,6 +57,7 @@ void install_packed_git(struct repository *r, struct packed_git *pack);
 struct packed_git *get_packed_git(struct repository *r);
 struct list_head *get_packed_git_mru(struct repository *r);
 struct multi_pack_index *get_multi_pack_index(struct repository *r);
+struct multi_pack_index *get_local_multi_pack_index(struct repository *r);
 struct packed_git *get_all_packs(struct repository *r);
 
 /*