midx: fix `BUG()` when getting preferred pack without a reverse index

The function `midx_preferred_pack()` returns the preferred pack for a given multi-pack index. To compute the preferred pack we: 1. Take the first position indexed by the MIDX in pseudo-pack order. 2. Convert this pseudo-pack position into the MIDX position. 3. We then look up the pack that corresponds to this MIDX position. This reliably returns the preferred pack given that all of its contained objects will be up front in pseudo-pack order. The second step that turns the pseudo-pack order into MIDX order requires the reverse index though, which may not exist for example when the MIDX does not have a bitmap. And in that case one may easily hit a bug: BUG: ../pack-revindex.c:491: pack_pos_to_midx: reverse index not yet loaded In theory, `midx_preferred_pack()` already knows to handle the case where no reverse index exists, as it calls `load_midx_revindex()` before calling into `midx_preferred_pack()`. But we only check for negative return values there, even though the function returns a positive error code in case the reverse index does not exist. Fix the issue by testing for a non-zero return value instead, same as all the other callers of this function already do. While at it, document the return value of `load_midx_revindex()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Dec 10, 2025 at 13:52 UTC 665d19ec7bcc2d578e2fa2701f7399a6a965b086
3 files changed +16 -2
midx.c
+1 -1
@@ -688,7 +688,7 @@ int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id)
688 {
689 if (m->preferred_pack_idx == -1) {
690 uint32_t midx_pos;
691 - if (load_midx_revindex(m) < 0) {
691 + if (load_midx_revindex(m)) {
692 m->preferred_pack_idx = -2;
693 return -1;
694 }
pack-revindex.h
+2 -1
@@ -72,7 +72,8 @@ int verify_pack_revindex(struct packed_git *p);
72 * multi-pack index by mmap-ing it and assigning pointers in the
73 * multi_pack_index to point at it.
74 *
75 - * A negative number is returned on error.
75 + * A negative number is returned on error. A positive number is returned in
76 + * case the multi-pack-index does not have a reverse index.
77 */
78 int load_midx_revindex(struct multi_pack_index *m);
79
t/t5319-multi-pack-index.sh
+13
@@ -350,7 +350,20 @@ test_expect_success 'preferred pack from existing MIDX without bitmaps' '
350 # the new MIDX
351 git multi-pack-index write --preferred-pack=pack-$pack.pack
352 )
353 +'
354
355 +test_expect_success 'preferred pack cannot be determined without bitmap' '
356 + test_when_finished "rm -fr preferred-can-be-queried" &&
357 + git init preferred-can-be-queried &&
358 + (
359 + cd preferred-can-be-queried &&
360 + test_commit initial &&
361 + git repack -Adl --write-midx --no-write-bitmap-index &&
362 + test_must_fail test-tool read-midx --preferred-pack .git/objects 2>err &&
363 + test_grep "could not determine MIDX preferred pack" err &&
364 + git repack -Adl --write-midx --write-bitmap-index &&
365 + test-tool read-midx --preferred-pack .git/objects
366 + )
367 '
368
369 test_expect_success 'verify multi-pack-index success' '