Git development
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs
Date: Mon, 11 May 2026 20:46:44 -0400	[thread overview]
Message-ID: <cover.1778546804.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1776124588.git.me@ttaylorr.com>

[Note to the maintainer: this series has been rebased onto the current
tip of master, which is 7760f83b597 (Merge branch
'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].

This is a (very) small reroll of my series to fix several bugs in the
pseudo-merge bitmap implementation. The only changes since last time are:

 - consistently ordering `test_when_finished "rm -fr ..."` above `git
   init` in tests, and

 - avoiding a single Git invocation on the left-hand side of a pipe in
   the final patch

Otherwise, the series is entirely unchanged save for the above and the
rebase.

As usual, a range-diff is included below for convenience. The original
cover letter is as follows:

========================================================================

This series fixes several bugs in the pseudo-merge bitmap implementation
that caused the pseudo-merge application path to be effectively broken
during fill-in traversal.

Peff noticed that this code path was never triggered by the existing
test suite, and investigating that observation uncovered a handful of
bugs, some compounding.

The first two patches introduce test infrastructure: a 'bitmap write'
test helper that gives tests precise control over which commits receive
individual bitmaps, and a set of "test_expect_failure" tests
demonstrating each bug.

The next four patches fix the bugs in the per-commit pseudo-merge
lookup:

  - The pseudo-merge commit lookup table was sorted by OID rather than
    by bit position, causing the reader's binary search to fail.

  - The binary search in pseudo_merge_at() had its lo/hi updates
    swapped.

  - The extended pseudo-merge lookup path had three compounding bugs: a
    wrong entry-size calculation in the writer, a misinterpretation of
    extended table entries in the reader, and a silently-swallowed error
    check.

The final two patches fix issues in pseudo-merge group selection:

  - find_pseudo_merge_group_for_ref() did not parse commits before
    inspecting their dates, so all candidates had date == 0 and were
    unconditionally placed in the "stable" bucket.

  - The config validation for bitmapPseudoMerge.*.sampleRate accepted 0,
    which leads to a division by zero once the date classification is
    fixed and the unstable code path is exercised.

There is also a small fix for a regex leak when the pattern key is
overridden in config.

Thanks in advance for your review!

Taylor Blau (9):
  t/helper: add 'test-tool bitmap write' subcommand
  t5333: demonstrate various pseudo-merge bugs
  pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
  pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
  pack-bitmap: fix pseudo-merge lookup for shared commits
  pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
  pack-bitmap: reject pseudo-merge "sampleRate" of 0
  Documentation: fix broken `sampleRate` in gitpacking(7)
  pack-bitmap: prevent pattern leak on pseudo-merge re-assignment

 Documentation/config/bitmap-pseudo-merge.adoc |   4 +-
 Documentation/gitpacking.adoc                 |   4 +-
 pack-bitmap-write.c                           |  23 +-
 pseudo-merge.c                                |  19 +-
 t/helper/test-bitmap.c                        | 113 ++++++++-
 t/t5310-pack-bitmaps.sh                       |  24 ++
 t/t5333-pseudo-merge-bitmaps.sh               | 229 ++++++++++++++++++
 7 files changed, 402 insertions(+), 14 deletions(-)

Range-diff against v2:
 1:  c0df35f8ebd =  1:  9c7a829cbeb t/helper: add 'test-tool bitmap write' subcommand
 2:  11de3343726 =  2:  d1ed4aadf75 t5333: demonstrate various pseudo-merge bugs
 3:  8d908ab415e =  3:  bf3a9a07e5f pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
 4:  07f70a07c20 !  4:  a1d341c92eb pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
    @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during
     +test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '
      	test_when_finished "rm -fr pseudo-merge-fill-in-multi" &&
      	git init pseudo-merge-fill-in-multi &&
    -+	git init pseudo-merge-fill-in-multi &&
      	(
    - 		cd pseudo-merge-fill-in-multi &&
    - 
 5:  3ed0b39843f =  5:  06e3410d323 pack-bitmap: fix pseudo-merge lookup for shared commits
 6:  95f847211f3 =  6:  78cf7e6d80d pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
 7:  f8a01cfb893 =  7:  4dbf6686718 pack-bitmap: reject pseudo-merge "sampleRate" of 0
 8:  c37156502c0 =  8:  46d0ee2f168 Documentation: fix broken `sampleRate` in gitpacking(7)
 9:  b905fd5d0ae !  9:  9b17dab2cf7 pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
    @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus
      '
      
     +test_expect_success 'duplicate pseudo-merge pattern does not leak' '
    -+	git init pseudo-merge-dup-pattern &&
     +	test_when_finished "rm -fr pseudo-merge-dup-pattern" &&
    -+
    ++	git init pseudo-merge-dup-pattern &&
     +	(
     +		cd pseudo-merge-dup-pattern &&
     +
    @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus
     +		git config bitmapPseudoMerge.test.threshold now &&
     +		git config bitmapPseudoMerge.test.stableThreshold never &&
     +
    -+		git rev-parse HEAD~63 |
    -+		test-tool bitmap write "$(basename $pack)" &&
    ++		git rev-parse HEAD~63 >in &&
    ++		test-tool bitmap write "$(basename $pack)" <in &&
     +
     +		test_pseudo_merges >merges &&
     +		test_line_count = 1 merges

base-commit: 7760f83b59750c27df653c5c46d0f80e44cfe02c
-- 
2.54.0.76.g9b17dab2cf7

  parent reply	other threads:[~2026-05-12  0:46 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-13 23:56 [PATCH 0/8] pack-bitmap: fix various pseudo-merge bugs Taylor Blau
2026-04-13 23:56 ` [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand Taylor Blau
2026-04-14 19:48   ` Junio C Hamano
2026-04-14 21:29     ` Taylor Blau
2026-04-14 21:34       ` Junio C Hamano
2026-04-14 21:40         ` Taylor Blau
2026-04-14 20:08   ` Junio C Hamano
2026-04-14 21:40     ` Taylor Blau
2026-04-19  0:24   ` Elijah Newren
2026-04-21 18:51     ` Taylor Blau
2026-04-13 23:56 ` [PATCH 2/8] t5333: demonstrate various pseudo-merge bugs Taylor Blau
2026-04-19  0:25   ` Elijah Newren
2026-04-13 23:56 ` [PATCH 3/8] pack-bitmap-write: sort pseudo-merge commit lookup table in pack order Taylor Blau
2026-04-13 23:56 ` [PATCH 4/8] pack-bitmap: fix inverted binary search in `pseudo_merge_at()` Taylor Blau
2026-04-13 23:56 ` [PATCH 5/8] pack-bitmap: fix pseudo-merge lookup for shared commits Taylor Blau
2026-04-13 23:56 ` [PATCH 6/8] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()` Taylor Blau
2026-04-13 23:56 ` [PATCH 7/8] pack-bitmap: reject pseudo-merge "sampleRate" of 0 Taylor Blau
2026-04-19  0:26   ` Elijah Newren
2026-04-13 23:57 ` [PATCH 8/8] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment Taylor Blau
2026-04-21 20:01 ` [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs Taylor Blau
2026-04-21 20:01   ` [PATCH v2 1/9] t/helper: add 'test-tool bitmap write' subcommand Taylor Blau
2026-04-21 20:01   ` [PATCH v2 2/9] t5333: demonstrate various pseudo-merge bugs Taylor Blau
2026-04-21 20:02   ` [PATCH v2 3/9] pack-bitmap-write: sort pseudo-merge commit lookup table in pack order Taylor Blau
2026-04-21 20:02   ` [PATCH v2 4/9] pack-bitmap: fix inverted binary search in `pseudo_merge_at()` Taylor Blau
2026-04-21 20:02   ` [PATCH v2 5/9] pack-bitmap: fix pseudo-merge lookup for shared commits Taylor Blau
2026-04-21 20:02   ` [PATCH v2 6/9] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()` Taylor Blau
2026-04-21 20:02   ` [PATCH v2 7/9] pack-bitmap: reject pseudo-merge "sampleRate" of 0 Taylor Blau
2026-04-21 20:02   ` [PATCH v2 8/9] Documentation: fix broken `sampleRate` in gitpacking(7) Taylor Blau
2026-04-21 20:02   ` [PATCH v2 9/9] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment Taylor Blau
2026-04-22  1:37   ` [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs Elijah Newren
2026-05-11  2:53     ` Junio C Hamano
2026-05-12  0:48       ` Taylor Blau
2026-05-12  0:10     ` Taylor Blau
2026-05-12  0:46 ` Taylor Blau [this message]
2026-05-12  0:46   ` [PATCH v3 1/9] t/helper: add 'test-tool bitmap write' subcommand Taylor Blau
2026-05-12  0:46   ` [PATCH v3 2/9] t5333: demonstrate various pseudo-merge bugs Taylor Blau
2026-05-12  0:46   ` [PATCH v3 3/9] pack-bitmap-write: sort pseudo-merge commit lookup table in pack order Taylor Blau
2026-05-12  0:46   ` [PATCH v3 4/9] pack-bitmap: fix inverted binary search in `pseudo_merge_at()` Taylor Blau
2026-05-12  0:47   ` [PATCH v3 5/9] pack-bitmap: fix pseudo-merge lookup for shared commits Taylor Blau
2026-05-12  0:47   ` [PATCH v3 6/9] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()` Taylor Blau
2026-05-12  0:47   ` [PATCH v3 7/9] pack-bitmap: reject pseudo-merge "sampleRate" of 0 Taylor Blau
2026-05-12  0:47   ` [PATCH v3 8/9] Documentation: fix broken `sampleRate` in gitpacking(7) Taylor Blau
2026-05-12  0:47   ` [PATCH v3 9/9] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment Taylor Blau
2026-05-12  1:38   ` [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs Junio C Hamano
2026-05-12  1:46     ` Taylor Blau
2026-05-12  1:49     ` 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=cover.1778546804.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /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