[PATCH 2/3] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 28, 2026, 08:49 UTC
- Message-ID
- <20260128-b4-pks-fix-for-each-ref-in-misuse-v1-2-deccae3ea725@pks.im>
- In-Reply-To
- <20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im>
The "pack.preferBitmapTips" configuration allows the user to specify which references should be preferred when generating bitmaps. This option is typically expected to be set to a reference prefix, like for example "refs/heads/".
It's not unreasonable though for a user to configure one specific reference as preferred. But if they do, they'll hit a `BUG()`:
$ git -c pack.preferBitmapTips=refs/heads/main repack -adb
BUG: ../refs/iterator.c:366: attempt to trim too many characters
error: pack-objects died of signal 6The root cause for this bug is how we enumerate these references. We call `refs_for_each_ref_in()`, which will:
- Yield all references that have a user-specified prefix.
- Trim each of these references so that the prefix is removed.
Typically, this function is called with a trailing slash, like "refs/heads/", and in that case things work alright. But if the function is called with the name of an existing reference then we'll try to trim the full reference name, which would leave us with an empty name. And as this would not really leave us with anything sensible, we call `BUG()` instead of yielding this reference.
One could argue that this is a bug in `refs_for_each_ref_in()`. But the question then becomes what the correct behaviour would be:
- Do we want to skip exact matches? In our case we certainly don't
want that, as the user has asked us to generate a bitmap for it. - Do we want to yield the reference with the empty refname? That would
lead to a somewhat weird result.Neither of these feel like viable options, so calling `BUG()` feels like a sensible way out.
The root cause really is that we try to trim the whole refname. We can thus easily fix the bug itself by calling `refs_for_each_fullref_in()` instead. This function behaves the same as `refs_for_each_ref_in()`, except that it doesn't strip the prefix. Consequently, it correctly yields also exact refnames.
One resulting weirdness is that two refs "refs/heads/base" and "refs/heads/base-something" would now match if the user configured "refs/heads/base" as bitmap tips. One could arguably change the semantics of the configuration such that a string without a trailing slash needs to be an exact reference match, whereas a string with a trailing slash indicates a directory hierarchy. But such a change would potentially cause regressions with dubious benefits, so this issue is ignored for now.
Signed-off-by: Patrick Steinhardt <ps@pks.im> --- pack-bitmap.c | 4 ++-- t/t5310-pack-bitmaps.sh | 35 +++++++++++++++++++++++++++++++++++ t/t5319-multi-pack-index.sh | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c index 2f5cb34009..8d3b5ac037 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -3334,8 +3334,8 @@ void for_each_preferred_bitmap_tip(struct repository *repo, return; for_each_string_list_item(item, preferred_tips) { - refs_for_each_ref_in(get_main_ref_store(repo), - item->string, cb, cb_data); + refs_for_each_fullref_in(get_main_ref_store(repo), + item->string, NULL, cb, cb_data); } } diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 6718fb98c0..7ef91b502c 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -466,6 +466,41 @@ test_bitmap_cases () { ) ' + test_expect_success 'pack.preferBitmapTips can use direct refname' ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + # Create enough commits that not all will receive bitmap + # coverage even if they are all at the tip of some reference. + test_commit_bulk --message="%s" 103 && + git log --format="create refs/tags/%s %H" HEAD >refs && + git update-ref --stdin <refs && + + # Create the bitmap. + git repack -adb && + test-tool bitmap list-commits | sort >commits-with-bitmap && + + # Verify that we have at least one commit that did not + # receive a bitmap. + git rev-list HEAD >commits.raw && + sort <commits.raw >commits && + comm -13 commits-with-bitmap commits >commits-wo-bitmap && + test_file_not_empty commits-wo-bitmap && + commit_id=$(head commits-wo-bitmap) && + + # We now create a reference for this commit and repack + # with "preferBitmapTips" pointing to that exact + # reference. The expectation is that it will now be + # covered by a bitmap. + git update-ref refs/heads/cover-me "$commit_id" && + git -c pack.preferBitmapTips=refs/heads/cover-me repack -adb && + test-tool bitmap list-commits >after && + test_grep "$commit_id" after + ) + ' + test_expect_success 'complains about multiple pack bitmaps' ' rm -fr repo && git init repo && diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index faae98c7e7..40d36118bd 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -1345,4 +1345,40 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' ' ) ' +test_expect_success 'pack.preferBitmapTips can use direct refname' ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + # Create enough commits that not all will receive bitmap + # coverage even if they are all at the tip of some reference. + test_commit_bulk --message="%s" 103 && + git log --format="create refs/tags/%s %H" HEAD >refs && + git update-ref --stdin <refs && + + # Create the bitmap via the MIDX. + git repack -adb --write-midx && + test-tool bitmap list-commits | sort >commits-with-bitmap && + + # Verify that we have at least one commit that did not + # receive a bitmap. + git rev-list HEAD >commits.raw && + sort <commits.raw >commits && + comm -13 commits-with-bitmap commits >commits-wo-bitmap && + test_file_not_empty commits-wo-bitmap && + commit_id=$(head commits-wo-bitmap) && + + # We now create a reference for this commit and repack + # with "preferBitmapTips" pointing to that exact + # reference. The expectation is that it will now be + # covered by a bitmap. + git update-ref refs/heads/cover-me "$commit_id" && + rm .git/objects/pack/multi-pack-index* && + git -c pack.preferBitmapTips=refs/heads/cover-me repack -adb --write-midx && + test-tool bitmap list-commits >after && + test_grep "$commit_id" after + ) +' + test_done
-- 2.53.0.rc2.206.g60c1bca835.dirty