From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, stolee@gmail.com
Subject: [PATCH v3 9/9] pack-bitmap.c: gracefully fallback after opening pack/MIDX
Date: Tue, 4 Jan 2022 13:16:02 -0500 [thread overview]
Message-ID: <9707d5ea4433d9a5c7f8581dbb2d8a05f410efd3.1641320129.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1641320129.git.me@ttaylorr.com>
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>
---
pack-bitmap.c | 4 ++++
t/t5310-pack-bitmaps.sh | 28 ++++++++++++++++++++++++++++
t/t5326-multi-pack-bitmaps.sh | 19 +++++++++++++++++++
3 files changed, 51 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index f772d3cb7f..9c666cdb8b 100644
--- 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 d05ab716f6..f775fc1ce6 100755
--- 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 c0924074c4..3c1ecc7e25 100755
--- 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
--
2.34.1.25.gb3157a20e6
next prev parent reply other threads:[~2022-01-04 18:16 UTC|newest]
Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-08 19:26 [PATCH 0/2] midx: prevent bitmap corruption when permuting pack order Taylor Blau
2021-12-08 19:26 ` [PATCH 1/2] t5326: demonstrate bitmap corruption after permutation Taylor Blau
2021-12-08 19:26 ` [PATCH 2/2] midx.c: make changing the preferred pack safe Taylor Blau
2021-12-08 19:30 ` [PATCH 0/2] midx: prevent bitmap corruption when permuting pack order Derrick Stolee
2021-12-08 19:55 ` Jeff King
2021-12-10 18:36 ` Taylor Blau
2021-12-10 22:31 ` Taylor Blau
2021-12-11 1:39 ` Taylor Blau
2021-12-13 14:00 ` Derrick Stolee
2021-12-13 14:31 ` Taylor Blau
2021-12-14 1:55 ` [PATCH v2 0/8] " Taylor Blau
2021-12-14 1:55 ` [PATCH v2 1/8] t5326: demonstrate bitmap corruption after permutation Taylor Blau
2021-12-14 1:55 ` [PATCH v2 2/8] midx.c: make changing the preferred pack safe Taylor Blau
2021-12-14 1:55 ` [PATCH v2 3/8] pack-revindex.c: instrument loading on-disk reverse index Taylor Blau
2021-12-14 1:55 ` [PATCH v2 4/8] t5326: drop unnecessary setup Taylor Blau
2021-12-14 1:55 ` [PATCH v2 5/8] t5326: extract `test_rev_exists` Taylor Blau
2021-12-20 18:33 ` Derrick Stolee
2022-01-04 15:33 ` Taylor Blau
2021-12-14 1:55 ` [PATCH v2 6/8] t5326: move tests to t/lib-bitmap.sh Taylor Blau
2021-12-14 1:55 ` [PATCH v2 7/8] t/lib-bitmap.sh: parameterize tests over reverse index source Taylor Blau
2021-12-14 1:55 ` [PATCH v2 8/8] midx: read `RIDX` chunk when present Taylor Blau
2021-12-20 18:42 ` Derrick Stolee
2022-01-04 15:21 ` Taylor Blau
2021-12-15 19:46 ` [PATCH v2 0/8] midx: prevent bitmap corruption when permuting pack order Junio C Hamano
2021-12-15 21:37 ` Taylor Blau
2021-12-15 22:17 ` Junio C Hamano
2021-12-15 22:55 ` Junio C Hamano
2021-12-20 18:51 ` Derrick Stolee
2021-12-20 19:52 ` Taylor Blau
2021-12-20 20:09 ` Derrick Stolee
2021-12-15 22:58 ` Junio C Hamano
2021-12-15 23:01 ` Taylor Blau
2022-01-04 18:15 ` [PATCH v3 0/9] " Taylor Blau
2022-01-04 18:15 ` [PATCH v3 1/9] t5326: demonstrate bitmap corruption after permutation Taylor Blau
2022-01-20 17:55 ` Jonathan Tan
2022-01-20 22:11 ` Taylor Blau
2022-01-20 22:41 ` Junio C Hamano
2022-01-20 22:46 ` Taylor Blau
2022-01-24 17:40 ` Jonathan Tan
2022-01-04 18:15 ` [PATCH v3 2/9] midx.c: make changing the preferred pack safe Taylor Blau
2022-01-14 21:35 ` Junio C Hamano
2022-01-14 21:43 ` Junio C Hamano
2022-01-15 0:59 ` Taylor Blau
2022-01-15 6:27 ` Junio C Hamano
2022-01-20 18:08 ` Jonathan Tan
2022-01-20 22:13 ` Taylor Blau
2022-01-04 18:15 ` [PATCH v3 3/9] pack-revindex.c: instrument loading on-disk reverse index Taylor Blau
2022-01-20 18:15 ` Jonathan Tan
2022-01-20 22:18 ` Taylor Blau
2022-01-24 17:53 ` Jonathan Tan
2022-01-04 18:15 ` [PATCH v3 4/9] t5326: drop unnecessary setup Taylor Blau
2022-01-04 18:15 ` [PATCH v3 5/9] t5326: extract `test_rev_exists` Taylor Blau
2022-01-04 18:15 ` [PATCH v3 6/9] t5326: move tests to t/lib-bitmap.sh Taylor Blau
2022-01-04 18:15 ` [PATCH v3 7/9] t/lib-bitmap.sh: parameterize tests over reverse index source Taylor Blau
2022-01-24 19:15 ` Jonathan Tan
2022-01-25 21:40 ` Taylor Blau
2022-01-26 21:00 ` Jonathan Tan
2022-01-04 18:16 ` [PATCH v3 8/9] midx: read `RIDX` chunk when present Taylor Blau
2022-01-24 19:27 ` Jonathan Tan
2022-01-25 21:45 ` Taylor Blau
2022-01-26 21:28 ` Jonathan Tan
2022-01-04 18:16 ` Taylor Blau [this message]
2022-01-24 19:29 ` [PATCH v3 9/9] pack-bitmap.c: gracefully fallback after opening pack/MIDX Jonathan Tan
2022-01-25 21:46 ` Taylor Blau
2022-01-25 22:40 ` [PATCH v4 0/9] midx: prevent bitmap corruption when permuting pack order Taylor Blau
2022-01-25 22:41 ` [PATCH v4 1/9] t5326: demonstrate bitmap corruption after permutation Taylor Blau
2022-01-26 15:01 ` Ævar Arnfjörð Bjarmason
2022-01-26 20:18 ` Taylor Blau
2022-01-25 22:41 ` [PATCH v4 2/9] midx.c: make changing the preferred pack safe Taylor Blau
2022-01-25 22:41 ` [PATCH v4 3/9] pack-revindex.c: instrument loading on-disk reverse index Taylor Blau
2022-01-26 15:03 ` Ævar Arnfjörð Bjarmason
2022-01-25 22:41 ` [PATCH v4 4/9] t5326: drop unnecessary setup Taylor Blau
2022-01-25 22:41 ` [PATCH v4 5/9] t5326: extract `test_rev_exists` Taylor Blau
2022-01-26 15:04 ` Ævar Arnfjörð Bjarmason
2022-01-26 20:19 ` Taylor Blau
2022-01-25 22:41 ` [PATCH v4 6/9] t5326: move tests to t/lib-bitmap.sh Taylor Blau
2022-01-25 22:41 ` [PATCH v4 7/9] t/lib-bitmap.sh: parameterize tests over reverse index source Taylor Blau
2022-01-25 22:41 ` [PATCH v4 8/9] midx: read `RIDX` chunk when present Taylor Blau
2022-01-26 15:10 ` Ævar Arnfjörð Bjarmason
2022-01-26 20:23 ` Taylor Blau
2022-01-25 22:41 ` [PATCH v4 9/9] pack-bitmap.c: gracefully fallback after opening pack/MIDX Taylor Blau
2022-01-26 15:08 ` Ævar Arnfjörð Bjarmason
2022-01-26 17:50 ` [PATCH v4 0/9] midx: prevent bitmap corruption when permuting pack order Ævar Arnfjörð Bjarmason
2022-01-26 20:24 ` Taylor Blau
2022-01-27 17:15 ` Jonathan Tan
2022-02-24 22:50 ` Taylor Blau
2022-01-27 14:13 ` Derrick Stolee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9707d5ea4433d9a5c7f8581dbb2d8a05f410efd3.1641320129.git.me@ttaylorr.com \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=stolee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).