midx-write.c: tolerate `--preferred-pack` without bitmaps

When passing a preferred pack to the MIDX write machinery, we ensure
that the given preferred pack is non-empty since 5d3cd09a808 (midx:
reject empty `--preferred-pack`'s, 2021-08-31).

However packs are only loaded (via `write_midx_internal()`, though a
subsequent patch will refactor this code out to its own function) when
the `MIDX_WRITE_REV_INDEX` flag is set.

So if a caller runs:

    $ git multi-pack-index write --preferred-pack=...

with both (a) an existing MIDX, and (b) specifies a pack from that MIDX
as the preferred one, without passing `--bitmap`, then the check added
in 5d3cd09a808 will result in a segfault.

Note that packs loaded from disk which don't appear in an existing MIDX
do not trigger this issue, as those packs are loaded unconditionally. We
conditionally load packs from a MIDX since we tolerate MIDXs whose
packs do not resolve (i.e., via the MIDX write after removing
unreferenced packs via 'git multi-pack-index expire').

In practice, this isn't possible to trigger when running `git
multi-pack-index write` from `git repack`, as the latter always passes
`--stdin-packs`, which prevents us from loading an existing MIDX, as it
forces all packs to be read from disk.

But a future commit in this series will change that behavior to
unconditionally load an existing MIDX, even with `--stdin-packs`, making
this behavior trigger-able from 'repack' much more easily.

Prevent this from being an issue by removing the segfault altogether by
calling `prepare_midx_pack()` on packs loaded from an existing MIDX when
either the `MIDX_WRITE_REV_INDEX` flag is set *or* we specified a
`--preferred-pack`.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/midx-write.c b/midx-write.c
index 65e69d2de787cf784bb1736c7d834ce1f8aeebaf..fface1e93a8f1d733e2bd423fb329d7ec77b94f4 100644 (file)
--- a/midx-write.c
+++ b/midx-write.c
@@ -929,11 +929,17 @@ static int write_midx_internal(const char *object_dir,
                for (i = 0; i < ctx.m->num_packs; i++) {
                        ALLOC_GROW(ctx.info, ctx.nr + 1, ctx.alloc);
 
-                       if (flags & MIDX_WRITE_REV_INDEX) {
+                       if (flags & MIDX_WRITE_REV_INDEX ||
+                           preferred_pack_name) {
                                /*
                                 * If generating a reverse index, need to have
                                 * packed_git's loaded to compare their
                                 * mtimes and object count.
+                                *
+                                * If a preferred pack is specified,
+                                * need to have packed_git's loaded to
+                                * ensure the chosen preferred pack has
+                                * a non-zero object count.
                                 */
                                if (prepare_midx_pack(the_repository, ctx.m, i)) {
                                        error(_("could not load pack"));

diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index dd09134db03638fd01e73d89acbac45cceaa6e46..10d2a6bf9231d1931e7d06065c2e1b15370edfd7 100755 (executable)
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -350,6 +350,29 @@ test_expect_success 'preferred packs must be non-empty' '
        )
 '
 
+test_expect_success 'preferred pack from existing MIDX without bitmaps' '
+       git init preferred-without-bitmaps &&
+       (
+               cd preferred-without-bitmaps &&
+
+               test_commit one &&
+               pack="$(git pack-objects --all $objdir/pack/pack </dev/null)" &&
+
+               git multi-pack-index write &&
+
+               # make another pack so that the subsequent MIDX write
+               # has something to do
+               test_commit two &&
+               git repack -d &&
+
+               # write a new MIDX without bitmaps reusing the singular
+               # pack from the existing MIDX as the preferred pack in
+               # the new MIDX
+               git multi-pack-index write --preferred-pack=pack-$pack.pack
+       )
+
+'
+
 test_expect_success 'verify multi-pack-index success' '
        git multi-pack-index verify --object-dir=$objdir
 '