All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"
Date: Thu, 29 Jan 2026 14:34:27 -0500	[thread overview]
Message-ID: <aXu2Q1TgsaUIo30+@nand.local> (raw)
In-Reply-To: <xmqq7bt0cqec.fsf@gitster.g>

On Thu, Jan 29, 2026 at 08:52:43AM -0800, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > 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.
>
> Hmph.  Is this documented?
>
> 	... Goes and looks ...
>
> Yes, it is documented.  We say "This is because ..." but it just
> explains it as what the chosen design of the implementation happens
> to do, without saying for what benefit the implementation was chosen,
> so it is unclear if this is designed behaviour, or more importantly,
> even if this were designed, what the rationale of choosing that
> design was.
>
> "When they are so close to fall into the same chunk, there is no
> point having bitmaps individually for them, as their bitmaps will be
> very similar anyway, so this design saves space without sacrificing
> the quality of the resulting set of bitmaps" or something?

The commits are generally presented in the order they are traversed
(regardless of whether we are generating single- or multi-pack bitmaps).
That makes it likely that commits within the same window are likely to
generate very similar bitmaps, but it is not guaranteed.

When looking at the documentation, I ended up with the following:

--- 8< ---
diff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc
index 75402d5579d..b65cbaaebb4 100644
--- a/Documentation/config/pack.adoc
+++ b/Documentation/config/pack.adoc
@@ -168,7 +168,10 @@ pack.preferBitmapTips::
 Note that setting this configuration to `refs/foo` does not mean that
 the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will
 necessarily be selected. This is because commits are selected for
-bitmaps from within a series of windows of variable length.
+bitmaps from within a series of windows of variable length (in order to
+space bitmaps out throughout history), and we only select one commit per
+window. Thus if multiple preferred commits appear in the same window,
+only one will be selected.
 +
 If a commit at the tip of any reference which is a suffix of any value
 of this configuration is seen in a window, it is immediately given
--- >8 ---

> > 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).
>
> The documentation says "... reference that is a suffix of any value
> of this configuration".  Is "refs/heads/foobar" a "suffix" of
> "refs/heads/foo"?  I actually find this phrasing fairly strange, as
> I do not think of "refs/heads/main" be a "suffix" of "refs/heads/".

I agree, the use of "suffix" is confusing at best. I think if/how we
change this section depends on the outcome of this series, but the
original intent was to say that preferring "refs/heads/foo" would make
the commits at the tips of "refs/heads/foo/bar" and "refs/heads/foo/baz"
preferred, but not "refs/heads/foobar".

Thanks,
Taylor

  reply	other threads:[~2026-01-29 19:34 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 [this message]
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
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=aXu2Q1TgsaUIo30+@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.