public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"
Date: Sun, 1 Feb 2026 21:13:43 -0500	[thread overview]
Message-ID: <aYAIVw5UMQeP3Ilr@nand.local> (raw)
In-Reply-To: <20260130-b4-pks-fix-for-each-ref-in-misuse-v2-2-0449b198a681@pks.im>

On Fri, Jan 30, 2026 at 02:27:43PM +0100, Patrick Steinhardt wrote:
> [...] There are two possible ways to fix this issue:
>
>   - We can fix the bug by using `refs_for_each_fullref_in()` instead,
>     which does not strip the prefix at all. Consequently, we would now
>     start to accept all references that start with the configured
>     prefix, including exact matches. So if we had "refs/heads/main", we
>     would both match "refs/heads/main" and "refs/heads/main-branch".
>
>   - Or we can fix the bug by appending a slash to the prefix if it
>     doesn't already have one. This would mean that we only match
>     ref hierarchies that start with this prefix.
>
> The first fix leaves the user with strictly _more_ configuration
> options: they can have prefix matches by not appending a slash to the
> configuration, and they can have ref hierarchy matches by appending one.

I would definitely like to err on the side of more flexible
configuration options, but I am still concerned that this change would
lead to somewhat surprising behavior.

A couple of thoughts:

 - Like I mentioned in the earlier round, 10e8a9352bc (refs.c: stop
   matching non-directory prefixes in exclude patterns, 2025-03-06)
   takes the opposite approach as what is being proposed here. I worry
   that users will find the difference in behavior between
   pack.preferBitmapTips and for-each-ref's --exclude patterns to be
   confusing.

 - If a user wants to list all references that start with
   "refs/heads/ma" in the string prefix sense (that is, matching
   "refs/heads/ma", "refs/heads/main", "refs/heads/master" and so on),
   then they would do

     $ git for-each-ref 'refs/heads/ma*'

   , not 'refs/heads/ma'. In fact, enumerating 'refs/heads/ma' when
   there exist references "refs/heads/ma/foo", "refs/heads/ma/bar",
   etc., for-each-ref will output those three references (but only
   "refs/heads/ma" itself if it exists).

I suppose there is an argument to be made that we are dealing with
"patterns" vs. "prefixes" here, but TBH I am not sure that is a
distinction that is well-understood by users (nor should we expect it to
be).

The original intent of this configuration was that "suffix" in this
context meant directory suffix or exact match, not string suffix. The
implementation does not match that intent, but I think there is enough
ambiguity here that I wouldn't consider the change I'm suggesting to be
a breaking one.

Overall, I think interpreting the pack.preferBitmapTips configuration as
a reference pattern gives the user both (a) more flexibility in which
references to match, and (b) does so in a way that is consistent with
10e8a9352bc and the existing behavior of for-each-ref.

Thanks,
Taylor

  reply	other threads:[~2026-02-02  2:13 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-28  8:49 [PATCH 0/3] Fix misuse of `refs_for_each_ref_in()` Patrick Steinhardt
2026-01-28  8:49 ` [PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips Patrick Steinhardt
2026-01-28 10:37   ` Karthik Nayak
2026-01-29  2:16   ` Taylor Blau
2026-01-29 16:35     ` Junio C Hamano
2026-01-30 12:58     ` Patrick Steinhardt
2026-01-28  8:49 ` [PATCH 2/3] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips" Patrick Steinhardt
2026-01-28 11:04   ` Karthik Nayak
2026-01-29  2:31   ` Taylor Blau
2026-01-29 16:52     ` Junio C Hamano
2026-01-29 19:34       ` Taylor Blau
2026-01-29 23:17         ` Junio C Hamano
2026-01-30 12:58     ` Patrick Steinhardt
2026-01-28  8:49 ` [PATCH 3/3] bisect: fix misuse of `refs_for_each_ref_in()` Patrick Steinhardt
2026-01-29  8:14   ` Jeff King
2026-01-29 17:28     ` Junio C Hamano
2026-01-30 12:58       ` Patrick Steinhardt
2026-01-28 11:05 ` [PATCH 0/3] Fix " Karthik Nayak
2026-01-30 13:27 ` [PATCH v2 0/4] " Patrick Steinhardt
2026-01-30 13:27   ` [PATCH v2 1/4] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips Patrick Steinhardt
2026-01-30 13:27   ` [PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips" Patrick Steinhardt
2026-02-02  2:13     ` Taylor Blau [this message]
2026-02-06  7:13       ` Patrick Steinhardt
2026-01-30 13:27   ` [PATCH v2 3/4] bisect: fix misuse of `refs_for_each_ref_in()` Patrick Steinhardt
2026-01-30 13:27   ` [PATCH v2 4/4] bisect: simplify string_list memory handling Patrick Steinhardt
2026-01-30 16:56     ` Junio C Hamano
2026-02-02  2:14       ` Taylor Blau
2026-02-06  7:49 ` [PATCH v3 0/4] Fix misuse of `refs_for_each_ref_in()` Patrick Steinhardt
2026-02-06  7:49   ` [PATCH v3 1/4] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips Patrick Steinhardt
2026-02-06  7:49   ` [PATCH v3 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips" Patrick Steinhardt
2026-02-06 17:18     ` Junio C Hamano
2026-02-06  7:49   ` [PATCH v3 3/4] bisect: fix misuse of `refs_for_each_ref_in()` Patrick Steinhardt
2026-02-06  7:49   ` [PATCH v3 4/4] bisect: simplify string_list memory handling Patrick Steinhardt
2026-02-18 18:36   ` [PATCH v3 0/4] Fix misuse of `refs_for_each_ref_in()` Taylor Blau
2026-02-19 15:22     ` Junio C Hamano
2026-02-19  7:57 ` [PATCH v4 " Patrick Steinhardt
2026-02-19  7:57   ` [PATCH v4 1/4] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips Patrick Steinhardt
2026-02-19  7:57   ` [PATCH v4 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips" Patrick Steinhardt
2026-02-19  7:57   ` [PATCH v4 3/4] bisect: fix misuse of `refs_for_each_ref_in()` Patrick Steinhardt
2026-02-19  7:57   ` [PATCH v4 4/4] bisect: simplify string_list memory handling Patrick Steinhardt
2026-02-26 17:39   ` [PATCH v4 0/4] Fix misuse of `refs_for_each_ref_in()` Junio C Hamano
2026-02-26 17:39   ` Junio C Hamano

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aYAIVw5UMQeP3Ilr@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    /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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox