pack-bitmap.c: gracefully fallback after opening pack/MIDX

When opening a MIDX/pack-bitmap, we call open_midx_bitmap_1() or open_pack_bitmap_1() respectively in a loop over the set of MIDXs/packs. By design, these functions are supposed to be called over every pack and MIDX, since only one of them should have a valid bitmap. Ordinarily we return '0' from these two functions in order to indicate that we successfully loaded a bitmap To signal that we couldn't load a bitmap corresponding to the MIDX/pack (either because one doesn't exist, or because there was an error with loading it), we can return '-1'. In either case, the callers each enumerate all MIDXs/packs to ensure that at most one bitmap per-kind is present. But when we fail to load a bitmap that does exist (for example, loading a MIDX bitmap without finding a corresponding reverse index), we'll return -1 but leave the 'midx' field non-NULL. So when we fallback to loading a pack bitmap, we'll complain that the bitmap we're trying to populate already is "opened", even though it isn't. Rectify this by setting the '->pack' and '->midx' field back to NULL as appropriate. Two tests are added: one to ensure that the MIDX-to-pack bitmap fallback works, and another to ensure we still complain when there are multiple pack bitmaps in a repository. Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Jan 25, 2022 at 17:41 UTC f8b60cf99b0ab10d915c7bfd4802a1af45d0d439
3 files changed +51
pack-bitmap.c
+4
@@ -358,7 +358,9 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
358 cleanup:
359 munmap(bitmap_git->map, bitmap_git->map_size);
360 bitmap_git->map_size = 0;
361 + bitmap_git->map_pos = 0;
362 bitmap_git->map = NULL;
363 + bitmap_git->midx = NULL;
364 return -1;
365 }
366
@@ -405,6 +407,8 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
407 munmap(bitmap_git->map, bitmap_git->map_size);
408 bitmap_git->map = NULL;
409 bitmap_git->map_size = 0;
410 + bitmap_git->map_pos = 0;
411 + bitmap_git->pack = NULL;
412 return -1;
413 }
414
t/t5310-pack-bitmaps.sh
+28
@@ -397,4 +397,32 @@ test_expect_success 'pack.preferBitmapTips' '
397 )
398 '
399
400 +test_expect_success 'complains about multiple pack bitmaps' '
401 + rm -fr repo &&
402 + git init repo &&
403 + test_when_finished "rm -fr repo" &&
404 + (
405 + cd repo &&
406 +
407 + test_commit base &&
408 +
409 + git repack -adb &&
410 + bitmap="$(ls .git/objects/pack/pack-*.bitmap)" &&
411 + mv "$bitmap" "$bitmap.bak" &&
412 +
413 + test_commit other &&
414 + git repack -ab &&
415 +
416 + mv "$bitmap.bak" "$bitmap" &&
417 +
418 + find .git/objects/pack -type f -name "*.pack" >packs &&
419 + find .git/objects/pack -type f -name "*.bitmap" >bitmaps &&
420 + test_line_count = 2 packs &&
421 + test_line_count = 2 bitmaps &&
422 +
423 + git rev-list --use-bitmap-index HEAD 2>err &&
424 + grep "ignoring extra bitmap file" err
425 + )
426 +'
427 +
428 test_done
t/t5326-multi-pack-bitmaps.sh
+19
@@ -266,4 +266,23 @@ test_expect_success 'hash-cache values are propagated from pack bitmaps' '
266 )
267 '
268
269 +test_expect_success 'graceful fallback when missing reverse index' '
270 + rm -fr repo &&
271 + git init repo &&
272 + test_when_finished "rm -fr repo" &&
273 + (
274 + cd repo &&
275 +
276 + test_commit base &&
277 +
278 + # write a pack and MIDX bitmap containing base
279 + git repack -adb &&
280 + git multi-pack-index write --bitmap &&
281 +
282 + GIT_TEST_MIDX_READ_RIDX=0 \
283 + git rev-list --use-bitmap-index HEAD 2>err &&
284 + ! grep "ignoring extra bitmap file" err
285 + )
286 +'
287 +
288 test_done