Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 29, 2026, 02:31 UTC
Message-ID: <aXrGfGUJQ34JAmuz@nand.local>
In-Reply-To: <20260128-b4-pks-fix-for-each-ref-in-misuse-v1-2-deccae3ea725@pks.im>

On Wed, Jan 28, 2026 at 09:49:21AM +0100, Patrick Steinhardt wrote:

Show 11 quoted lines

> 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 6

Oops. While we should definitely not BUG() here, I am not sure I understand the desired use-case of specifying a single reference as a value for pack.preferBitmapTips.

Looking at the implementation of bitmap_writer_select_commits(), we do not guarantee that *any* reference specified by pack.preferBitmapTips will receive a bitmap. That's because we don't necessarily enumerate the entire set of commits when determining which ones to bitmap.

For example, if I do something like (assuming that the bug described here is fixed):

    $ git -c pack.preferBitmapTips=refs/heads/foo \
          -c pack.preferBitmapTips=refs/heads/bar repack -adb

, and suppose "indexed_commits" list has the commits pointed to by "foo" and "bar" next to each other. We'll look at the next batch of bitmap candidates, realize that commit "foo" has the NEEDS_BITMAP flag set, mark it as chosen, and then skip ahead to the next chunk, all without having looked at "bar".

Looking at the code, I *think* it's the case that specifying a single preferred bitmap tip with an exact reference name will guarantee that we select it for bitmapping, but it's not the case in general.

Show 8 quoted lines

> 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.

(Setting aside the for_each_ref vs. for_each_fullref issue for a moment...)

Am I understanding this change correctly that doing something like -c pack.preferBitmapTips=refs/heads/foo would match both foo and foobar?

If so, I am not sure that that is a desirable interface, especially since we went the opposite direction in 10e8a9352bc (refs.c: stop matching non-directory prefixes in exclude patterns, 2025-03-06). Having the two behave inconsistently from one another feels somewhat awkward to me and may lead to unexpected results.

At the very least, if we do end up going in this direction (and I am not necessarily advocating that we do, since I would prefer a more consistent set of behavior), we should at minimum document it in git-config(1).

Thanks, Taylor