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>

diff --git a/pack-bitmap.c b/pack-bitmap.c
index f772d3cb7f7c181a9b703a9535f9ec55dfd60513..9c666cdb8bd9203f0749b54b6c012ba33896341e 100644 (file)
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -358,7 +358,9 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 cleanup:
        munmap(bitmap_git->map, bitmap_git->map_size);
        bitmap_git->map_size = 0;
+       bitmap_git->map_pos = 0;
        bitmap_git->map = NULL;
+       bitmap_git->midx = NULL;
        return -1;
 }
 
@@ -405,6 +407,8 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
                munmap(bitmap_git->map, bitmap_git->map_size);
                bitmap_git->map = NULL;
                bitmap_git->map_size = 0;
+               bitmap_git->map_pos = 0;
+               bitmap_git->pack = NULL;
                return -1;
        }
 

diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index d05ab716f6aa1fd6f641f14e308e2e5b80e70af3..f775fc1ce691d5337b31d067cd93944b7bae3bc5 100755 (executable)
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -397,4 +397,32 @@ test_expect_success 'pack.preferBitmapTips' '
        )
 '
 
+test_expect_success 'complains about multiple pack bitmaps' '
+       rm -fr repo &&
+       git init repo &&
+       test_when_finished "rm -fr repo" &&
+       (
+               cd repo &&
+
+               test_commit base &&
+
+               git repack -adb &&
+               bitmap="$(ls .git/objects/pack/pack-*.bitmap)" &&
+               mv "$bitmap" "$bitmap.bak" &&
+
+               test_commit other &&
+               git repack -ab &&
+
+               mv "$bitmap.bak" "$bitmap" &&
+
+               find .git/objects/pack -type f -name "*.pack" >packs &&
+               find .git/objects/pack -type f -name "*.bitmap" >bitmaps &&
+               test_line_count = 2 packs &&
+               test_line_count = 2 bitmaps &&
+
+               git rev-list --use-bitmap-index HEAD 2>err &&
+               grep "ignoring extra bitmap file" err
+       )
+'
+
 test_done

diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh
index c0924074c4de0eddec3ebb8ff9c66484c79483c9..3c1ecc7e25017399381d581258d3d0817a5f4b1b 100755 (executable)
--- a/t/t5326-multi-pack-bitmaps.sh
+++ b/t/t5326-multi-pack-bitmaps.sh
@@ -266,4 +266,23 @@ test_expect_success 'hash-cache values are propagated from pack bitmaps' '
        )
 '
 
+test_expect_success 'graceful fallback when missing reverse index' '
+       rm -fr repo &&
+       git init repo &&
+       test_when_finished "rm -fr repo" &&
+       (
+               cd repo &&
+
+               test_commit base &&
+
+               # write a pack and MIDX bitmap containing base
+               git repack -adb &&
+               git multi-pack-index write --bitmap &&
+
+               GIT_TEST_MIDX_READ_RIDX=0 \
+                       git rev-list --use-bitmap-index HEAD 2>err &&
+               ! grep "ignoring extra bitmap file" err
+       )
+'
+
 test_done