{"thread":{"id":"65478","subject":"[PATCH 0/8] pack-bitmap: fix various pseudo-merge bugs","startedAt":"2026-04-13T23:56:40Z","lastAt":"2026-05-12T01:49:42Z","messageCount":46,"participants":["Taylor Blau","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"541505","messageId":"cover.1776124588.git.me@ttaylorr.com","threadId":"65478","inReplyTo":null,"subject":"[PATCH 0/8] pack-bitmap: fix various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:37Z","receivedAt":"2026-04-13T23:56:40Z","isPatch":true,"body":"This series fixes several bugs in the pseudo-merge bitmap implementation\nthat caused the pseudo-merge application path to be effectively broken\nduring fill-in traversal.\n\nPeff noticed that this code path was never triggered by the existing\ntest suite, and investigating that observation uncovered a handful of\nbugs, some compounding.\n\nThe first two patches introduce test infrastructure: a 'bitmap write'\ntest helper that gives tests precise control over which commits receive\nindividual bitmaps, and a set of \"test_expect_failure\" tests\ndemonstrating each bug.\n\nThe next four patches fix the bugs in the per-commit pseudo-merge\nlookup:\n\n - The pseudo-merge commit lookup table was sorted by OID rather than\n by bit position, causing the reader's binary search to fail.\n\n - The binary search in pseudo_merge_at() had its lo/hi updates\n swapped.\n\n - The extended pseudo-merge lookup path had three compounding bugs: a\n wrong entry-size calculation in the writer, a misinterpretation of\n extended table entries in the reader, and a silently-swallowed error\n check.\n\nThe final two patches fix issues in pseudo-merge group selection:\n\n - find_pseudo_merge_group_for_ref() did not parse commits before\n inspecting their dates, so all candidates had date == 0 and were\n unconditionally placed in the \"stable\" bucket.\n\n - The config validation for bitmapPseudoMerge.*.sampleRate accepted 0,\n which leads to a division by zero once the date classification is\n fixed and the unstable code path is exercised.\n\nThere is also a small fix for a regex leak when the pattern key is\noverridden in config.\n\nThanks in advance for your review!\n\nTaylor Blau (8):\n t/helper: add 'test-tool bitmap write' subcommand\n t5333: demonstrate various pseudo-merge bugs\n pack-bitmap-write: sort pseudo-merge commit lookup table in pack order\n pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n pack-bitmap: fix pseudo-merge lookup for shared commits\n pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`\n pack-bitmap: reject pseudo-merge \"sampleRate\" of 0\n pack-bitmap: prevent pattern leak on pseudo-merge re-assignment\n\n pack-bitmap-write.c | 23 +++-\n pseudo-merge.c | 19 ++-\n t/helper/test-bitmap.c | 110 ++++++++++++++-\n t/t5310-pack-bitmaps.sh | 24 ++++\n t/t5333-pseudo-merge-bitmaps.sh | 230 ++++++++++++++++++++++++++++++++\n 5 files changed, 396 insertions(+), 10 deletions(-)\n\n\nbase-commit: 8c9303b1ffae5b745d1b0a1f98330cf7944d8db0\n-- \n2.54.0.rc1.73.g8f4e0170952\n"},{"id":"541506","messageId":"d5ef6b959fd7c05c73bd33aa2b394558320aceac.1776124588.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:40Z","receivedAt":"2026-04-13T23:56:42Z","isPatch":true,"body":"In f16eb1c091 (pseudo-merge: fix disk reads from find_pseudo_merge(),\n2026-03-31), we noted that `apply_pseudo_merges_for_commit()` is never\ntriggered by the existing test suite, and that this bears further\ninvestigation.\n\nThis patch is the first one to begin that investigation. The following\npatches will expose and fix a variety of bugs in the implementation of\npseudo-merge bitmaps.\n\nIn order to do so, however, many of these tests require very precise\nselection of which commits receive bitmaps and which do not. To date,\nthere isn't a standard approach to easily facilitate this. Address this\nby introducing a `test-tool bitmap write` subcommand that writes a\nbitmap for a given packfile, reading the set of commits which should\nreceive individual bitmaps from stdin like so:\n\n test-tool bitmap write \" is the filename for a specific packfile (e.g.,\n\"pack-abc123.pack\"), and \"/path/to/commits.list\" is a list of commit\nOIDs which will receive bitmaps.\n\nThe helper respects `bitmapPseudoMerge.*` configuration for creating\npseudo-merge bitmaps alongside the regular commit bitmaps.\n\nSigned-off-by: Taylor Blau \n---\n t/helper/test-bitmap.c | 110 +++++++++++++++++++++++++++++++++++++++-\n t/t5310-pack-bitmaps.sh | 24 +++++++++\n 2 files changed, 133 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c\nindex 16a01669e41..96c0000c787 100644\n--- a/t/helper/test-bitmap.c\n+++ b/t/helper/test-bitmap.c\n@@ -2,7 +2,10 @@\n \n #include \"test-tool.h\"\n #include \"git-compat-util.h\"\n+#include \"hex.h\"\n+#include \"odb.h\"\n #include \"pack-bitmap.h\"\n+#include \"pseudo-merge.h\"\n #include \"setup.h\"\n \n static int bitmap_list_commits(void)\n@@ -35,6 +38,108 @@ static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n \treturn test_bitmap_pseudo_merge_objects(the_repository, n);\n }\n \n+struct bitmap_writer_data {\n+\tstruct packing_data packed;\n+\tstruct pack_idx_entry **index;\n+\tuint32_t nr;\n+};\n+\n+static int add_packed_object(const struct object_id *oid,\n+\t\t\t struct packed_git *pack,\n+\t\t\t uint32_t pos,\n+\t\t\t void *_data)\n+{\n+\tstruct bitmap_writer_data *data = _data;\n+\tstruct object_entry *entry;\n+\tstruct object_info oi = OBJECT_INFO_INIT;\n+\tenum object_type type;\n+\n+\toi.typep = &type;\n+\n+\tentry = packlist_alloc(&data->packed, oid);\n+\tentry->idx.offset = nth_packed_object_offset(pack, pos);\n+\tif (packed_object_info(pack, entry->idx.offset, &oi) < 0)\n+\t\tdie(\"could not get type of object %s\",\n+\t\t oid_to_hex(oid));\n+\toe_set_type(entry, type);\n+\toe_set_in_pack(&data->packed, entry, pack);\n+\tdata->index[data->nr++] = &entry->idx;\n+\n+\treturn 0;\n+}\n+\n+static int idx_oid_cmp(const void *va, const void *vb)\n+{\n+\tconst struct pack_idx_entry *a = *(const struct pack_idx_entry **)va;\n+\tconst struct pack_idx_entry *b = *(const struct pack_idx_entry **)vb;\n+\n+\treturn oidcmp(&a->oid, &b->oid);\n+}\n+\n+static int bitmap_write(const char *basename)\n+{\n+\tstruct packed_git *p = NULL;\n+\tstruct bitmap_writer_data data = { 0 };\n+\tstruct bitmap_writer writer;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tprepare_repo_settings(the_repository);\n+\trepo_for_each_pack(the_repository, p) {\n+\t\tif (!strcmp(pack_basename(p), basename))\n+\t\t\tbreak;\n+\t}\n+\n+\tif (!p)\n+\t\tdie(\"could not find pack '%s'\", basename);\n+\n+\tif (open_pack_index(p))\n+\t\tdie(\"cannot open pack index for '%s'\", p->pack_name);\n+\n+\tprepare_packing_data(the_repository, &data.packed);\n+\tALLOC_ARRAY(data.index, p->num_objects);\n+\n+\tfor_each_object_in_pack(p, add_packed_object, &data,\n+\t\t\t\tODB_FOR_EACH_OBJECT_PACK_ORDER);\n+\n+\tbitmap_writer_init(&writer, the_repository, &data.packed, NULL);\n+\tbitmap_writer_build_type_index(&writer, data.index);\n+\n+\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n+\t\tstruct object_id oid;\n+\t\tstruct commit *c;\n+\n+\t\tif (get_oid_hex(buf.buf, &oid))\n+\t\t\tdie(\"invalid OID: %s\", buf.buf);\n+\n+\t\tc = lookup_commit(the_repository, &oid);\n+\t\tif (!c || repo_parse_commit(the_repository, c))\n+\t\t\tdie(\"could not parse commit %s\", buf.buf);\n+\n+\t\tbitmap_writer_push_commit(&writer, c, false);\n+\t}\n+\n+\tselect_pseudo_merges(&writer);\n+\tif (bitmap_writer_build(&writer) < 0)\n+\t\tdie(\"failed to build bitmaps\");\n+\n+\tbitmap_writer_set_checksum(&writer, p->hash);\n+\n+\tQSORT(data.index, p->num_objects, idx_oid_cmp);\n+\n+\tstrbuf_reset(&buf);\n+\tstrbuf_addstr(&buf, p->pack_name);\n+\tstrbuf_strip_suffix(&buf, \".pack\");\n+\tstrbuf_addstr(&buf, \".bitmap\");\n+\tbitmap_writer_finish(&writer, data.index, buf.buf, 0);\n+\n+\tbitmap_writer_free(&writer);\n+\tstrbuf_release(&buf);\n+\tfree(data.index);\n+\tclear_packing_data(&data.packed);\n+\n+\treturn 0;\n+}\n+\n int cmd__bitmap(int argc, const char **argv)\n {\n \tsetup_git_directory();\n@@ -51,13 +156,16 @@ int cmd__bitmap(int argc, const char **argv)\n \t\treturn bitmap_dump_pseudo_merge_commits(atoi(argv[2]));\n \tif (argc == 3 && !strcmp(argv[1], \"dump-pseudo-merge-objects\"))\n \t\treturn bitmap_dump_pseudo_merge_objects(atoi(argv[2]));\n+\tif (argc == 3 && !strcmp(argv[1], \"write\"))\n+\t\treturn bitmap_write(argv[2]);\n \n \tusage(\"\\ttest-tool bitmap list-commits\\n\"\n \t \"\\ttest-tool bitmap list-commits-with-offset\\n\"\n \t \"\\ttest-tool bitmap dump-hashes\\n\"\n \t \"\\ttest-tool bitmap dump-pseudo-merges\\n\"\n \t \"\\ttest-tool bitmap dump-pseudo-merge-commits \\n\"\n-\t \"\\ttest-tool bitmap dump-pseudo-merge-objects \");\n+\t \"\\ttest-tool bitmap dump-pseudo-merge-objects \\n\"\n+\t \"\\ttest-tool bitmap write < \");\n \n \treturn -1;\n }\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex f693cb56691..9489e59fa55 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -648,4 +648,28 @@ test_expect_success 'truncated bitmap fails gracefully (lookup table)' '\n \ttest_grep corrupted.bitmap.index stderr\n '\n \n+test_expect_success 'test-tool bitmap write' '\n+\tgit init bitmap-write-helper &&\n+\ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n+\t(\n+\t\tcd bitmap-write-helper &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n+\n+\t\tgit rev-parse HEAD >commits &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" actual &&\n+\t\tsort commits >expect &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.54.0.rc1.73.g8f4e0170952\n\n"},{"id":"541507","messageId":"f4899b668e229069a10d7fc627835dbdc12d7b39.1776124588.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 2/8] t5333: demonstrate various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:43Z","receivedAt":"2026-04-13T23:56:45Z","isPatch":true,"body":"Using the test helper introduced via the previous commit, add various\nfailing tests demonstrating bugs in the pseudo-merge implementation.\n\nThese are all marked as failing with one exception. The \"sampleRate=0\"\ntest describes a latent bug, which is only reachable through a code path\nthat is itself masked by a separate bug. A future commit will fix that\nbug, and, in turn, cause the aforementioned test to fail. Accordingly,\nthat commit will mark the test as failing, and it will be re-marked as\npassing in a separate commit which fixes the once-latent bug.\n\nFor the rest: the following commits will explain and fix the underlying\nbugs in detail.\n\nSigned-off-by: Taylor Blau \n---\n t/t5333-pseudo-merge-bitmaps.sh | 198 ++++++++++++++++++++++++++++++++\n 1 file changed, 198 insertions(+)\n\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 1f7a5d82ee4..20e77ab4390 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -462,4 +462,202 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n \t)\n '\n \n+test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n+\tgit init pseudo-merge-fill-in-traversal &&\n+\ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n+\t(\n+\t\tcd pseudo-merge-fill-in-traversal &&\n+\n+\t\tgit config bitmapPseudoMerge.test.pattern refs/tags/ &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 1 in &&\n+\t\t\twhile read oid\n+\t\t\tdo\n+\t\t\t\techo \"create refs/group-$side/$oid $oid\" || return 1\n+\t\t\tdone in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 2 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 2 in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 2 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 2 in &&\n+\t\tGIT_TEST_DATE_NOW=$test_tick \\\n+\t\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges\n+\t)\n+'\n+\n+test_expect_success 'sampleRate=0 does not cause division by zero' '\n+\tgit init pseudo-merge-sample-rate-zero &&\n+\ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n+\t(\n+\t\tcd pseudo-merge-sample-rate-zero &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n+\n+\t\tgit config bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.sampleRate 0 &&\n+\t\tgit config bitmapPseudoMerge.test.threshold now &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" \n---\n pack-bitmap-write.c | 21 ++++++++++++++++++++-\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 8338d7217ef..86ed6a5d78c 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -819,6 +819,20 @@ static void write_selected_commits_v1(struct bitmap_writer *writer,\n \t}\n }\n \n+static int pseudo_merge_commit_pos_cmp(const void *_va, const void *_vb,\n+\t\t\t\t void *_data)\n+{\n+\tstruct bitmap_writer *writer = _data;\n+\tuint32_t pos_a = find_object_pos(writer, _va, NULL);\n+\tuint32_t pos_b = find_object_pos(writer, _vb, NULL);\n+\n+\tif (pos_a < pos_b)\n+\t\treturn -1;\n+\tif (pos_a > pos_b)\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n static void write_pseudo_merges(struct bitmap_writer *writer,\n \t\t\t\tstruct hashfile *f)\n {\n@@ -876,7 +890,12 @@ static void write_pseudo_merges(struct bitmap_writer *writer,\n \t\toid_array_append(&commits, &kh_key(writer->pseudo_merge_commits, i));\n \t}\n \n-\toid_array_sort(&commits);\n+\t/*\n+\t * Sort the commits by their bit position so that the lookup\n+\t * table can be binary searched by the reader (see\n+\t * find_pseudo_merge()).\n+\t */\n+\tQSORT_S(commits.oid, commits.nr, pseudo_merge_commit_pos_cmp, writer);\n \n \t/* write lookup table (non-extended) */\n \tfor (i = 0; i < commits.nr; i++) {\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 20e77ab4390..dce43ed8dc6 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -462,7 +462,7 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n+test_expect_success 'apply pseudo-merges during fill-in traversal' '\n \tgit init pseudo-merge-fill-in-traversal &&\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n \t(\n-- \n2.54.0.rc1.73.g8f4e0170952\n\n"},{"id":"541509","messageId":"af9f651269d7898ba18410500c69fb30446940e2.1776124589.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 4/8] pack-bitmap: fix inverted binary search in `pseudo_merge_at()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:49Z","receivedAt":"2026-04-13T23:56:51Z","isPatch":true,"body":"The binary search in `pseudo_merge_at()` has its \"lo\" and \"hi\" updates\nswapped: when the midpoint's offset is less than the target, it sets `hi\n= mi` (searching left) instead of `lo = mi + 1` (searching right), and\nvice versa.\n\nThis means that lookups for pseudo-merges whose offset is not near the\nmidpoint of the pseudo-merge table are likely to fail.\n\nIn practice, with a single pseudo-merge group this is masked because the\nlone entry is always at the midpoint. With multiple groups, the inverted\ncomparisons cause lookups to search in the wrong direction, potentially\nmissing entries.\n\nSwap the \"lo\" and \"hi\" assignments to search in the correct direction,\nmaking it possible to apply pseudo-merges during fill-in when more than\none pseudo-merge exists in a group.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex ff18b6c3642..fb71c761792 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -559,9 +559,9 @@ static struct pseudo_merge *pseudo_merge_at(const struct pseudo_merge_map *pm,\n \t\tif (got == want)\n \t\t\treturn use_pseudo_merge(pm, &pm->v[mi]);\n \t\telse if (got < want)\n-\t\t\thi = mi;\n-\t\telse\n \t\t\tlo = mi + 1;\n+\t\telse\n+\t\t\thi = mi;\n \t}\n \n \twarning(_(\"could not find pseudo-merge for commit %s at offset %\"PRIuMAX),\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex dce43ed8dc6..5bfb5103124 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -496,7 +496,7 @@ test_expect_success 'apply pseudo-merges during fill-in traversal' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '\n+test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \tgit init pseudo-merge-fill-in-multi &&\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n \t(\n-- \n2.54.0.rc1.73.g8f4e0170952\n\n"},{"id":"541510","messageId":"01f1d6f08c6487c9103cf87e222668b664b30d83.1776124589.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 5/8] pack-bitmap: fix pseudo-merge lookup for shared commits","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:52Z","receivedAt":"2026-04-13T23:56:54Z","isPatch":true,"body":"When a commit appears in more than one pseudo-merge group, its entry in\nthe commit lookup table has the high bit set in its offset field,\nindicating that the offset points to an \"extended\" table containing the\nset of pseudo-merges for that commit.\n\nThere are three bugs in this path:\n\n * The `next_ext` offset in `write_pseudo_merges()` undercounts the\n per-entry size of the lookup table (8 vs. 12 bytes).\n\n * `nth_pseudo_merge_ext()` calls `read_pseudo_merge_commit_at()` on a\n pseudo-merge bitmap offset, misinterpreting it as a 12-byte commit\n table entry.\n\n * The error check after `pseudo_merge_ext_at()` in\n `apply_pseudo_merges_for_commit()` tests `< -1` instead of `< 0`,\n silently swallowing errors from `error()`.\n\nThe first bug is on the write side: each commit lookup entry contains a\n4- and 8-byte unsigned value for a total of 12 bytes, but the\ncalculation assumes that the entry only contains 8 bytes of data. This\nmakes `next_ext` too small, so the extended-table offsets that get\nwritten point into the middle of the non-extended lookup table rather\nthan past it. The reader then interprets non-extended lookup data as\nextended entries, producing garbage.\n\nThe second bug is on the read side and is independently fatal: even with\na correctly positioned extended table, `nth_pseudo_merge_ext()` feeds\nthe offset it reads (which points at pseudo-merge bitmap data) to\n`read_pseudo_merge_commit_at()`. That function tries to parse 12 bytes\nas a `pseudo_merge_commit` struct, clobbering `merge->pseudo_merge_ofs`\nwith whatever happens to be at that location. The caller only needs\n`pseudo_merge_ofs`, so the fix is to store the offset directly rather\nthan re-parsing a commit table entry. The `commit_pos` field is left\nuntouched, retaining the value that `find_pseudo_merge()` set earlier.\n\nThe third bug is latent. With the first two fixes applied, the extended\ntable is correctly written and read, so `pseudo_merge_ext_at()` does not\nfail during normal operation. The `< -1` vs `< 0` distinction only\nmatters when the bitmap file is corrupt or truncated, in which case the\nerror would be silently ignored and the code would proceed with\nuninitialized data.\n\nSigned-off-by: Taylor Blau \n---\n pack-bitmap-write.c | 2 +-\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 86ed6a5d78c..1c8070f99c0 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -877,7 +877,7 @@ static void write_pseudo_merges(struct bitmap_writer *writer,\n \n \tnext_ext = st_add(hashfile_total(f),\n \t\t\t st_mult(kh_size(writer->pseudo_merge_commits),\n-\t\t\t\t sizeof(uint64_t)));\n+\t\t\t\t sizeof(uint32_t) + sizeof(uint64_t)));\n \n \ttable_start = hashfile_total(f);\n \ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex fb71c761792..34e1da00b4e 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -600,7 +600,7 @@ static int nth_pseudo_merge_ext(const struct pseudo_merge_map *pm,\n \t\treturn error(_(\"out-of-bounds read: (%\"PRIuMAX\" >= %\"PRIuMAX\")\"),\n \t\t\t (uintmax_t)ofs, (uintmax_t)pm->map_size);\n \n-\tread_pseudo_merge_commit_at(merge, pm->map + ofs);\n+\tmerge->pseudo_merge_ofs = ofs;\n \n \treturn 0;\n }\n@@ -671,7 +671,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\toff_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63);\n \t\tuint32_t i;\n \n-\t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < -1) {\n+\t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < 0) {\n \t\t\twarning(_(\"could not read extended pseudo-merge table \"\n \t\t\t\t \"for commit %s\"),\n \t\t\t\toid_to_hex(&commit->object.oid));\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 5bfb5103124..8844a3bced9 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -549,7 +549,7 @@ test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges with overlapping groups during fill-in' '\n+test_expect_success 'apply pseudo-merges with overlapping groups during fill-in' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-overlap\" &&\n \tgit init pseudo-merge-fill-in-overlap &&\n \t(\n-- \n2.54.0.rc1.73.g8f4e0170952\n\n"},{"id":"541511","messageId":"6d74c0a177a8d910aa0394add38f5d57573eebd9.1776124589.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 6/8] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:55Z","receivedAt":"2026-04-13T23:56:57Z","isPatch":true,"body":"`find_pseudo_merge_group_for_ref()` uses the commit's date to classify\nit as either \"stable\" (older than the stable threshold) or \"unstable\"\n(otherwise).\n\nHowever, to find the relevant commit from a given OID, the function\n`find_pseudo_merge_group_for_ref()` uses `lookup_commit()` which does\nnot parse commits.\n\nBecause an unparsed commit has its \"date\" set to zero, every candidate\nis placed in the \"stable\" bucket regardless of its actual committer\ntimestamp. This means the `bitmapPseudoMerge.*.threshold` and\n`stableThreshold` configuration options have no effect: the\nstable/unstable split is always determined by comparing against zero\nrather than the real commit date.\n\nThe net result is that pseudo-merge groups are partitioned by\n`stableSize` instead of the intended decay-based sizing, and the\n`sampleRate` knob (which only applies to the unstable path) is never\nexercised.\n\nFix this by calling `repo_parse_commit()` after `lookup_commit()`,\nbailing out of the callback if parsing fails.\n\nThe corresponding test configures two pseudo-merge groups that both\nmatch all tags. The \"stable\" group uses `threshold=1.month.ago`, and the\n\"all\" group uses `threshold=now`. The test use our custom\n\"GIT_TEST_DATE_NOW\" environment variable by setting it to the value of\n\"$test_tick\" to align Git's notion of \"now\" (and therefore\n\"1.month.ago\") with the `test_tick` timestamps, so the commits appear to\nbe younger than one month: only the \"all\" group matches them, producing\nexactly one pseudo-merge.\n\nWithout the fix every commit has `date == 0`, which satisfies `date <=\nthreshold` for both groups (since 0 is older than one month ago), and\nthe \"stable\" group erroneously matches as well.\n\nNow that commits are correctly classified as \"unstable\", the bug\ndescribed in the test exercising the \"sampleRate=0\" test is reachable,\nand the test is marked as failing. It will be fixed in a following\ncommit.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 2 ++\n t/t5333-pseudo-merge-bitmaps.sh | 22 ++++++++++++----------\n 2 files changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex 34e1da00b4e..d79e5fb649a 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -236,6 +236,8 @@ static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_d\n \tc = lookup_commit(the_repository, maybe_peeled);\n \tif (!c)\n \t\treturn 0;\n+\tif (repo_parse_commit(the_repository, c))\n+\t\treturn 0;\n \tif (!packlist_find(writer->to_pack, maybe_peeled))\n \t\treturn 0;\n \ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 8844a3bced9..63d2f64361d 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -592,32 +592,34 @@ test_expect_success 'apply pseudo-merges with overlapping groups during fill-in'\n \t)\n '\n \n-test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n+test_expect_success 'pseudo-merge commits are correctly classified by date' '\n \tgit init pseudo-merge-date-classification &&\n \ttest_when_finished \"rm -fr pseudo-merge-date-classification\" &&\n \t(\n \t\tcd pseudo-merge-date-classification &&\n \n \t\ttest_commit_bulk 64 &&\n+\n \t\ttag_everything &&\n \t\tgit repack -ad &&\n \n \t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n \n \t\t# Configure two pseudo-merge groups: one that only\n-\t\t# matches \"stable\" refs (older than one month), and one\n-\t\t# that matches all refs. With 64 freshly-created tags\n-\t\t# (all younger than one month) the stable group should\n-\t\t# have zero pseudo-merges and the catch-all group should\n-\t\t# have one.\n+\t\t# matches \"stable\" refs (older than one month), and\n+\t\t# one that matches all refs. With 64 tags whose\n+\t\t# commits are all younger than one month, the\n+\t\t# \"stable\" group should have zero pseudo-merges and\n+\t\t# the \"all\" group should have one.\n \t\t#\n \t\t# Use GIT_TEST_DATE_NOW to align \"now\" (and therefore\n \t\t# \"1.month.ago\") with the test_tick timestamps so that\n \t\t# the commits are within the last month.\n \t\t#\n-\t\t# This exercises the date-based classification in\n-\t\t# find_pseudo_merge_group_for_ref(), which requires\n-\t\t# that commits are parsed before inspecting their date.\n+\t\t# Without parsing the commit, its date field would\n+\t\t# be zero, causing it to satisfy date <= threshold\n+\t\t# for the \"stable\" group as well, and both groups\n+\t\t# would produce pseudo-merges.\n \t\tgit config bitmapPseudoMerge.stable.pattern \"refs/tags/\" &&\n \t\tgit config bitmapPseudoMerge.stable.maxMerges 64 &&\n \t\tgit config bitmapPseudoMerge.stable.stableThreshold never &&\n@@ -637,7 +639,7 @@ test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n \t)\n '\n \n-test_expect_success 'sampleRate=0 does not cause division by zero' '\n+test_expect_failure 'sampleRate=0 does not cause division by zero' '\n \tgit init pseudo-merge-sample-rate-zero &&\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n \t(\n-- \n2.54.0.rc1.73.g8f4e0170952\n\n"},{"id":"541512","messageId":"1b0f7295c21bf6240bef975e5f3fb9da685f29d3.1776124589.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 7/8] pack-bitmap: reject pseudo-merge \"sampleRate\" of 0","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:56:58Z","receivedAt":"2026-04-13T23:57:00Z","isPatch":true,"body":"The \"bitmapPseudoMerge.*.sampleRate\" configuration controls what\nfraction of unstable commits are included in each pseudo-merge group.\nThe config validation accepts values in the range `[0, 1]`, but a value\nof exactly 0 causes a division by zero in `select_pseudo_merges_1()`:\n\n if (j % (uint32_t)(1.0 / group->sample_rate))\n\nWhen `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting\ninfinity to `uint32_t` is undefined behavior in C. On most platforms\nthis yields 0, making the subsequent modulo operation (`j % 0`) a\nfatal arithmetic trap.\n\nThis path was not previously reachable because an earlier bug caused\nall pseudo-merge candidates to be classified as \"stable\" (where the\nsampling rate is not used), regardless of their actual commit date. Now\nthat the date classification is fixed, the unstable path is exercised\nand the division by zero can fire.\n\nFix this by changing the validation to require a strict lower bound and\nthus reject 0.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex d79e5fb649a..75bed043602 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -169,8 +169,8 @@ static int pseudo_merge_config(const char *var, const char *value,\n \t\t}\n \t} else if (!strcmp(key, \"samplerate\")) {\n \t\tgroup->sample_rate = git_config_double(var, value, ctx->kvi);\n-\t\tif (!(0 <= group->sample_rate && group->sample_rate <= 1)) {\n-\t\t\twarning(_(\"%s must be between 0 and 1, using default\"), var);\n+\t\tif (!(0 < group->sample_rate && group->sample_rate <= 1)) {\n+\t\t\twarning(_(\"%s must be between 0 (exclusive) and 1, using default\"), var);\n \t\t\tgroup->sample_rate = DEFAULT_PSEUDO_MERGE_SAMPLE_RATE;\n \t\t}\n \t} else if (!strcmp(key, \"threshold\")) {\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 63d2f64361d..46e8e6a8ea1 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -639,7 +639,7 @@ test_expect_success 'pseudo-merge commits are correctly classified by date' '\n \t)\n '\n \n-test_expect_failure 'sampleRate=0 does not cause division by zero' '\n+test_expect_success 'sampleRate=0 does not cause division by zero' '\n \tgit init pseudo-merge-sample-rate-zero &&\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n \t(\n-- \n2.54.0.rc1.73.g8f4e0170952\n\n"},{"id":"541513","messageId":"8f4e017095210afb79e547832f50bc8fb51017bc.1776124589.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH 8/8] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-13T23:57:01Z","receivedAt":"2026-04-13T23:57:03Z","isPatch":true,"body":"When \"bitmapPseudoMerge.*.pattern\" appears more than once for the same\ngroup, `pseudo_merge_config()` frees the old `regex_t *` pointer\nbut does not call `regfree()` on it first. This leaks whatever internal\nstate `regcomp()` allocated.\n\nThe final cleanup path in `pseudo_merge_group_release()` does call\n`regfree()` before `free()`, so only the intermediate replacement is\naffected.\n\nFix this by guarding the replacement with a NULL check and calling\n`regfree()` before `free()` when the pointer is non-NULL.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 5 ++++-\n t/t5333-pseudo-merge-bitmaps.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex 75bed043602..22b8600d689 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -150,7 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,\n \tif (!strcmp(key, \"pattern\")) {\n \t\tstruct strbuf re = STRBUF_INIT;\n \n-\t\tfree(group->pattern);\n+\t\tif (group->pattern) {\n+\t\t\tregfree(group->pattern);\n+\t\t\tfree(group->pattern);\n+\t\t}\n \t\tif (*value != '^')\n \t\t\tstrbuf_addch(&re, '^');\n \t\tstrbuf_addstr(&re, value);\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 46e8e6a8ea1..34d432ce76d 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -662,4 +662,34 @@ test_expect_success 'sampleRate=0 does not cause division by zero' '\n \t)\n '\n \n+test_expect_success 'duplicate pseudo-merge pattern does not leak' '\n+\tgit init pseudo-merge-dup-pattern &&\n+\ttest_when_finished \"rm -fr pseudo-merge-dup-pattern\" &&\n+\n+\t(\n+\t\tcd pseudo-merge-dup-pattern &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\n+\t\t# Set the same group'\\''s pattern twice. The second\n+\t\t# assignment should cleanly release the compiled regex\n+\t\t# from the first without leaking.\n+\t\tgit config bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config --add bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.threshold now &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\tgit rev-parse HEAD~63 |\n+\t\ttest-tool bitmap write \"$(basename $pack)\" &&\n+\n+\t\ttest_pseudo_merges >merges &&\n+\t\ttest_line_count = 1 merges\n+\t)\n+'\n+\n test_done\n-- \n2.54.0.rc1.73.g8f4e0170952\n"},{"id":"541580","messageId":"xmqqik9t9vby.fsf@gitster.g","threadId":"65478","inReplyTo":"d5ef6b959fd7c05c73bd33aa2b394558320aceac.1776124588.git.me@ttaylorr.com","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-14T19:48:49Z","receivedAt":"2026-04-14T19:48:51Z","isPatch":true,"body":"Taylor Blau writes:\n\n> In f16eb1c091 (pseudo-merge: fix disk reads from find_pseudo_merge(),\n> 2026-03-31), we noted that `apply_pseudo_merges_for_commit()` is never\n> triggered by the existing test suite, and that this bears further\n> investigation.\n>\n> This patch is the first one to begin that investigation. The following\n> patches will expose and fix a variety of bugs in the implementation of\n> pseudo-merge bitmaps.\n>\n> In order to do so, however, many of these tests require very precise\n> selection of which commits receive bitmaps and which do not. To date,\n> there isn't a standard approach to easily facilitate this. Address this\n> by introducing a `test-tool bitmap write` subcommand that writes a\n> bitmap for a given packfile, reading the set of commits which should\n> receive individual bitmaps from stdin like so:\n>\n> test-tool bitmap write \n> , where \"\" is the filename for a specific packfile (e.g.,\n> \"pack-abc123.pack\"), and \"/path/to/commits.list\" is a list of commit\n> OIDs which will receive bitmaps.\n>\n> The helper respects `bitmapPseudoMerge.*` configuration for creating\n> pseudo-merge bitmaps alongside the regular commit bitmaps.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> t/helper/test-bitmap.c | 110 +++++++++++++++++++++++++++++++++++++++-\n> t/t5310-pack-bitmaps.sh | 24 +++++++++\n> 2 files changed, 133 insertions(+), 1 deletion(-)\n\nI haven't been paying attention at all to pseudo-merge stuff, so my\ncomment may be too trivial and/or misses the point, but please bear\nwith me.\n\n> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\n> index f693cb56691..9489e59fa55 100755\n> --- a/t/t5310-pack-bitmaps.sh\n> +++ b/t/t5310-pack-bitmaps.sh\n> @@ -648,4 +648,28 @@ test_expect_success 'truncated bitmap fails gracefully (lookup table)' '\n> \ttest_grep corrupted.bitmap.index stderr\n> '\n> \n> +test_expect_success 'test-tool bitmap write' '\n\nIt is very unclear what aspect of \"test-tool bitmap write\" is being\ntested to me. Let me think aloud to see if I can convey my\npuzzlement.\n\n> +\tgit init bitmap-write-helper &&\n> +\ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n\nA tangent but the above two lines may want to be swapped. \"rm -fr\"\ndoes not fail when bitmap-write-helper directory does not yet exist,\nso \"prepare to clear anytime it fails from now on and then create\"\nwould be a safer order than \"create, and prepare to clear anytime it\nfails from now on\".\n\n> +\t(\n> +\t\tcd bitmap-write-helper &&\n> +\n> +\t\ttest_commit_bulk 64 &&\n> +\t\tgit repack -ad &&\n> +\n> +\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n\nSo we bulk-created 64 commits, repacked into one, and then\n\n> +\t\tgit rev-parse HEAD >commits &&\n\nwrote the tip-commit in \"commits\".\n\n> +\t\ttest-tool bitmap write \"$(basename $pack)\" +\t\ttest-tool bitmap list-commits | sort >actual &&\n> +\t\tsort commits >expect &&\n> +\t\ttest_cmp expect actual &&\n\nAnd compare the list of bitmapped commits with the singleton HEAD\n(it is puzzling to sort a single element list, though).\n\n> +\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n> +\t\tgit rev-list --count --objects HEAD >expect &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> test_done\n\nIf we look at the implementation of bitmap_write() below, the object\nname for HEAD is fed to bitmap_write_push_commit() with pseudo bit\noff. After reading the list (which has only one element), we call\nselect_pseudo_merges(). What do we expect to happen in this call?\nSince no bitmap_write_push_commit() call is made with pseudo bit on,\nwe will return immediately without doing anything?\n\nAfter that bitmap_write_build() is called, and as this is expected\nto add bitmaps to the commits fed to bitmap_write_push_commit(), we\nare expecting to see bitmap given to HEAD (and nothing else)?\n\nI said it is unclear what is being tested. Putting it another way,\nwhat could go wrong to cause this test to fail? We give commit A,\nB, and C to \"bitmap write\", and it somehow chooses other commits to\nalso give bitmap, which will be reported by \"bitmap list-commits\"\nand we detect that as a failure?\n\nThanks.\n"},{"id":"541583","messageId":"xmqqeckh9uew.fsf@gitster.g","threadId":"65478","inReplyTo":"d5ef6b959fd7c05c73bd33aa2b394558320aceac.1776124588.git.me@ttaylorr.com","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-14T20:08:39Z","receivedAt":"2026-04-14T20:08:42Z","isPatch":true,"body":"Taylor Blau writes:\n\n> +struct bitmap_writer_data {\n> +\tstruct packing_data packed;\n> +\tstruct pack_idx_entry **index;\n> +\tuint32_t nr;\n> +};\n> +\n> +static int add_packed_object(const struct object_id *oid,\n> +\t\t\t struct packed_git *pack,\n> +\t\t\t uint32_t pos,\n> +\t\t\t void *_data)\n> +{\n> +\tstruct bitmap_writer_data *data = _data;\n> +\tstruct object_entry *entry;\n> +\tstruct object_info oi = OBJECT_INFO_INIT;\n> +\tenum object_type type;\n> +\n> +\toi.typep = &type;\n> +\n> +\tentry = packlist_alloc(&data->packed, oid);\n\ndata->packed is \"packing data\" that has a pointer \"objects\" that is\na flat array of \"struct object_entry\". This array is dynamically\nexpanded with realloc() in packlist_alloc(), and it returns a\npointer into this data->packed->objects[] array. We receive it in a\nlocal variable \"entry\" here.\n\n> +\tentry->idx.offset = nth_packed_object_offset(pack, pos);\n> +\tif (packed_object_info(pack, entry->idx.offset, &oi) < 0)\n> +\t\tdie(\"could not get type of object %s\",\n> +\t\t oid_to_hex(oid));\n> +\toe_set_type(entry, type);\n> +\toe_set_in_pack(&data->packed, entry, pack);\n\nAnd populate the entry.\n\n> +\tdata->index[data->nr++] = &entry->idx;\n\nAnd then store the pointer to one of the members (actually the first\nmember) in that \"struct object_entry\" instance in that data->packed->objects[]\narray we took from.\n\nWhat happens when a repeated call to this function to add many\nobjects (those contained within the pack we are iterating over)\ncaused the packlist_alloc() to realloc data->packed->objects[] array\neventually? Wouldn't it invalidate the address of &entry->idx we\nare taking from before the realloc() happens?\n\nI must be missing something?\n\n"},{"id":"541592","messageId":"ad6xn6KmP3TsdpcH@nand.local","threadId":"65478","inReplyTo":"xmqqik9t9vby.fsf@gitster.g","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-14T21:29:03Z","receivedAt":"2026-04-14T21:29:08Z","isPatch":true,"body":"On Tue, Apr 14, 2026 at 12:48:49PM -0700, Junio C Hamano wrote:\n> I haven't been paying attention at all to pseudo-merge stuff, so my\n> comment may be too trivial and/or misses the point, but please bear\n> with me.\n\nIt has been a while since I have thought about pseudo-merges since I\nreturned to the topic with this series, so bear with me just the same\n;-).\n\n> > +\tgit init bitmap-write-helper &&\n> > +\ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n>\n> A tangent but the above two lines may want to be swapped. \"rm -fr\"\n> does not fail when bitmap-write-helper directory does not yet exist,\n> so \"prepare to clear anytime it fails from now on and then create\"\n> would be a safer order than \"create, and prepare to clear anytime it\n> fails from now on\".\n\nI agree. I vaguely remember discussing this on the list with Ævar years\nago, but clearly we should register our cleanup handler before we do the\nthing that needs to be cleaned up in case it fails part of the way\nthrough.\n\n> So we bulk-created 64 commits, repacked into one, and then\n> [...]\n> wrote the tip-commit in \"commits\".\n> [...]\n> And told the new tool to write a bitmap file, with only that HEAD\n> commit and nothing else to get bitmap.\n>\n> > +\t\ttest-tool bitmap list-commits | sort >actual &&\n> > +\t\tsort commits >expect &&\n> > +\t\ttest_cmp expect actual &&\n>\n> And compare the list of bitmapped commits with the singleton HEAD\n> (it is puzzling to sort a single element list, though).\n\nThat's right.\n\n> > +\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n> > +\t\tgit rev-list --count --objects HEAD >expect &&\n> > +\t\ttest_cmp expect actual\n> > +\t)\n> > +'\n> > +\n> > test_done\n>\n> If we look at the implementation of bitmap_write() below, the object\n> name for HEAD is fed to bitmap_write_push_commit() with pseudo bit\n> off. After reading the list (which has only one element), we call\n> select_pseudo_merges(). What do we expect to happen in this call?\n> Since no bitmap_write_push_commit() call is made with pseudo bit on,\n> we will return immediately without doing anything?\n\nIn this particular instance, nothing should happen, and this test\nisn't directly testing the pseudo-merge selection at all here. We don't\nexpect any pseudo-merges to be generated here, only for us to limit the\nselection of which commits get bitmaps.\n\n> After that bitmap_write_build() is called, and as this is expected\n> to add bitmaps to the commits fed to bitmap_write_push_commit(), we\n> are expecting to see bitmap given to HEAD (and nothing else)?\n\nYes.\n\n> I said it is unclear what is being tested. Putting it another way,\n> what could go wrong to cause this test to fail? We give commit A,\n> B, and C to \"bitmap write\", and it somehow chooses other commits to\n> also give bitmap, which will be reported by \"bitmap list-commits\"\n> and we detect that as a failure?\n\nThat's right, and to the point of your original question, I think a\nbetter name is warranted here, perhaps: \"test-tool bitmap write limits\nbitmap selection\" or something.\n\nTo the question of what it's testing, it's testing only its basic\nfunctionality of altering the selection of which commits receive\nbitmaps. In that sense I view it as a smoke test of that basic\nfunctionality more than anything else.\n\nHow about:\n\n--- 8< ---\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 9489e59fa55..ea94735fd66 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -649,8 +649,8 @@ test_expect_success 'truncated bitmap fails gracefully (lookup table)' '\n '\n\n test_expect_success 'test-tool bitmap write' '\n-\tgit init bitmap-write-helper &&\n \ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n+\tgit init bitmap-write-helper &&\n \t(\n \t\tcd bitmap-write-helper &&\n\n@@ -659,12 +659,12 @@ test_expect_success 'test-tool bitmap write' '\n\n \t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n\n-\t\tgit rev-parse HEAD >commits &&\n-\t\ttest-tool bitmap write \"$(basename $pack)\" in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" actual &&\n-\t\tsort commits >expect &&\n-\t\ttest_cmp expect actual &&\n+\t\ttest-tool bitmap list-commits >bitmaps.raw &&\n+\t\tsort bitmaps.raw >bitmaps &&\n+\t\ttest_cmp in bitmaps &&\n\n \t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n \t\tgit rev-list --count --objects HEAD >expect &&\n--- >8 ---\n\nThanks,\nTaylor\n"},{"id":"541593","messageId":"xmqqqzoh8bvq.fsf@gitster.g","threadId":"65478","inReplyTo":"ad6xn6KmP3TsdpcH@nand.local","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-14T21:34:17Z","receivedAt":"2026-04-14T21:34:19Z","isPatch":true,"body":"Taylor Blau writes:\n\n> That's right, and to the point of your original question, I think a\n> better name is warranted here, perhaps: \"test-tool bitmap write limits\n> bitmap selection\" or something.\n\nPerhaps. Or \"limits\" -> \"forces\"? Neither verb exactly conveys\nthat the outcome must be exactly the same as the input specifies,\nnothing added, nothing removed, so I dunno.\n\n> To the question of what it's testing, it's testing only its basic\n> functionality of altering the selection of which commits receive\n> bitmaps. In that sense I view it as a smoke test of that basic\n> functionality more than anything else.\n\nOK.\n\nThanks.\n"},{"id":"541594","messageId":"ad60PJ/pM/wG3krQ@nand.local","threadId":"65478","inReplyTo":"xmqqeckh9uew.fsf@gitster.g","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-14T21:40:12Z","receivedAt":"2026-04-14T21:40:14Z","isPatch":true,"body":"On Tue, Apr 14, 2026 at 01:08:39PM -0700, Junio C Hamano wrote:\n> What happens when a repeated call to this function to add many\n> objects (those contained within the pack we are iterating over)\n> caused the packlist_alloc() to realloc data->packed->objects[] array\n> eventually? Wouldn't it invalidate the address of &entry->idx we\n> are taking from before the realloc() happens?\n>\n> I must be missing something?\n\nGood catch, I'm the one that is missing something here, not you. This is\ndefinitely a use-after-realloc(), though in practice it won't bite us\nbecause we are likely extending into an over-sized heap allocation\nwithout actually moving the data.\n\nI don't know why I thought we allocated the packlist with a fixed size\nequal to p->num_objects ahead of time, but we don't, and this is clearly\na bug.\n\nWill fix, and thanks again for spotting.\n\nThanks,\nTaylor\n"},{"id":"541595","messageId":"ad60X98/sbp9ck49@nand.local","threadId":"65478","inReplyTo":"xmqqqzoh8bvq.fsf@gitster.g","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-14T21:40:47Z","receivedAt":"2026-04-14T21:40:49Z","isPatch":true,"body":"On Tue, Apr 14, 2026 at 02:34:17PM -0700, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > That's right, and to the point of your original question, I think a\n> > better name is warranted here, perhaps: \"test-tool bitmap write limits\n> > bitmap selection\" or something.\n>\n> Perhaps. Or \"limits\" -> \"forces\"? Neither verb exactly conveys\n> that the outcome must be exactly the same as the input specifies,\n> nothing added, nothing removed, so I dunno.\n\nHow about \"determines\" or \"controls\"?\n\nThanks,\nTaylor\n"},{"id":"541875","messageId":"CABPp-BELG+poD67JCojze=bzYsWr0UvdXb2Vai=eEY=2CzaGCg@mail.gmail.com","threadId":"65478","inReplyTo":"d5ef6b959fd7c05c73bd33aa2b394558320aceac.1776124588.git.me@ttaylorr.com","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-04-19T00:24:04Z","receivedAt":"2026-04-19T00:24:17Z","isPatch":true,"body":"On Mon, Apr 13, 2026 at 4:56 PM Taylor Blau wrote:\n[...]\n> + bitmap_writer_push_commit(&writer, c, false);\n\n$ git grep -h -A 1 bitmap_writer_push_commit -- '*.h'\nvoid bitmap_writer_push_commit(struct bitmap_writer *writer,\n struct commit *commit, unsigned pseudo_merge);\n\nNot a big deal, but for consistency, would it make more sense to pass\n0 for the third argument, or to change the function signature change\nto accept bool instead of unsigned?\n"},{"id":"541876","messageId":"CABPp-BFFWpeHUemuDiJjXEhqyJ=amSOsEdrLFtYBrMWg3LpAmg@mail.gmail.com","threadId":"65478","inReplyTo":"f4899b668e229069a10d7fc627835dbdc12d7b39.1776124588.git.me@ttaylorr.com","subject":"Re: [PATCH 2/8] t5333: demonstrate various pseudo-merge bugs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-04-19T00:25:55Z","receivedAt":"2026-04-19T00:26:09Z","isPatch":true,"body":"On Mon, Apr 13, 2026 at 4:56 PM Taylor Blau wrote:\n>\n> Using the test helper introduced via the previous commit, add various\n> failing tests demonstrating bugs in the pseudo-merge implementation.\n>\n> These are all marked as failing with one exception. The \"sampleRate=0\"\n> test describes a latent bug, which is only reachable through a code path\n> that is itself masked by a separate bug. A future commit will fix that\n> bug, and, in turn, cause the aforementioned test to fail. Accordingly,\n> that commit will mark the test as failing, and it will be re-marked as\n> passing in a separate commit which fixes the once-latent bug.\n>\n> For the rest: the following commits will explain and fix the underlying\n> bugs in detail.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> t/t5333-pseudo-merge-bitmaps.sh | 198 ++++++++++++++++++++++++++++++++\n> 1 file changed, 198 insertions(+)\n>\n> diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\n> index 1f7a5d82ee4..20e77ab4390 100755\n> --- a/t/t5333-pseudo-merge-bitmaps.sh\n> +++ b/t/t5333-pseudo-merge-bitmaps.sh\n> @@ -462,4 +462,202 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n> )\n> '\n>\n> +test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n> + git init pseudo-merge-fill-in-traversal &&\n> + test_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n\nAs suggested in the first patch, test_when_finished before the git\ninit. (Same issue occurs later in this file as well.)\n\n[...]\n> + : >trace2.txt &&\n\nThe `: >trace2.txt` struck me as odd, since this file doesn't even\nexist yet in this test...but thinking more, is this just defensive in\ncase someone adds inserts or modifies a previous test which writes to\ntrace2.txt? I like that idea; somehow hadn't seen it before.\n\n> + GIT_TRACE2_EVENT=$PWD/trace2.txt \\\n> + git rev-list --count --objects --use-bitmap-index HEAD >actual &&\n\nI thought this was broken without quoting $PWD, but looks like I\nforgot shell quoting rules again. Assignments don't undergo word\nsplitting, globbing, or brace expansion. So, nothing to see here\neither.\n\n> + test_pseudo_merges_satisfied 1 +\n> + test_cmp expect actual\n> + )\n> +'\n\nDidn't spot anything different to comment on for the rest of the\npatch, so the only substantive comment I had was on the\ntest_when_finished and init ordering.\n"},{"id":"541877","messageId":"CABPp-BH2Zsf03DT8MOtCn=3Sn=Tz_7MF1VwosPjVLdAGo42OCA@mail.gmail.com","threadId":"65478","inReplyTo":"1b0f7295c21bf6240bef975e5f3fb9da685f29d3.1776124589.git.me@ttaylorr.com","subject":"Re: [PATCH 7/8] pack-bitmap: reject pseudo-merge \"sampleRate\" of 0","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-04-19T00:26:08Z","receivedAt":"2026-04-19T00:26:21Z","isPatch":true,"body":"On Mon, Apr 13, 2026 at 4:56 PM Taylor Blau wrote:\n>\n> The \"bitmapPseudoMerge.*.sampleRate\" configuration controls what\n> fraction of unstable commits are included in each pseudo-merge group.\n> The config validation accepts values in the range `[0, 1]`, but a value\n> of exactly 0 causes a division by zero in `select_pseudo_merges_1()`:\n>\n> if (j % (uint32_t)(1.0 / group->sample_rate))\n>\n> When `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting\n> infinity to `uint32_t` is undefined behavior in C. On most platforms\n> this yields 0, making the subsequent modulo operation (`j % 0`) a\n> fatal arithmetic trap.\n>\n> This path was not previously reachable because an earlier bug caused\n> all pseudo-merge candidates to be classified as \"stable\" (where the\n> sampling rate is not used), regardless of their actual commit date. Now\n> that the date classification is fixed, the unstable path is exercised\n> and the division by zero can fire.\n>\n> Fix this by changing the validation to require a strict lower bound and\n> thus reject 0.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> pseudo-merge.c | 4 ++--\n> t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n> 2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/pseudo-merge.c b/pseudo-merge.c\n> index d79e5fb649a..75bed043602 100644\n> --- a/pseudo-merge.c\n> +++ b/pseudo-merge.c\n> @@ -169,8 +169,8 @@ static int pseudo_merge_config(const char *var, const char *value,\n> }\n> } else if (!strcmp(key, \"samplerate\")) {\n> group->sample_rate = git_config_double(var, value, ctx->kvi);\n> - if (!(0 <= group->sample_rate && group->sample_rate <= 1)) {\n> - warning(_(\"%s must be between 0 and 1, using default\"), var);\n> + if (!(0 < group->sample_rate && group->sample_rate <= 1)) {\n> + warning(_(\"%s must be between 0 (exclusive) and 1, using default\"), var);\n\nThe documentation for `bitmapPseudoMerge..sampleRate` in\nDocumentation/config/bitmap-pseudo-merge.adoc still claims that 0 is\nallowed; should that be fixed as part of this patch?\n\nAlso:\n\n```\n$ git grep -B 4 -i samplerate.*=\nDocumentation/gitpacking.adoc-[bitmapPseudoMerge \"all\"]\nDocumentation/gitpacking.adoc- pattern = \"refs/\"\nDocumentation/gitpacking.adoc- threshold = now\nDocumentation/gitpacking.adoc- stableThreshold = never\nDocumentation/gitpacking.adoc: sampleRate = 100\n--\nDocumentation/gitpacking.adoc-[bitmapPseudoMerge \"all\"]\nDocumentation/gitpacking.adoc- pattern = \"refs/virtual/([0-9]+)/(heads|tags)/\"\nDocumentation/gitpacking.adoc- threshold = now\nDocumentation/gitpacking.adoc- stableThreshold = never\nDocumentation/gitpacking.adoc: sampleRate = 100\n```\n\nShould those sampleRates be fixed?\n"},{"id":"542051","messageId":"aefHRByiRJotSIEB@nand.local","threadId":"65478","inReplyTo":"CABPp-BELG+poD67JCojze=bzYsWr0UvdXb2Vai=eEY=2CzaGCg@mail.gmail.com","subject":"Re: [PATCH 1/8] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T18:51:48Z","receivedAt":"2026-04-21T18:51:50Z","isPatch":true,"body":"On Sat, Apr 18, 2026 at 05:24:04PM -0700, Elijah Newren wrote:\n> On Mon, Apr 13, 2026 at 4:56 PM Taylor Blau wrote:\n> [...]\n> > + bitmap_writer_push_commit(&writer, c, false);\n>\n> $ git grep -h -A 1 bitmap_writer_push_commit -- '*.h'\n> void bitmap_writer_push_commit(struct bitmap_writer *writer,\n> struct commit *commit, unsigned pseudo_merge);\n>\n> Not a big deal, but for consistency, would it make more sense to pass\n> 0 for the third argument, or to change the function signature change\n> to accept bool instead of unsigned?\n\nLet's change it to pass \"0\" for now. I think that it's fine to clean\nthis up in the future, but I do not want to make a habit of changing\nmany \"int/unsigned -> bool\" signatures each time we wish to call such a\nfunction.\n\nThanks,\nTaylor\n"},{"id":"542057","messageId":"cover.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:01:52Z","receivedAt":"2026-04-21T20:01:55Z","isPatch":true,"body":"[Note to the maintainer: this series has been rebased onto the current\ntip of master, which is 94f057755b7 (Git 2.54, 2026-04-19) at the time\nof writing.]\n\nThis is a small reroll of my series to fix several bugs in the\npseudo-merge bitmap implementation. The main changes since last time\nare:\n\n - Fixed a use-after-realloc bug in the test helper introduced in the\n first commit.\n\n - Swapped the order of initializing and cleaning up repositories in the\n new test scripts.\n\n - Updated bitmapPseudoMerge..sampleRate's documentation to\n describe the range as (0,1], and added a new commit fixing a broken\n example in gitpacking(7).\n\nAs usual, a range-diff is included below for convenience. The original\ncover letter is as follows:\n\n========================================================================\n\nThis series fixes several bugs in the pseudo-merge bitmap implementation\nthat caused the pseudo-merge application path to be effectively broken\nduring fill-in traversal.\n\nPeff noticed that this code path was never triggered by the existing\ntest suite, and investigating that observation uncovered a handful of\nbugs, some compounding.\n\nThe first two patches introduce test infrastructure: a 'bitmap write'\ntest helper that gives tests precise control over which commits receive\nindividual bitmaps, and a set of \"test_expect_failure\" tests\ndemonstrating each bug.\n\nThe next four patches fix the bugs in the per-commit pseudo-merge\nlookup:\n\n - The pseudo-merge commit lookup table was sorted by OID rather than\n by bit position, causing the reader's binary search to fail.\n\n - The binary search in pseudo_merge_at() had its lo/hi updates\n swapped.\n\n - The extended pseudo-merge lookup path had three compounding bugs: a\n wrong entry-size calculation in the writer, a misinterpretation of\n extended table entries in the reader, and a silently-swallowed error\n check.\n\nThe final two patches fix issues in pseudo-merge group selection:\n\n - find_pseudo_merge_group_for_ref() did not parse commits before\n inspecting their dates, so all candidates had date == 0 and were\n unconditionally placed in the \"stable\" bucket.\n\n - The config validation for bitmapPseudoMerge.*.sampleRate accepted 0,\n which leads to a division by zero once the date classification is\n fixed and the unstable code path is exercised.\n\nThere is also a small fix for a regex leak when the pattern key is\noverridden in config.\n\nThanks in advance for your review!\n\nTaylor Blau (9):\n t/helper: add 'test-tool bitmap write' subcommand\n t5333: demonstrate various pseudo-merge bugs\n pack-bitmap-write: sort pseudo-merge commit lookup table in pack order\n pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n pack-bitmap: fix pseudo-merge lookup for shared commits\n pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`\n pack-bitmap: reject pseudo-merge \"sampleRate\" of 0\n Documentation: fix broken `sampleRate` in gitpacking(7)\n pack-bitmap: prevent pattern leak on pseudo-merge re-assignment\n\n Documentation/config/bitmap-pseudo-merge.adoc | 4 +-\n Documentation/gitpacking.adoc | 4 +-\n pack-bitmap-write.c | 23 +-\n pseudo-merge.c | 19 +-\n t/helper/test-bitmap.c | 113 ++++++++-\n t/t5310-pack-bitmaps.sh | 24 ++\n t/t5333-pseudo-merge-bitmaps.sh | 231 ++++++++++++++++++\n 7 files changed, 404 insertions(+), 14 deletions(-)\n\nRange-diff against v1:\n 1: d5ef6b959fd ! 1: c0df35f8ebd t/helper: add 'test-tool bitmap write' subcommand\n @@ t/helper/test-bitmap.c: static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n \treturn test_bitmap_pseudo_merge_objects(the_repository, n);\n }\n \n -+struct bitmap_writer_data {\n -+\tstruct packing_data packed;\n -+\tstruct pack_idx_entry **index;\n -+\tuint32_t nr;\n -+};\n -+\n +static int add_packed_object(const struct object_id *oid,\n +\t\t\t struct packed_git *pack,\n +\t\t\t uint32_t pos,\n +\t\t\t void *_data)\n +{\n -+\tstruct bitmap_writer_data *data = _data;\n ++\tstruct packing_data *packed = _data;\n +\tstruct object_entry *entry;\n +\tstruct object_info oi = OBJECT_INFO_INIT;\n +\tenum object_type type;\n +\n +\toi.typep = &type;\n +\n -+\tentry = packlist_alloc(&data->packed, oid);\n ++\tentry = packlist_alloc(packed, oid);\n +\tentry->idx.offset = nth_packed_object_offset(pack, pos);\n +\tif (packed_object_info(pack, entry->idx.offset, &oi) < 0)\n +\t\tdie(\"could not get type of object %s\",\n +\t\t oid_to_hex(oid));\n +\toe_set_type(entry, type);\n -+\toe_set_in_pack(&data->packed, entry, pack);\n -+\tdata->index[data->nr++] = &entry->idx;\n ++\toe_set_in_pack(packed, entry, pack);\n +\n +\treturn 0;\n +}\n @@ t/helper/test-bitmap.c: static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n +static int bitmap_write(const char *basename)\n +{\n +\tstruct packed_git *p = NULL;\n -+\tstruct bitmap_writer_data data = { 0 };\n ++\tstruct packing_data packed = { 0 };\n +\tstruct bitmap_writer writer;\n ++\tstruct pack_idx_entry **index;\n +\tstruct strbuf buf = STRBUF_INIT;\n ++\tuint32_t i;\n +\n +\tprepare_repo_settings(the_repository);\n +\trepo_for_each_pack(the_repository, p) {\n @@ t/helper/test-bitmap.c: static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n +\tif (open_pack_index(p))\n +\t\tdie(\"cannot open pack index for '%s'\", p->pack_name);\n +\n -+\tprepare_packing_data(the_repository, &data.packed);\n -+\tALLOC_ARRAY(data.index, p->num_objects);\n ++\tprepare_packing_data(the_repository, &packed);\n +\n -+\tfor_each_object_in_pack(p, add_packed_object, &data,\n ++\tfor_each_object_in_pack(p, add_packed_object, &packed,\n +\t\t\t\tODB_FOR_EACH_OBJECT_PACK_ORDER);\n +\n -+\tbitmap_writer_init(&writer, the_repository, &data.packed, NULL);\n -+\tbitmap_writer_build_type_index(&writer, data.index);\n ++\t/*\n ++\t * Build the index array now that data.packed.objects[] is\n ++\t * fully allocated (packlist_alloc() may have reallocated it\n ++\t * during the loop above).\n ++\t */\n ++\tALLOC_ARRAY(index, p->num_objects);\n ++\tfor (i = 0; i < p->num_objects; i++)\n ++\t\tindex[i] = &packed.objects[i].idx;\n ++\n ++\tbitmap_writer_init(&writer, the_repository, &packed, NULL);\n ++\tbitmap_writer_build_type_index(&writer, index);\n +\n +\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n +\t\tstruct object_id oid;\n @@ t/helper/test-bitmap.c: static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n +\t\tif (!c || repo_parse_commit(the_repository, c))\n +\t\t\tdie(\"could not parse commit %s\", buf.buf);\n +\n -+\t\tbitmap_writer_push_commit(&writer, c, false);\n ++\t\tbitmap_writer_push_commit(&writer, c, 0);\n +\t}\n +\n +\tselect_pseudo_merges(&writer);\n @@ t/helper/test-bitmap.c: static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n +\n +\tbitmap_writer_set_checksum(&writer, p->hash);\n +\n -+\tQSORT(data.index, p->num_objects, idx_oid_cmp);\n ++\tQSORT(index, p->num_objects, idx_oid_cmp);\n +\n +\tstrbuf_reset(&buf);\n +\tstrbuf_addstr(&buf, p->pack_name);\n +\tstrbuf_strip_suffix(&buf, \".pack\");\n +\tstrbuf_addstr(&buf, \".bitmap\");\n -+\tbitmap_writer_finish(&writer, data.index, buf.buf, 0);\n ++\tbitmap_writer_finish(&writer, index, buf.buf, 0);\n +\n +\tbitmap_writer_free(&writer);\n +\tstrbuf_release(&buf);\n -+\tfree(data.index);\n -+\tclear_packing_data(&data.packed);\n ++\tfree(index);\n ++\tclear_packing_data(&packed);\n +\n +\treturn 0;\n +}\n @@ t/t5310-pack-bitmaps.sh: test_expect_success 'truncated bitmap fails gracefully\n \ttest_grep corrupted.bitmap.index stderr\n '\n \n -+test_expect_success 'test-tool bitmap write' '\n -+\tgit init bitmap-write-helper &&\n ++test_expect_success 'test-tool bitmap write determines bitmap selection' '\n +\ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n ++\tgit init bitmap-write-helper &&\n +\t(\n +\t\tcd bitmap-write-helper &&\n +\n @@ t/t5310-pack-bitmaps.sh: test_expect_success 'truncated bitmap fails gracefully\n +\n +\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n +\n -+\t\tgit rev-parse HEAD >commits &&\n -+\t\ttest-tool bitmap write \"$(basename $pack)\" in &&\n ++\t\ttest-tool bitmap write \"$(basename $pack)\" actual &&\n -+\t\tsort commits >expect &&\n -+\t\ttest_cmp expect actual &&\n ++\t\ttest-tool bitmap list-commits >bitmaps.raw &&\n ++\t\tsort bitmaps.raw >bitmaps &&\n ++\t\ttest_cmp in bitmaps &&\n +\n +\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n +\t\tgit rev-list --count --objects HEAD >expect &&\n 2: f4899b668e2 ! 2: 11de3343726 t5333: demonstrate various pseudo-merge bugs\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'use pseudo-merge in bounda\n '\n \n +test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n -+\tgit init pseudo-merge-fill-in-traversal &&\n +\ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n ++\tgit init pseudo-merge-fill-in-traversal &&\n +\t(\n +\t\tcd pseudo-merge-fill-in-traversal &&\n +\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'use pseudo-merge in bounda\n +'\n +\n +test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '\n -+\tgit init pseudo-merge-fill-in-multi &&\n +\ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n ++\tgit init pseudo-merge-fill-in-multi &&\n +\t(\n +\t\tcd pseudo-merge-fill-in-multi &&\n +\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'use pseudo-merge in bounda\n +'\n +\n +test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n -+\tgit init pseudo-merge-date-classification &&\n +\ttest_when_finished \"rm -fr pseudo-merge-date-classification\" &&\n ++\tgit init pseudo-merge-date-classification &&\n +\t(\n +\t\tcd pseudo-merge-date-classification &&\n +\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'use pseudo-merge in bounda\n +'\n +\n +test_expect_success 'sampleRate=0 does not cause division by zero' '\n -+\tgit init pseudo-merge-sample-rate-zero &&\n +\ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n ++\tgit init pseudo-merge-sample-rate-zero &&\n +\t(\n +\t\tcd pseudo-merge-sample-rate-zero &&\n +\n 3: 1f5835e8c62 ! 3: 8d908ab415e pack-bitmap-write: sort pseudo-merge commit lookup table in pack order\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'use pseudo-merge in bounda\n \n -test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n +test_expect_success 'apply pseudo-merges during fill-in traversal' '\n - \tgit init pseudo-merge-fill-in-traversal &&\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n + \tgit init pseudo-merge-fill-in-traversal &&\n \t(\n 4: af9f651269d ! 4: 07f70a07c20 pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during\n \n -test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '\n +test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n - \tgit init pseudo-merge-fill-in-multi &&\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n + \tgit init pseudo-merge-fill-in-multi &&\n ++\tgit init pseudo-merge-fill-in-multi &&\n \t(\n + \t\tcd pseudo-merge-fill-in-multi &&\n + \n 5: 01f1d6f08c6 = 5: 3ed0b39843f pack-bitmap: fix pseudo-merge lookup for shared commits\n 6: 6d74c0a177a ! 6: 95f847211f3 pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges with o\n \n -test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n +test_expect_success 'pseudo-merge commits are correctly classified by date' '\n - \tgit init pseudo-merge-date-classification &&\n \ttest_when_finished \"rm -fr pseudo-merge-date-classification\" &&\n + \tgit init pseudo-merge-date-classification &&\n \t(\n \t\tcd pseudo-merge-date-classification &&\n \n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_failure 'pseudo-merge commits are c\n \n -test_expect_success 'sampleRate=0 does not cause division by zero' '\n +test_expect_failure 'sampleRate=0 does not cause division by zero' '\n - \tgit init pseudo-merge-sample-rate-zero &&\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n + \tgit init pseudo-merge-sample-rate-zero &&\n \t(\n 7: 1b0f7295c21 ! 7: f8a01cfb893 pack-bitmap: reject pseudo-merge \"sampleRate\" of 0\n @@ Commit message\n \n Signed-off-by: Taylor Blau \n \n + ## Documentation/config/bitmap-pseudo-merge.adoc ##\n +@@ Documentation/config/bitmap-pseudo-merge.adoc: will be updated more often than a reference pointing at an old commit.\n + bitmapPseudoMerge..sampleRate::\n + \tDetermines the proportion of non-bitmapped commits (among\n + \treference tips) which are selected for inclusion in an\n +-\tunstable pseudo-merge bitmap. Must be between `0` and `1`\n +-\t(inclusive). The default is `1`.\n ++\tunstable pseudo-merge bitmap. Must be greater than `0` and\n ++\tless than or equal to `1`. The default is `1`.\n + \n + bitmapPseudoMerge..threshold::\n + \tDetermines the minimum age of non-bitmapped commits (among\n +\n ## pseudo-merge.c ##\n @@ pseudo-merge.c: static int pseudo_merge_config(const char *var, const char *value,\n \t\t}\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'pseudo-merge commits are c\n \n -test_expect_failure 'sampleRate=0 does not cause division by zero' '\n +test_expect_success 'sampleRate=0 does not cause division by zero' '\n - \tgit init pseudo-merge-sample-rate-zero &&\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n + \tgit init pseudo-merge-sample-rate-zero &&\n \t(\n -: ----------- > 8: c37156502c0 Documentation: fix broken `sampleRate` in gitpacking(7)\n 8: 8f4e0170952 = 9: b905fd5d0ae pack-bitmap: prevent pattern leak on pseudo-merge re-assignment\n\nbase-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0\n-- \n2.54.0.9.gb905fd5d0ae\n"},{"id":"542058","messageId":"c0df35f8ebd910e7844ea0ce0c9de62dfe18d423.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 1/9] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:01:55Z","receivedAt":"2026-04-21T20:01:58Z","isPatch":true,"body":"In f16eb1c091 (pseudo-merge: fix disk reads from find_pseudo_merge(),\n2026-03-31), we noted that `apply_pseudo_merges_for_commit()` is never\ntriggered by the existing test suite, and that this bears further\ninvestigation.\n\nThis patch is the first one to begin that investigation. The following\npatches will expose and fix a variety of bugs in the implementation of\npseudo-merge bitmaps.\n\nIn order to do so, however, many of these tests require very precise\nselection of which commits receive bitmaps and which do not. To date,\nthere isn't a standard approach to easily facilitate this. Address this\nby introducing a `test-tool bitmap write` subcommand that writes a\nbitmap for a given packfile, reading the set of commits which should\nreceive individual bitmaps from stdin like so:\n\n test-tool bitmap write \" is the filename for a specific packfile (e.g.,\n\"pack-abc123.pack\"), and \"/path/to/commits.list\" is a list of commit\nOIDs which will receive bitmaps.\n\nThe helper respects `bitmapPseudoMerge.*` configuration for creating\npseudo-merge bitmaps alongside the regular commit bitmaps.\n\nSigned-off-by: Taylor Blau \n---\n t/helper/test-bitmap.c | 113 +++++++++++++++++++++++++++++++++++++++-\n t/t5310-pack-bitmaps.sh | 24 +++++++++\n 2 files changed, 136 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c\nindex 16a01669e41..381e9b58b2c 100644\n--- a/t/helper/test-bitmap.c\n+++ b/t/helper/test-bitmap.c\n@@ -2,7 +2,10 @@\n \n #include \"test-tool.h\"\n #include \"git-compat-util.h\"\n+#include \"hex.h\"\n+#include \"odb.h\"\n #include \"pack-bitmap.h\"\n+#include \"pseudo-merge.h\"\n #include \"setup.h\"\n \n static int bitmap_list_commits(void)\n@@ -35,6 +38,111 @@ static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n \treturn test_bitmap_pseudo_merge_objects(the_repository, n);\n }\n \n+static int add_packed_object(const struct object_id *oid,\n+\t\t\t struct packed_git *pack,\n+\t\t\t uint32_t pos,\n+\t\t\t void *_data)\n+{\n+\tstruct packing_data *packed = _data;\n+\tstruct object_entry *entry;\n+\tstruct object_info oi = OBJECT_INFO_INIT;\n+\tenum object_type type;\n+\n+\toi.typep = &type;\n+\n+\tentry = packlist_alloc(packed, oid);\n+\tentry->idx.offset = nth_packed_object_offset(pack, pos);\n+\tif (packed_object_info(pack, entry->idx.offset, &oi) < 0)\n+\t\tdie(\"could not get type of object %s\",\n+\t\t oid_to_hex(oid));\n+\toe_set_type(entry, type);\n+\toe_set_in_pack(packed, entry, pack);\n+\n+\treturn 0;\n+}\n+\n+static int idx_oid_cmp(const void *va, const void *vb)\n+{\n+\tconst struct pack_idx_entry *a = *(const struct pack_idx_entry **)va;\n+\tconst struct pack_idx_entry *b = *(const struct pack_idx_entry **)vb;\n+\n+\treturn oidcmp(&a->oid, &b->oid);\n+}\n+\n+static int bitmap_write(const char *basename)\n+{\n+\tstruct packed_git *p = NULL;\n+\tstruct packing_data packed = { 0 };\n+\tstruct bitmap_writer writer;\n+\tstruct pack_idx_entry **index;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tuint32_t i;\n+\n+\tprepare_repo_settings(the_repository);\n+\trepo_for_each_pack(the_repository, p) {\n+\t\tif (!strcmp(pack_basename(p), basename))\n+\t\t\tbreak;\n+\t}\n+\n+\tif (!p)\n+\t\tdie(\"could not find pack '%s'\", basename);\n+\n+\tif (open_pack_index(p))\n+\t\tdie(\"cannot open pack index for '%s'\", p->pack_name);\n+\n+\tprepare_packing_data(the_repository, &packed);\n+\n+\tfor_each_object_in_pack(p, add_packed_object, &packed,\n+\t\t\t\tODB_FOR_EACH_OBJECT_PACK_ORDER);\n+\n+\t/*\n+\t * Build the index array now that data.packed.objects[] is\n+\t * fully allocated (packlist_alloc() may have reallocated it\n+\t * during the loop above).\n+\t */\n+\tALLOC_ARRAY(index, p->num_objects);\n+\tfor (i = 0; i < p->num_objects; i++)\n+\t\tindex[i] = &packed.objects[i].idx;\n+\n+\tbitmap_writer_init(&writer, the_repository, &packed, NULL);\n+\tbitmap_writer_build_type_index(&writer, index);\n+\n+\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n+\t\tstruct object_id oid;\n+\t\tstruct commit *c;\n+\n+\t\tif (get_oid_hex(buf.buf, &oid))\n+\t\t\tdie(\"invalid OID: %s\", buf.buf);\n+\n+\t\tc = lookup_commit(the_repository, &oid);\n+\t\tif (!c || repo_parse_commit(the_repository, c))\n+\t\t\tdie(\"could not parse commit %s\", buf.buf);\n+\n+\t\tbitmap_writer_push_commit(&writer, c, 0);\n+\t}\n+\n+\tselect_pseudo_merges(&writer);\n+\tif (bitmap_writer_build(&writer) < 0)\n+\t\tdie(\"failed to build bitmaps\");\n+\n+\tbitmap_writer_set_checksum(&writer, p->hash);\n+\n+\tQSORT(index, p->num_objects, idx_oid_cmp);\n+\n+\tstrbuf_reset(&buf);\n+\tstrbuf_addstr(&buf, p->pack_name);\n+\tstrbuf_strip_suffix(&buf, \".pack\");\n+\tstrbuf_addstr(&buf, \".bitmap\");\n+\tbitmap_writer_finish(&writer, index, buf.buf, 0);\n+\n+\tbitmap_writer_free(&writer);\n+\tstrbuf_release(&buf);\n+\tfree(index);\n+\tclear_packing_data(&packed);\n+\n+\treturn 0;\n+}\n+\n int cmd__bitmap(int argc, const char **argv)\n {\n \tsetup_git_directory();\n@@ -51,13 +159,16 @@ int cmd__bitmap(int argc, const char **argv)\n \t\treturn bitmap_dump_pseudo_merge_commits(atoi(argv[2]));\n \tif (argc == 3 && !strcmp(argv[1], \"dump-pseudo-merge-objects\"))\n \t\treturn bitmap_dump_pseudo_merge_objects(atoi(argv[2]));\n+\tif (argc == 3 && !strcmp(argv[1], \"write\"))\n+\t\treturn bitmap_write(argv[2]);\n \n \tusage(\"\\ttest-tool bitmap list-commits\\n\"\n \t \"\\ttest-tool bitmap list-commits-with-offset\\n\"\n \t \"\\ttest-tool bitmap dump-hashes\\n\"\n \t \"\\ttest-tool bitmap dump-pseudo-merges\\n\"\n \t \"\\ttest-tool bitmap dump-pseudo-merge-commits \\n\"\n-\t \"\\ttest-tool bitmap dump-pseudo-merge-objects \");\n+\t \"\\ttest-tool bitmap dump-pseudo-merge-objects \\n\"\n+\t \"\\ttest-tool bitmap write < \");\n \n \treturn -1;\n }\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex f693cb56691..efeb71593bf 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -648,4 +648,28 @@ test_expect_success 'truncated bitmap fails gracefully (lookup table)' '\n \ttest_grep corrupted.bitmap.index stderr\n '\n \n+test_expect_success 'test-tool bitmap write determines bitmap selection' '\n+\ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n+\tgit init bitmap-write-helper &&\n+\t(\n+\t\tcd bitmap-write-helper &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n+\n+\t\tgit rev-parse HEAD >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" bitmaps.raw &&\n+\t\tsort bitmaps.raw >bitmaps &&\n+\t\ttest_cmp in bitmaps &&\n+\n+\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542059","messageId":"11de33437264ef2d4eaae9b718ff2f3b1e26a114.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 2/9] t5333: demonstrate various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:01:58Z","receivedAt":"2026-04-21T20:02:01Z","isPatch":true,"body":"Using the test helper introduced via the previous commit, add various\nfailing tests demonstrating bugs in the pseudo-merge implementation.\n\nThese are all marked as failing with one exception. The \"sampleRate=0\"\ntest describes a latent bug, which is only reachable through a code path\nthat is itself masked by a separate bug. A future commit will fix that\nbug, and, in turn, cause the aforementioned test to fail. Accordingly,\nthat commit will mark the test as failing, and it will be re-marked as\npassing in a separate commit which fixes the once-latent bug.\n\nFor the rest: the following commits will explain and fix the underlying\nbugs in detail.\n\nSigned-off-by: Taylor Blau \n---\n t/t5333-pseudo-merge-bitmaps.sh | 198 ++++++++++++++++++++++++++++++++\n 1 file changed, 198 insertions(+)\n\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 1f7a5d82ee4..0e9638c31c3 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -462,4 +462,202 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n \t)\n '\n \n+test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n+\ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n+\tgit init pseudo-merge-fill-in-traversal &&\n+\t(\n+\t\tcd pseudo-merge-fill-in-traversal &&\n+\n+\t\tgit config bitmapPseudoMerge.test.pattern refs/tags/ &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 1 in &&\n+\t\t\twhile read oid\n+\t\t\tdo\n+\t\t\t\techo \"create refs/group-$side/$oid $oid\" || return 1\n+\t\t\tdone in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 2 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 2 in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 2 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 2 in &&\n+\t\tGIT_TEST_DATE_NOW=$test_tick \\\n+\t\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges\n+\t)\n+'\n+\n+test_expect_success 'sampleRate=0 does not cause division by zero' '\n+\ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n+\tgit init pseudo-merge-sample-rate-zero &&\n+\t(\n+\t\tcd pseudo-merge-sample-rate-zero &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n+\n+\t\tgit config bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.sampleRate 0 &&\n+\t\tgit config bitmapPseudoMerge.test.threshold now &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" \n---\n pack-bitmap-write.c | 21 ++++++++++++++++++++-\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 8338d7217ef..86ed6a5d78c 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -819,6 +819,20 @@ static void write_selected_commits_v1(struct bitmap_writer *writer,\n \t}\n }\n \n+static int pseudo_merge_commit_pos_cmp(const void *_va, const void *_vb,\n+\t\t\t\t void *_data)\n+{\n+\tstruct bitmap_writer *writer = _data;\n+\tuint32_t pos_a = find_object_pos(writer, _va, NULL);\n+\tuint32_t pos_b = find_object_pos(writer, _vb, NULL);\n+\n+\tif (pos_a < pos_b)\n+\t\treturn -1;\n+\tif (pos_a > pos_b)\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n static void write_pseudo_merges(struct bitmap_writer *writer,\n \t\t\t\tstruct hashfile *f)\n {\n@@ -876,7 +890,12 @@ static void write_pseudo_merges(struct bitmap_writer *writer,\n \t\toid_array_append(&commits, &kh_key(writer->pseudo_merge_commits, i));\n \t}\n \n-\toid_array_sort(&commits);\n+\t/*\n+\t * Sort the commits by their bit position so that the lookup\n+\t * table can be binary searched by the reader (see\n+\t * find_pseudo_merge()).\n+\t */\n+\tQSORT_S(commits.oid, commits.nr, pseudo_merge_commit_pos_cmp, writer);\n \n \t/* write lookup table (non-extended) */\n \tfor (i = 0; i < commits.nr; i++) {\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 0e9638c31c3..3d7a7668121 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -462,7 +462,7 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n+test_expect_success 'apply pseudo-merges during fill-in traversal' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n \tgit init pseudo-merge-fill-in-traversal &&\n \t(\n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542061","messageId":"07f70a07c2034ae6bfc718d0e599a1a41dd77290.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 4/9] pack-bitmap: fix inverted binary search in `pseudo_merge_at()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:02:05Z","receivedAt":"2026-04-21T20:02:08Z","isPatch":true,"body":"The binary search in `pseudo_merge_at()` has its \"lo\" and \"hi\" updates\nswapped: when the midpoint's offset is less than the target, it sets `hi\n= mi` (searching left) instead of `lo = mi + 1` (searching right), and\nvice versa.\n\nThis means that lookups for pseudo-merges whose offset is not near the\nmidpoint of the pseudo-merge table are likely to fail.\n\nIn practice, with a single pseudo-merge group this is masked because the\nlone entry is always at the midpoint. With multiple groups, the inverted\ncomparisons cause lookups to search in the wrong direction, potentially\nmissing entries.\n\nSwap the \"lo\" and \"hi\" assignments to search in the correct direction,\nmaking it possible to apply pseudo-merges during fill-in when more than\none pseudo-merge exists in a group.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 3 ++-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex ff18b6c3642..fb71c761792 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -559,9 +559,9 @@ static struct pseudo_merge *pseudo_merge_at(const struct pseudo_merge_map *pm,\n \t\tif (got == want)\n \t\t\treturn use_pseudo_merge(pm, &pm->v[mi]);\n \t\telse if (got < want)\n-\t\t\thi = mi;\n-\t\telse\n \t\t\tlo = mi + 1;\n+\t\telse\n+\t\t\thi = mi;\n \t}\n \n \twarning(_(\"could not find pseudo-merge for commit %s at offset %\"PRIuMAX),\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 3d7a7668121..c5db6a11f6a 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -496,9 +496,10 @@ test_expect_success 'apply pseudo-merges during fill-in traversal' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '\n+test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n \tgit init pseudo-merge-fill-in-multi &&\n+\tgit init pseudo-merge-fill-in-multi &&\n \t(\n \t\tcd pseudo-merge-fill-in-multi &&\n \n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542062","messageId":"3ed0b39843f2102f17a4c9865e05eceba0198b46.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 5/9] pack-bitmap: fix pseudo-merge lookup for shared commits","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:02:08Z","receivedAt":"2026-04-21T20:02:13Z","isPatch":true,"body":"When a commit appears in more than one pseudo-merge group, its entry in\nthe commit lookup table has the high bit set in its offset field,\nindicating that the offset points to an \"extended\" table containing the\nset of pseudo-merges for that commit.\n\nThere are three bugs in this path:\n\n * The `next_ext` offset in `write_pseudo_merges()` undercounts the\n per-entry size of the lookup table (8 vs. 12 bytes).\n\n * `nth_pseudo_merge_ext()` calls `read_pseudo_merge_commit_at()` on a\n pseudo-merge bitmap offset, misinterpreting it as a 12-byte commit\n table entry.\n\n * The error check after `pseudo_merge_ext_at()` in\n `apply_pseudo_merges_for_commit()` tests `< -1` instead of `< 0`,\n silently swallowing errors from `error()`.\n\nThe first bug is on the write side: each commit lookup entry contains a\n4- and 8-byte unsigned value for a total of 12 bytes, but the\ncalculation assumes that the entry only contains 8 bytes of data. This\nmakes `next_ext` too small, so the extended-table offsets that get\nwritten point into the middle of the non-extended lookup table rather\nthan past it. The reader then interprets non-extended lookup data as\nextended entries, producing garbage.\n\nThe second bug is on the read side and is independently fatal: even with\na correctly positioned extended table, `nth_pseudo_merge_ext()` feeds\nthe offset it reads (which points at pseudo-merge bitmap data) to\n`read_pseudo_merge_commit_at()`. That function tries to parse 12 bytes\nas a `pseudo_merge_commit` struct, clobbering `merge->pseudo_merge_ofs`\nwith whatever happens to be at that location. The caller only needs\n`pseudo_merge_ofs`, so the fix is to store the offset directly rather\nthan re-parsing a commit table entry. The `commit_pos` field is left\nuntouched, retaining the value that `find_pseudo_merge()` set earlier.\n\nThe third bug is latent. With the first two fixes applied, the extended\ntable is correctly written and read, so `pseudo_merge_ext_at()` does not\nfail during normal operation. The `< -1` vs `< 0` distinction only\nmatters when the bitmap file is corrupt or truncated, in which case the\nerror would be silently ignored and the code would proceed with\nuninitialized data.\n\nSigned-off-by: Taylor Blau \n---\n pack-bitmap-write.c | 2 +-\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 86ed6a5d78c..1c8070f99c0 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -877,7 +877,7 @@ static void write_pseudo_merges(struct bitmap_writer *writer,\n \n \tnext_ext = st_add(hashfile_total(f),\n \t\t\t st_mult(kh_size(writer->pseudo_merge_commits),\n-\t\t\t\t sizeof(uint64_t)));\n+\t\t\t\t sizeof(uint32_t) + sizeof(uint64_t)));\n \n \ttable_start = hashfile_total(f);\n \ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex fb71c761792..34e1da00b4e 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -600,7 +600,7 @@ static int nth_pseudo_merge_ext(const struct pseudo_merge_map *pm,\n \t\treturn error(_(\"out-of-bounds read: (%\"PRIuMAX\" >= %\"PRIuMAX\")\"),\n \t\t\t (uintmax_t)ofs, (uintmax_t)pm->map_size);\n \n-\tread_pseudo_merge_commit_at(merge, pm->map + ofs);\n+\tmerge->pseudo_merge_ofs = ofs;\n \n \treturn 0;\n }\n@@ -671,7 +671,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\toff_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63);\n \t\tuint32_t i;\n \n-\t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < -1) {\n+\t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < 0) {\n \t\t\twarning(_(\"could not read extended pseudo-merge table \"\n \t\t\t\t \"for commit %s\"),\n \t\t\t\toid_to_hex(&commit->object.oid));\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex c5db6a11f6a..f558e87ab21 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -550,7 +550,7 @@ test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges with overlapping groups during fill-in' '\n+test_expect_success 'apply pseudo-merges with overlapping groups during fill-in' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-overlap\" &&\n \tgit init pseudo-merge-fill-in-overlap &&\n \t(\n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542063","messageId":"95f847211f339ab8a32350198902e113d85b8e5d.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 6/9] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:02:11Z","receivedAt":"2026-04-21T20:02:14Z","isPatch":true,"body":"`find_pseudo_merge_group_for_ref()` uses the commit's date to classify\nit as either \"stable\" (older than the stable threshold) or \"unstable\"\n(otherwise).\n\nHowever, to find the relevant commit from a given OID, the function\n`find_pseudo_merge_group_for_ref()` uses `lookup_commit()` which does\nnot parse commits.\n\nBecause an unparsed commit has its \"date\" set to zero, every candidate\nis placed in the \"stable\" bucket regardless of its actual committer\ntimestamp. This means the `bitmapPseudoMerge.*.threshold` and\n`stableThreshold` configuration options have no effect: the\nstable/unstable split is always determined by comparing against zero\nrather than the real commit date.\n\nThe net result is that pseudo-merge groups are partitioned by\n`stableSize` instead of the intended decay-based sizing, and the\n`sampleRate` knob (which only applies to the unstable path) is never\nexercised.\n\nFix this by calling `repo_parse_commit()` after `lookup_commit()`,\nbailing out of the callback if parsing fails.\n\nThe corresponding test configures two pseudo-merge groups that both\nmatch all tags. The \"stable\" group uses `threshold=1.month.ago`, and the\n\"all\" group uses `threshold=now`. The test use our custom\n\"GIT_TEST_DATE_NOW\" environment variable by setting it to the value of\n\"$test_tick\" to align Git's notion of \"now\" (and therefore\n\"1.month.ago\") with the `test_tick` timestamps, so the commits appear to\nbe younger than one month: only the \"all\" group matches them, producing\nexactly one pseudo-merge.\n\nWithout the fix every commit has `date == 0`, which satisfies `date <=\nthreshold` for both groups (since 0 is older than one month ago), and\nthe \"stable\" group erroneously matches as well.\n\nNow that commits are correctly classified as \"unstable\", the bug\ndescribed in the test exercising the \"sampleRate=0\" test is reachable,\nand the test is marked as failing. It will be fixed in a following\ncommit.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 2 ++\n t/t5333-pseudo-merge-bitmaps.sh | 22 ++++++++++++----------\n 2 files changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex 34e1da00b4e..d79e5fb649a 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -236,6 +236,8 @@ static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_d\n \tc = lookup_commit(the_repository, maybe_peeled);\n \tif (!c)\n \t\treturn 0;\n+\tif (repo_parse_commit(the_repository, c))\n+\t\treturn 0;\n \tif (!packlist_find(writer->to_pack, maybe_peeled))\n \t\treturn 0;\n \ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex f558e87ab21..e27f9850dc4 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -593,32 +593,34 @@ test_expect_success 'apply pseudo-merges with overlapping groups during fill-in'\n \t)\n '\n \n-test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n+test_expect_success 'pseudo-merge commits are correctly classified by date' '\n \ttest_when_finished \"rm -fr pseudo-merge-date-classification\" &&\n \tgit init pseudo-merge-date-classification &&\n \t(\n \t\tcd pseudo-merge-date-classification &&\n \n \t\ttest_commit_bulk 64 &&\n+\n \t\ttag_everything &&\n \t\tgit repack -ad &&\n \n \t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n \n \t\t# Configure two pseudo-merge groups: one that only\n-\t\t# matches \"stable\" refs (older than one month), and one\n-\t\t# that matches all refs. With 64 freshly-created tags\n-\t\t# (all younger than one month) the stable group should\n-\t\t# have zero pseudo-merges and the catch-all group should\n-\t\t# have one.\n+\t\t# matches \"stable\" refs (older than one month), and\n+\t\t# one that matches all refs. With 64 tags whose\n+\t\t# commits are all younger than one month, the\n+\t\t# \"stable\" group should have zero pseudo-merges and\n+\t\t# the \"all\" group should have one.\n \t\t#\n \t\t# Use GIT_TEST_DATE_NOW to align \"now\" (and therefore\n \t\t# \"1.month.ago\") with the test_tick timestamps so that\n \t\t# the commits are within the last month.\n \t\t#\n-\t\t# This exercises the date-based classification in\n-\t\t# find_pseudo_merge_group_for_ref(), which requires\n-\t\t# that commits are parsed before inspecting their date.\n+\t\t# Without parsing the commit, its date field would\n+\t\t# be zero, causing it to satisfy date <= threshold\n+\t\t# for the \"stable\" group as well, and both groups\n+\t\t# would produce pseudo-merges.\n \t\tgit config bitmapPseudoMerge.stable.pattern \"refs/tags/\" &&\n \t\tgit config bitmapPseudoMerge.stable.maxMerges 64 &&\n \t\tgit config bitmapPseudoMerge.stable.stableThreshold never &&\n@@ -638,7 +640,7 @@ test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n \t)\n '\n \n-test_expect_success 'sampleRate=0 does not cause division by zero' '\n+test_expect_failure 'sampleRate=0 does not cause division by zero' '\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n \tgit init pseudo-merge-sample-rate-zero &&\n \t(\n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542064","messageId":"f8a01cfb8932f031999621eafda4c2600067eca5.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 7/9] pack-bitmap: reject pseudo-merge \"sampleRate\" of 0","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:02:14Z","receivedAt":"2026-04-21T20:02:16Z","isPatch":true,"body":"The \"bitmapPseudoMerge.*.sampleRate\" configuration controls what\nfraction of unstable commits are included in each pseudo-merge group.\nThe config validation accepts values in the range `[0, 1]`, but a value\nof exactly 0 causes a division by zero in `select_pseudo_merges_1()`:\n\n if (j % (uint32_t)(1.0 / group->sample_rate))\n\nWhen `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting\ninfinity to `uint32_t` is undefined behavior in C. On most platforms\nthis yields 0, making the subsequent modulo operation (`j % 0`) a\nfatal arithmetic trap.\n\nThis path was not previously reachable because an earlier bug caused\nall pseudo-merge candidates to be classified as \"stable\" (where the\nsampling rate is not used), regardless of their actual commit date. Now\nthat the date classification is fixed, the unstable path is exercised\nand the division by zero can fire.\n\nFix this by changing the validation to require a strict lower bound and\nthus reject 0.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/config/bitmap-pseudo-merge.adoc | 4 ++--\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/bitmap-pseudo-merge.adoc b/Documentation/config/bitmap-pseudo-merge.adoc\nindex 1f264eca99b..6bf52c80ba7 100644\n--- a/Documentation/config/bitmap-pseudo-merge.adoc\n+++ b/Documentation/config/bitmap-pseudo-merge.adoc\n@@ -47,8 +47,8 @@ will be updated more often than a reference pointing at an old commit.\n bitmapPseudoMerge..sampleRate::\n \tDetermines the proportion of non-bitmapped commits (among\n \treference tips) which are selected for inclusion in an\n-\tunstable pseudo-merge bitmap. Must be between `0` and `1`\n-\t(inclusive). The default is `1`.\n+\tunstable pseudo-merge bitmap. Must be greater than `0` and\n+\tless than or equal to `1`. The default is `1`.\n \n bitmapPseudoMerge..threshold::\n \tDetermines the minimum age of non-bitmapped commits (among\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex d79e5fb649a..75bed043602 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -169,8 +169,8 @@ static int pseudo_merge_config(const char *var, const char *value,\n \t\t}\n \t} else if (!strcmp(key, \"samplerate\")) {\n \t\tgroup->sample_rate = git_config_double(var, value, ctx->kvi);\n-\t\tif (!(0 <= group->sample_rate && group->sample_rate <= 1)) {\n-\t\t\twarning(_(\"%s must be between 0 and 1, using default\"), var);\n+\t\tif (!(0 < group->sample_rate && group->sample_rate <= 1)) {\n+\t\t\twarning(_(\"%s must be between 0 (exclusive) and 1, using default\"), var);\n \t\t\tgroup->sample_rate = DEFAULT_PSEUDO_MERGE_SAMPLE_RATE;\n \t\t}\n \t} else if (!strcmp(key, \"threshold\")) {\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex e27f9850dc4..3d0617a2e17 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -640,7 +640,7 @@ test_expect_success 'pseudo-merge commits are correctly classified by date' '\n \t)\n '\n \n-test_expect_failure 'sampleRate=0 does not cause division by zero' '\n+test_expect_success 'sampleRate=0 does not cause division by zero' '\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n \tgit init pseudo-merge-sample-rate-zero &&\n \t(\n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542065","messageId":"c37156502c006572fd54c2a41a3db9a1553d9a4e.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 8/9] Documentation: fix broken `sampleRate` in gitpacking(7)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:02:17Z","receivedAt":"2026-04-21T20:02:19Z","isPatch":true,"body":"The documentation explaining some sample configurations for bitmap\npseudo-merges incorrectly uses a sample rate outside of the allowed\n(0,1] range.\n\nThis dates back to faf558b23ef (pseudo-merge: implement support for\nselecting pseudo-merge commits, 2024-05-23), and was likely written when\nthe allowable range for this configuration was the integral values\nbetween (0,100].\n\nFix this to conform to the actual allowable range for this\nconfiguration.\n\nNoticed-by: Elijah Newren \nSigned-off-by: Taylor Blau \n---\n Documentation/gitpacking.adoc | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/gitpacking.adoc b/Documentation/gitpacking.adoc\nindex a56596e2d1d..e6de6ec8249 100644\n--- a/Documentation/gitpacking.adoc\n+++ b/Documentation/gitpacking.adoc\n@@ -150,7 +150,7 @@ with a configuration like so:\n \tpattern = \"refs/\"\n \tthreshold = now\n \tstableThreshold = never\n-\tsampleRate = 100\n+\tsampleRate = 1\n \tmaxMerges = 64\n ----\n \n@@ -177,7 +177,7 @@ like:\n \tpattern = \"refs/virtual/([0-9]+)/(heads|tags)/\"\n \tthreshold = now\n \tstableThreshold = never\n-\tsampleRate = 100\n+\tsampleRate = 1\n \tmaxMerges = 64\n ----\n \n-- \n2.54.0.9.gb905fd5d0ae\n\n"},{"id":"542066","messageId":"b905fd5d0ae206128aeb3ab2f4c3aaca6c8fe8d7.1776801694.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"[PATCH v2 9/9] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-21T20:02:20Z","receivedAt":"2026-04-21T20:02:23Z","isPatch":true,"body":"When \"bitmapPseudoMerge.*.pattern\" appears more than once for the same\ngroup, `pseudo_merge_config()` frees the old `regex_t *` pointer\nbut does not call `regfree()` on it first. This leaks whatever internal\nstate `regcomp()` allocated.\n\nThe final cleanup path in `pseudo_merge_group_release()` does call\n`regfree()` before `free()`, so only the intermediate replacement is\naffected.\n\nFix this by guarding the replacement with a NULL check and calling\n`regfree()` before `free()` when the pointer is non-NULL.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 5 ++++-\n t/t5333-pseudo-merge-bitmaps.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex 75bed043602..22b8600d689 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -150,7 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,\n \tif (!strcmp(key, \"pattern\")) {\n \t\tstruct strbuf re = STRBUF_INIT;\n \n-\t\tfree(group->pattern);\n+\t\tif (group->pattern) {\n+\t\t\tregfree(group->pattern);\n+\t\t\tfree(group->pattern);\n+\t\t}\n \t\tif (*value != '^')\n \t\t\tstrbuf_addch(&re, '^');\n \t\tstrbuf_addstr(&re, value);\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 3d0617a2e17..382513ca5cc 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -663,4 +663,34 @@ test_expect_success 'sampleRate=0 does not cause division by zero' '\n \t)\n '\n \n+test_expect_success 'duplicate pseudo-merge pattern does not leak' '\n+\tgit init pseudo-merge-dup-pattern &&\n+\ttest_when_finished \"rm -fr pseudo-merge-dup-pattern\" &&\n+\n+\t(\n+\t\tcd pseudo-merge-dup-pattern &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\n+\t\t# Set the same group'\\''s pattern twice. The second\n+\t\t# assignment should cleanly release the compiled regex\n+\t\t# from the first without leaking.\n+\t\tgit config bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config --add bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.threshold now &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\tgit rev-parse HEAD~63 |\n+\t\ttest-tool bitmap write \"$(basename $pack)\" &&\n+\n+\t\ttest_pseudo_merges >merges &&\n+\t\ttest_line_count = 1 merges\n+\t)\n+'\n+\n test_done\n-- \n2.54.0.9.gb905fd5d0ae\n"},{"id":"542094","messageId":"CABPp-BGkfavqezk2SV3+K6iF8MLm8j_=ijHiPDLmv_U_o_Ykgg@mail.gmail.com","threadId":"65478","inReplyTo":"cover.1776801694.git.me@ttaylorr.com","subject":"Re: [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-04-22T01:37:45Z","receivedAt":"2026-04-22T01:37:58Z","isPatch":true,"body":"On Tue, Apr 21, 2026 at 1:01 PM Taylor Blau wrote:\n>\n> [Note to the maintainer: this series has been rebased onto the current\n> tip of master, which is 94f057755b7 (Git 2.54, 2026-04-19) at the time\n> of writing.]\n>\n> This is a small reroll of my series to fix several bugs in the\n> pseudo-merge bitmap implementation. The main changes since last time\n> are:\n>\n> - Fixed a use-after-realloc bug in the test helper introduced in the\n> first commit.\n>\n> - Swapped the order of initializing and cleaning up repositories in the\n> new test scripts.\n>\n> - Updated bitmapPseudoMerge..sampleRate's documentation to\n> describe the range as (0,1], and added a new commit fixing a broken\n> example in gitpacking(7).\n\nThanks for fixing these.\n\n> Range-diff against v1:\n[...]\n> 4: af9f651269d ! 4: 07f70a07c20 pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during\n>\n> -test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '\n> +test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n> - git init pseudo-merge-fill-in-multi &&\n> test_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n> + git init pseudo-merge-fill-in-multi &&\n\nHere you fixed the order, but...\n\n> ++ git init pseudo-merge-fill-in-multi &&\n\n...then you immediately run git init a second time? I'm guessing this\nwas a stray edit made while trying to fix the order; could we get rid\nof the duplicate?\n\n> (\n> + cd pseudo-merge-fill-in-multi &&\n> +\n\nLooks like you addressed all the feedback so far from v1. There does\nappear to be a new accidental double-init that I noted above in patch\n4, but I didn't spot any other issues.\n"},{"id":"543003","messageId":"xmqqpl32u06q.fsf@gitster.g","threadId":"65478","inReplyTo":"CABPp-BGkfavqezk2SV3+K6iF8MLm8j_=ijHiPDLmv_U_o_Ykgg@mail.gmail.com","subject":"Re: [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-11T02:53:01Z","receivedAt":"2026-05-11T02:53:03Z","isPatch":true,"body":"Elijah Newren writes:\n\n>> + git init pseudo-merge-fill-in-multi &&\n>\n> Here you fixed the order, but...\n>\n>> ++ git init pseudo-merge-fill-in-multi &&\n>\n> ...then you immediately run git init a second time? I'm guessing this\n> was a stray edit made while trying to fix the order; could we get rid\n> of the duplicate?\n>\n>> (\n>> + cd pseudo-merge-fill-in-multi &&\n>> +\n>\n> Looks like you addressed all the feedback so far from v1. There does\n> appear to be a new accidental double-init that I noted above in patch\n> 4, but I didn't spot any other issues.\n\nThe topic went dormant after this comment, and it seems that it is\nso close to the finish line otherwise? I'll leave the topic marked\nas \"Expecting (hopefully minor and final) reroll\" in the draft\n\"What's cooking\" report I work from for now.\n\nThanks.\n"},{"id":"543101","messageId":"agJv3lVbud9V7Vxy@nand.local","threadId":"65478","inReplyTo":"CABPp-BGkfavqezk2SV3+K6iF8MLm8j_=ijHiPDLmv_U_o_Ykgg@mail.gmail.com","subject":"Re: [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:10:06Z","receivedAt":"2026-05-12T00:10:08Z","isPatch":true,"body":"On Tue, Apr 21, 2026 at 06:37:45PM -0700, Elijah Newren wrote:\n> Here you fixed the order, but...\n>\n> > ++ git init pseudo-merge-fill-in-multi &&\n>\n> ...then you immediately run git init a second time? I'm guessing this\n> was a stray edit made while trying to fix the order; could we get rid\n> of the duplicate?\n\nOof, good catch. I'm not sure how that snuck in there, but it's fixed on\nmy end.\n\n> > (\n> > + cd pseudo-merge-fill-in-multi &&\n> > +\n>\n> Looks like you addressed all the feedback so far from v1. There does\n> appear to be a new accidental double-init that I noted above in patch\n> 4, but I didn't spot any other issues.\n\nBesides that, I found one more spot that needed some love, which is the\nnew \"duplicate pseudo-merge pattern does not leak\" test added at the\nvery end of the series, which had a wrong ordering, and piped the output\nof 'git rev-parse' directly into a test helper.\n\nI'll send a fixed up round out now.\n\nThanks,\nTaylor\n"},{"id":"543104","messageId":"cover.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1776124588.git.me@ttaylorr.com","subject":"[PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:46:44Z","receivedAt":"2026-05-12T00:46:47Z","isPatch":true,"body":"[Note to the maintainer: this series has been rebased onto the current\ntip of master, which is 7760f83b597 (Merge branch\n'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].\n\nThis is a (very) small reroll of my series to fix several bugs in the\npseudo-merge bitmap implementation. The only changes since last time are:\n\n - consistently ordering `test_when_finished \"rm -fr ...\"` above `git\n init` in tests, and\n\n - avoiding a single Git invocation on the left-hand side of a pipe in\n the final patch\n\nOtherwise, the series is entirely unchanged save for the above and the\nrebase.\n\nAs usual, a range-diff is included below for convenience. The original\ncover letter is as follows:\n\n========================================================================\n\nThis series fixes several bugs in the pseudo-merge bitmap implementation\nthat caused the pseudo-merge application path to be effectively broken\nduring fill-in traversal.\n\nPeff noticed that this code path was never triggered by the existing\ntest suite, and investigating that observation uncovered a handful of\nbugs, some compounding.\n\nThe first two patches introduce test infrastructure: a 'bitmap write'\ntest helper that gives tests precise control over which commits receive\nindividual bitmaps, and a set of \"test_expect_failure\" tests\ndemonstrating each bug.\n\nThe next four patches fix the bugs in the per-commit pseudo-merge\nlookup:\n\n - The pseudo-merge commit lookup table was sorted by OID rather than\n by bit position, causing the reader's binary search to fail.\n\n - The binary search in pseudo_merge_at() had its lo/hi updates\n swapped.\n\n - The extended pseudo-merge lookup path had three compounding bugs: a\n wrong entry-size calculation in the writer, a misinterpretation of\n extended table entries in the reader, and a silently-swallowed error\n check.\n\nThe final two patches fix issues in pseudo-merge group selection:\n\n - find_pseudo_merge_group_for_ref() did not parse commits before\n inspecting their dates, so all candidates had date == 0 and were\n unconditionally placed in the \"stable\" bucket.\n\n - The config validation for bitmapPseudoMerge.*.sampleRate accepted 0,\n which leads to a division by zero once the date classification is\n fixed and the unstable code path is exercised.\n\nThere is also a small fix for a regex leak when the pattern key is\noverridden in config.\n\nThanks in advance for your review!\n\nTaylor Blau (9):\n t/helper: add 'test-tool bitmap write' subcommand\n t5333: demonstrate various pseudo-merge bugs\n pack-bitmap-write: sort pseudo-merge commit lookup table in pack order\n pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n pack-bitmap: fix pseudo-merge lookup for shared commits\n pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`\n pack-bitmap: reject pseudo-merge \"sampleRate\" of 0\n Documentation: fix broken `sampleRate` in gitpacking(7)\n pack-bitmap: prevent pattern leak on pseudo-merge re-assignment\n\n Documentation/config/bitmap-pseudo-merge.adoc | 4 +-\n Documentation/gitpacking.adoc | 4 +-\n pack-bitmap-write.c | 23 +-\n pseudo-merge.c | 19 +-\n t/helper/test-bitmap.c | 113 ++++++++-\n t/t5310-pack-bitmaps.sh | 24 ++\n t/t5333-pseudo-merge-bitmaps.sh | 229 ++++++++++++++++++\n 7 files changed, 402 insertions(+), 14 deletions(-)\n\nRange-diff against v2:\n 1: c0df35f8ebd = 1: 9c7a829cbeb t/helper: add 'test-tool bitmap write' subcommand\n 2: 11de3343726 = 2: d1ed4aadf75 t5333: demonstrate various pseudo-merge bugs\n 3: 8d908ab415e = 3: bf3a9a07e5f pack-bitmap-write: sort pseudo-merge commit lookup table in pack order\n 4: 07f70a07c20 ! 4: a1d341c92eb pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during\n +test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n \tgit init pseudo-merge-fill-in-multi &&\n -+\tgit init pseudo-merge-fill-in-multi &&\n \t(\n - \t\tcd pseudo-merge-fill-in-multi &&\n - \n 5: 3ed0b39843f = 5: 06e3410d323 pack-bitmap: fix pseudo-merge lookup for shared commits\n 6: 95f847211f3 = 6: 78cf7e6d80d pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`\n 7: f8a01cfb893 = 7: 4dbf6686718 pack-bitmap: reject pseudo-merge \"sampleRate\" of 0\n 8: c37156502c0 = 8: 46d0ee2f168 Documentation: fix broken `sampleRate` in gitpacking(7)\n 9: b905fd5d0ae ! 9: 9b17dab2cf7 pack-bitmap: prevent pattern leak on pseudo-merge re-assignment\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus\n '\n \n +test_expect_success 'duplicate pseudo-merge pattern does not leak' '\n -+\tgit init pseudo-merge-dup-pattern &&\n +\ttest_when_finished \"rm -fr pseudo-merge-dup-pattern\" &&\n -+\n ++\tgit init pseudo-merge-dup-pattern &&\n +\t(\n +\t\tcd pseudo-merge-dup-pattern &&\n +\n @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus\n +\t\tgit config bitmapPseudoMerge.test.threshold now &&\n +\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n +\n -+\t\tgit rev-parse HEAD~63 |\n -+\t\ttest-tool bitmap write \"$(basename $pack)\" &&\n ++\t\tgit rev-parse HEAD~63 >in &&\n ++\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n +\t\ttest_line_count = 1 merges\n\nbase-commit: 7760f83b59750c27df653c5c46d0f80e44cfe02c\n-- \n2.54.0.76.g9b17dab2cf7\n"},{"id":"543105","messageId":"9c7a829cbeb0321866e684228a954cbf9547ee02.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 1/9] t/helper: add 'test-tool bitmap write' subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:46:48Z","receivedAt":"2026-05-12T00:46:50Z","isPatch":true,"body":"In f16eb1c091 (pseudo-merge: fix disk reads from find_pseudo_merge(),\n2026-03-31), we noted that `apply_pseudo_merges_for_commit()` is never\ntriggered by the existing test suite, and that this bears further\ninvestigation.\n\nThis patch is the first one to begin that investigation. The following\npatches will expose and fix a variety of bugs in the implementation of\npseudo-merge bitmaps.\n\nIn order to do so, however, many of these tests require very precise\nselection of which commits receive bitmaps and which do not. To date,\nthere isn't a standard approach to easily facilitate this. Address this\nby introducing a `test-tool bitmap write` subcommand that writes a\nbitmap for a given packfile, reading the set of commits which should\nreceive individual bitmaps from stdin like so:\n\n test-tool bitmap write \" is the filename for a specific packfile (e.g.,\n\"pack-abc123.pack\"), and \"/path/to/commits.list\" is a list of commit\nOIDs which will receive bitmaps.\n\nThe helper respects `bitmapPseudoMerge.*` configuration for creating\npseudo-merge bitmaps alongside the regular commit bitmaps.\n\nSigned-off-by: Taylor Blau \n---\n t/helper/test-bitmap.c | 113 +++++++++++++++++++++++++++++++++++++++-\n t/t5310-pack-bitmaps.sh | 24 +++++++++\n 2 files changed, 136 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c\nindex 16a01669e41..381e9b58b2c 100644\n--- a/t/helper/test-bitmap.c\n+++ b/t/helper/test-bitmap.c\n@@ -2,7 +2,10 @@\n \n #include \"test-tool.h\"\n #include \"git-compat-util.h\"\n+#include \"hex.h\"\n+#include \"odb.h\"\n #include \"pack-bitmap.h\"\n+#include \"pseudo-merge.h\"\n #include \"setup.h\"\n \n static int bitmap_list_commits(void)\n@@ -35,6 +38,111 @@ static int bitmap_dump_pseudo_merge_objects(uint32_t n)\n \treturn test_bitmap_pseudo_merge_objects(the_repository, n);\n }\n \n+static int add_packed_object(const struct object_id *oid,\n+\t\t\t struct packed_git *pack,\n+\t\t\t uint32_t pos,\n+\t\t\t void *_data)\n+{\n+\tstruct packing_data *packed = _data;\n+\tstruct object_entry *entry;\n+\tstruct object_info oi = OBJECT_INFO_INIT;\n+\tenum object_type type;\n+\n+\toi.typep = &type;\n+\n+\tentry = packlist_alloc(packed, oid);\n+\tentry->idx.offset = nth_packed_object_offset(pack, pos);\n+\tif (packed_object_info(pack, entry->idx.offset, &oi) < 0)\n+\t\tdie(\"could not get type of object %s\",\n+\t\t oid_to_hex(oid));\n+\toe_set_type(entry, type);\n+\toe_set_in_pack(packed, entry, pack);\n+\n+\treturn 0;\n+}\n+\n+static int idx_oid_cmp(const void *va, const void *vb)\n+{\n+\tconst struct pack_idx_entry *a = *(const struct pack_idx_entry **)va;\n+\tconst struct pack_idx_entry *b = *(const struct pack_idx_entry **)vb;\n+\n+\treturn oidcmp(&a->oid, &b->oid);\n+}\n+\n+static int bitmap_write(const char *basename)\n+{\n+\tstruct packed_git *p = NULL;\n+\tstruct packing_data packed = { 0 };\n+\tstruct bitmap_writer writer;\n+\tstruct pack_idx_entry **index;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tuint32_t i;\n+\n+\tprepare_repo_settings(the_repository);\n+\trepo_for_each_pack(the_repository, p) {\n+\t\tif (!strcmp(pack_basename(p), basename))\n+\t\t\tbreak;\n+\t}\n+\n+\tif (!p)\n+\t\tdie(\"could not find pack '%s'\", basename);\n+\n+\tif (open_pack_index(p))\n+\t\tdie(\"cannot open pack index for '%s'\", p->pack_name);\n+\n+\tprepare_packing_data(the_repository, &packed);\n+\n+\tfor_each_object_in_pack(p, add_packed_object, &packed,\n+\t\t\t\tODB_FOR_EACH_OBJECT_PACK_ORDER);\n+\n+\t/*\n+\t * Build the index array now that data.packed.objects[] is\n+\t * fully allocated (packlist_alloc() may have reallocated it\n+\t * during the loop above).\n+\t */\n+\tALLOC_ARRAY(index, p->num_objects);\n+\tfor (i = 0; i < p->num_objects; i++)\n+\t\tindex[i] = &packed.objects[i].idx;\n+\n+\tbitmap_writer_init(&writer, the_repository, &packed, NULL);\n+\tbitmap_writer_build_type_index(&writer, index);\n+\n+\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n+\t\tstruct object_id oid;\n+\t\tstruct commit *c;\n+\n+\t\tif (get_oid_hex(buf.buf, &oid))\n+\t\t\tdie(\"invalid OID: %s\", buf.buf);\n+\n+\t\tc = lookup_commit(the_repository, &oid);\n+\t\tif (!c || repo_parse_commit(the_repository, c))\n+\t\t\tdie(\"could not parse commit %s\", buf.buf);\n+\n+\t\tbitmap_writer_push_commit(&writer, c, 0);\n+\t}\n+\n+\tselect_pseudo_merges(&writer);\n+\tif (bitmap_writer_build(&writer) < 0)\n+\t\tdie(\"failed to build bitmaps\");\n+\n+\tbitmap_writer_set_checksum(&writer, p->hash);\n+\n+\tQSORT(index, p->num_objects, idx_oid_cmp);\n+\n+\tstrbuf_reset(&buf);\n+\tstrbuf_addstr(&buf, p->pack_name);\n+\tstrbuf_strip_suffix(&buf, \".pack\");\n+\tstrbuf_addstr(&buf, \".bitmap\");\n+\tbitmap_writer_finish(&writer, index, buf.buf, 0);\n+\n+\tbitmap_writer_free(&writer);\n+\tstrbuf_release(&buf);\n+\tfree(index);\n+\tclear_packing_data(&packed);\n+\n+\treturn 0;\n+}\n+\n int cmd__bitmap(int argc, const char **argv)\n {\n \tsetup_git_directory();\n@@ -51,13 +159,16 @@ int cmd__bitmap(int argc, const char **argv)\n \t\treturn bitmap_dump_pseudo_merge_commits(atoi(argv[2]));\n \tif (argc == 3 && !strcmp(argv[1], \"dump-pseudo-merge-objects\"))\n \t\treturn bitmap_dump_pseudo_merge_objects(atoi(argv[2]));\n+\tif (argc == 3 && !strcmp(argv[1], \"write\"))\n+\t\treturn bitmap_write(argv[2]);\n \n \tusage(\"\\ttest-tool bitmap list-commits\\n\"\n \t \"\\ttest-tool bitmap list-commits-with-offset\\n\"\n \t \"\\ttest-tool bitmap dump-hashes\\n\"\n \t \"\\ttest-tool bitmap dump-pseudo-merges\\n\"\n \t \"\\ttest-tool bitmap dump-pseudo-merge-commits \\n\"\n-\t \"\\ttest-tool bitmap dump-pseudo-merge-objects \");\n+\t \"\\ttest-tool bitmap dump-pseudo-merge-objects \\n\"\n+\t \"\\ttest-tool bitmap write < \");\n \n \treturn -1;\n }\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex f693cb56691..efeb71593bf 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -648,4 +648,28 @@ test_expect_success 'truncated bitmap fails gracefully (lookup table)' '\n \ttest_grep corrupted.bitmap.index stderr\n '\n \n+test_expect_success 'test-tool bitmap write determines bitmap selection' '\n+\ttest_when_finished \"rm -fr bitmap-write-helper\" &&\n+\tgit init bitmap-write-helper &&\n+\t(\n+\t\tcd bitmap-write-helper &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n+\n+\t\tgit rev-parse HEAD >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" bitmaps.raw &&\n+\t\tsort bitmaps.raw >bitmaps &&\n+\t\ttest_cmp in bitmaps &&\n+\n+\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543106","messageId":"d1ed4aadf7547a62f2442ee247dcfca3d8b4ca9f.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 2/9] t5333: demonstrate various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:46:51Z","receivedAt":"2026-05-12T00:46:53Z","isPatch":true,"body":"Using the test helper introduced via the previous commit, add various\nfailing tests demonstrating bugs in the pseudo-merge implementation.\n\nThese are all marked as failing with one exception. The \"sampleRate=0\"\ntest describes a latent bug, which is only reachable through a code path\nthat is itself masked by a separate bug. A future commit will fix that\nbug, and, in turn, cause the aforementioned test to fail. Accordingly,\nthat commit will mark the test as failing, and it will be re-marked as\npassing in a separate commit which fixes the once-latent bug.\n\nFor the rest: the following commits will explain and fix the underlying\nbugs in detail.\n\nSigned-off-by: Taylor Blau \n---\n t/t5333-pseudo-merge-bitmaps.sh | 198 ++++++++++++++++++++++++++++++++\n 1 file changed, 198 insertions(+)\n\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 1f7a5d82ee4..0e9638c31c3 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -462,4 +462,202 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n \t)\n '\n \n+test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n+\ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n+\tgit init pseudo-merge-fill-in-traversal &&\n+\t(\n+\t\tcd pseudo-merge-fill-in-traversal &&\n+\n+\t\tgit config bitmapPseudoMerge.test.pattern refs/tags/ &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 1 in &&\n+\t\t\twhile read oid\n+\t\t\tdo\n+\t\t\t\techo \"create refs/group-$side/$oid $oid\" || return 1\n+\t\t\tdone in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 2 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 2 in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 2 merges &&\n+\n+\t\ttest_commit stale &&\n+\n+\t\tgit rev-list --count --objects HEAD >expect &&\n+\n+\t\t: >trace2.txt &&\n+\t\tGIT_TRACE2_EVENT=$PWD/trace2.txt \\\n+\t\t\tgit rev-list --count --objects --use-bitmap-index HEAD >actual &&\n+\t\ttest_pseudo_merges_satisfied 2 in &&\n+\t\tGIT_TEST_DATE_NOW=$test_tick \\\n+\t\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges\n+\t)\n+'\n+\n+test_expect_success 'sampleRate=0 does not cause division by zero' '\n+\ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n+\tgit init pseudo-merge-sample-rate-zero &&\n+\t(\n+\t\tcd pseudo-merge-sample-rate-zero &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n+\n+\t\tgit config bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.sampleRate 0 &&\n+\t\tgit config bitmapPseudoMerge.test.threshold now &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" \n---\n pack-bitmap-write.c | 21 ++++++++++++++++++++-\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 8338d7217ef..86ed6a5d78c 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -819,6 +819,20 @@ static void write_selected_commits_v1(struct bitmap_writer *writer,\n \t}\n }\n \n+static int pseudo_merge_commit_pos_cmp(const void *_va, const void *_vb,\n+\t\t\t\t void *_data)\n+{\n+\tstruct bitmap_writer *writer = _data;\n+\tuint32_t pos_a = find_object_pos(writer, _va, NULL);\n+\tuint32_t pos_b = find_object_pos(writer, _vb, NULL);\n+\n+\tif (pos_a < pos_b)\n+\t\treturn -1;\n+\tif (pos_a > pos_b)\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n static void write_pseudo_merges(struct bitmap_writer *writer,\n \t\t\t\tstruct hashfile *f)\n {\n@@ -876,7 +890,12 @@ static void write_pseudo_merges(struct bitmap_writer *writer,\n \t\toid_array_append(&commits, &kh_key(writer->pseudo_merge_commits, i));\n \t}\n \n-\toid_array_sort(&commits);\n+\t/*\n+\t * Sort the commits by their bit position so that the lookup\n+\t * table can be binary searched by the reader (see\n+\t * find_pseudo_merge()).\n+\t */\n+\tQSORT_S(commits.oid, commits.nr, pseudo_merge_commit_pos_cmp, writer);\n \n \t/* write lookup table (non-extended) */\n \tfor (i = 0; i < commits.nr; i++) {\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 0e9638c31c3..3d7a7668121 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -462,7 +462,7 @@ test_expect_success 'use pseudo-merge in boundary traversal' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges during fill-in traversal' '\n+test_expect_success 'apply pseudo-merges during fill-in traversal' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-traversal\" &&\n \tgit init pseudo-merge-fill-in-traversal &&\n \t(\n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543108","messageId":"a1d341c92eb6aa7defe87b6daf556c1643083ec3.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 4/9] pack-bitmap: fix inverted binary search in `pseudo_merge_at()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:46:57Z","receivedAt":"2026-05-12T00:46:59Z","isPatch":true,"body":"The binary search in `pseudo_merge_at()` has its \"lo\" and \"hi\" updates\nswapped: when the midpoint's offset is less than the target, it sets `hi\n= mi` (searching left) instead of `lo = mi + 1` (searching right), and\nvice versa.\n\nThis means that lookups for pseudo-merges whose offset is not near the\nmidpoint of the pseudo-merge table are likely to fail.\n\nIn practice, with a single pseudo-merge group this is masked because the\nlone entry is always at the midpoint. With multiple groups, the inverted\ncomparisons cause lookups to search in the wrong direction, potentially\nmissing entries.\n\nSwap the \"lo\" and \"hi\" assignments to search in the correct direction,\nmaking it possible to apply pseudo-merges during fill-in when more than\none pseudo-merge exists in a group.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex ff18b6c3642..fb71c761792 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -559,9 +559,9 @@ static struct pseudo_merge *pseudo_merge_at(const struct pseudo_merge_map *pm,\n \t\tif (got == want)\n \t\t\treturn use_pseudo_merge(pm, &pm->v[mi]);\n \t\telse if (got < want)\n-\t\t\thi = mi;\n-\t\telse\n \t\t\tlo = mi + 1;\n+\t\telse\n+\t\t\thi = mi;\n \t}\n \n \twarning(_(\"could not find pseudo-merge for commit %s at offset %\"PRIuMAX),\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 3d7a7668121..5411fbf1e04 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -496,7 +496,7 @@ test_expect_success 'apply pseudo-merges during fill-in traversal' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '\n+test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n \tgit init pseudo-merge-fill-in-multi &&\n \t(\n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543109","messageId":"06e3410d323db13f7fd836ba79cd97586c511227.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 5/9] pack-bitmap: fix pseudo-merge lookup for shared commits","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:47:00Z","receivedAt":"2026-05-12T00:47:02Z","isPatch":true,"body":"When a commit appears in more than one pseudo-merge group, its entry in\nthe commit lookup table has the high bit set in its offset field,\nindicating that the offset points to an \"extended\" table containing the\nset of pseudo-merges for that commit.\n\nThere are three bugs in this path:\n\n * The `next_ext` offset in `write_pseudo_merges()` undercounts the\n per-entry size of the lookup table (8 vs. 12 bytes).\n\n * `nth_pseudo_merge_ext()` calls `read_pseudo_merge_commit_at()` on a\n pseudo-merge bitmap offset, misinterpreting it as a 12-byte commit\n table entry.\n\n * The error check after `pseudo_merge_ext_at()` in\n `apply_pseudo_merges_for_commit()` tests `< -1` instead of `< 0`,\n silently swallowing errors from `error()`.\n\nThe first bug is on the write side: each commit lookup entry contains a\n4- and 8-byte unsigned value for a total of 12 bytes, but the\ncalculation assumes that the entry only contains 8 bytes of data. This\nmakes `next_ext` too small, so the extended-table offsets that get\nwritten point into the middle of the non-extended lookup table rather\nthan past it. The reader then interprets non-extended lookup data as\nextended entries, producing garbage.\n\nThe second bug is on the read side and is independently fatal: even with\na correctly positioned extended table, `nth_pseudo_merge_ext()` feeds\nthe offset it reads (which points at pseudo-merge bitmap data) to\n`read_pseudo_merge_commit_at()`. That function tries to parse 12 bytes\nas a `pseudo_merge_commit` struct, clobbering `merge->pseudo_merge_ofs`\nwith whatever happens to be at that location. The caller only needs\n`pseudo_merge_ofs`, so the fix is to store the offset directly rather\nthan re-parsing a commit table entry. The `commit_pos` field is left\nuntouched, retaining the value that `find_pseudo_merge()` set earlier.\n\nThe third bug is latent. With the first two fixes applied, the extended\ntable is correctly written and read, so `pseudo_merge_ext_at()` does not\nfail during normal operation. The `< -1` vs `< 0` distinction only\nmatters when the bitmap file is corrupt or truncated, in which case the\nerror would be silently ignored and the code would proceed with\nuninitialized data.\n\nSigned-off-by: Taylor Blau \n---\n pack-bitmap-write.c | 2 +-\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 86ed6a5d78c..1c8070f99c0 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -877,7 +877,7 @@ static void write_pseudo_merges(struct bitmap_writer *writer,\n \n \tnext_ext = st_add(hashfile_total(f),\n \t\t\t st_mult(kh_size(writer->pseudo_merge_commits),\n-\t\t\t\t sizeof(uint64_t)));\n+\t\t\t\t sizeof(uint32_t) + sizeof(uint64_t)));\n \n \ttable_start = hashfile_total(f);\n \ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex fb71c761792..34e1da00b4e 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -600,7 +600,7 @@ static int nth_pseudo_merge_ext(const struct pseudo_merge_map *pm,\n \t\treturn error(_(\"out-of-bounds read: (%\"PRIuMAX\" >= %\"PRIuMAX\")\"),\n \t\t\t (uintmax_t)ofs, (uintmax_t)pm->map_size);\n \n-\tread_pseudo_merge_commit_at(merge, pm->map + ofs);\n+\tmerge->pseudo_merge_ofs = ofs;\n \n \treturn 0;\n }\n@@ -671,7 +671,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\toff_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63);\n \t\tuint32_t i;\n \n-\t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < -1) {\n+\t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < 0) {\n \t\t\twarning(_(\"could not read extended pseudo-merge table \"\n \t\t\t\t \"for commit %s\"),\n \t\t\t\toid_to_hex(&commit->object.oid));\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 5411fbf1e04..90459da5e63 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -549,7 +549,7 @@ test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n \t)\n '\n \n-test_expect_failure 'apply pseudo-merges with overlapping groups during fill-in' '\n+test_expect_success 'apply pseudo-merges with overlapping groups during fill-in' '\n \ttest_when_finished \"rm -fr pseudo-merge-fill-in-overlap\" &&\n \tgit init pseudo-merge-fill-in-overlap &&\n \t(\n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543110","messageId":"78cf7e6d80d0990f13a46515b12a6da342aa7e32.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 6/9] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:47:03Z","receivedAt":"2026-05-12T00:47:05Z","isPatch":true,"body":"`find_pseudo_merge_group_for_ref()` uses the commit's date to classify\nit as either \"stable\" (older than the stable threshold) or \"unstable\"\n(otherwise).\n\nHowever, to find the relevant commit from a given OID, the function\n`find_pseudo_merge_group_for_ref()` uses `lookup_commit()` which does\nnot parse commits.\n\nBecause an unparsed commit has its \"date\" set to zero, every candidate\nis placed in the \"stable\" bucket regardless of its actual committer\ntimestamp. This means the `bitmapPseudoMerge.*.threshold` and\n`stableThreshold` configuration options have no effect: the\nstable/unstable split is always determined by comparing against zero\nrather than the real commit date.\n\nThe net result is that pseudo-merge groups are partitioned by\n`stableSize` instead of the intended decay-based sizing, and the\n`sampleRate` knob (which only applies to the unstable path) is never\nexercised.\n\nFix this by calling `repo_parse_commit()` after `lookup_commit()`,\nbailing out of the callback if parsing fails.\n\nThe corresponding test configures two pseudo-merge groups that both\nmatch all tags. The \"stable\" group uses `threshold=1.month.ago`, and the\n\"all\" group uses `threshold=now`. The test use our custom\n\"GIT_TEST_DATE_NOW\" environment variable by setting it to the value of\n\"$test_tick\" to align Git's notion of \"now\" (and therefore\n\"1.month.ago\") with the `test_tick` timestamps, so the commits appear to\nbe younger than one month: only the \"all\" group matches them, producing\nexactly one pseudo-merge.\n\nWithout the fix every commit has `date == 0`, which satisfies `date <=\nthreshold` for both groups (since 0 is older than one month ago), and\nthe \"stable\" group erroneously matches as well.\n\nNow that commits are correctly classified as \"unstable\", the bug\ndescribed in the test exercising the \"sampleRate=0\" test is reachable,\nand the test is marked as failing. It will be fixed in a following\ncommit.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 2 ++\n t/t5333-pseudo-merge-bitmaps.sh | 22 ++++++++++++----------\n 2 files changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex 34e1da00b4e..d79e5fb649a 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -236,6 +236,8 @@ static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_d\n \tc = lookup_commit(the_repository, maybe_peeled);\n \tif (!c)\n \t\treturn 0;\n+\tif (repo_parse_commit(the_repository, c))\n+\t\treturn 0;\n \tif (!packlist_find(writer->to_pack, maybe_peeled))\n \t\treturn 0;\n \ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 90459da5e63..0032a16606b 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -592,32 +592,34 @@ test_expect_success 'apply pseudo-merges with overlapping groups during fill-in'\n \t)\n '\n \n-test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n+test_expect_success 'pseudo-merge commits are correctly classified by date' '\n \ttest_when_finished \"rm -fr pseudo-merge-date-classification\" &&\n \tgit init pseudo-merge-date-classification &&\n \t(\n \t\tcd pseudo-merge-date-classification &&\n \n \t\ttest_commit_bulk 64 &&\n+\n \t\ttag_everything &&\n \t\tgit repack -ad &&\n \n \t\tpack=\"$(ls .git/objects/pack/pack-*.pack)\" &&\n \n \t\t# Configure two pseudo-merge groups: one that only\n-\t\t# matches \"stable\" refs (older than one month), and one\n-\t\t# that matches all refs. With 64 freshly-created tags\n-\t\t# (all younger than one month) the stable group should\n-\t\t# have zero pseudo-merges and the catch-all group should\n-\t\t# have one.\n+\t\t# matches \"stable\" refs (older than one month), and\n+\t\t# one that matches all refs. With 64 tags whose\n+\t\t# commits are all younger than one month, the\n+\t\t# \"stable\" group should have zero pseudo-merges and\n+\t\t# the \"all\" group should have one.\n \t\t#\n \t\t# Use GIT_TEST_DATE_NOW to align \"now\" (and therefore\n \t\t# \"1.month.ago\") with the test_tick timestamps so that\n \t\t# the commits are within the last month.\n \t\t#\n-\t\t# This exercises the date-based classification in\n-\t\t# find_pseudo_merge_group_for_ref(), which requires\n-\t\t# that commits are parsed before inspecting their date.\n+\t\t# Without parsing the commit, its date field would\n+\t\t# be zero, causing it to satisfy date <= threshold\n+\t\t# for the \"stable\" group as well, and both groups\n+\t\t# would produce pseudo-merges.\n \t\tgit config bitmapPseudoMerge.stable.pattern \"refs/tags/\" &&\n \t\tgit config bitmapPseudoMerge.stable.maxMerges 64 &&\n \t\tgit config bitmapPseudoMerge.stable.stableThreshold never &&\n@@ -637,7 +639,7 @@ test_expect_failure 'pseudo-merge commits are correctly classified by date' '\n \t)\n '\n \n-test_expect_success 'sampleRate=0 does not cause division by zero' '\n+test_expect_failure 'sampleRate=0 does not cause division by zero' '\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n \tgit init pseudo-merge-sample-rate-zero &&\n \t(\n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543111","messageId":"4dbf6686718efba5a89e8c725fd91f6d418bf161.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 7/9] pack-bitmap: reject pseudo-merge \"sampleRate\" of 0","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:47:06Z","receivedAt":"2026-05-12T00:47:08Z","isPatch":true,"body":"The \"bitmapPseudoMerge.*.sampleRate\" configuration controls what\nfraction of unstable commits are included in each pseudo-merge group.\nThe config validation accepts values in the range `[0, 1]`, but a value\nof exactly 0 causes a division by zero in `select_pseudo_merges_1()`:\n\n if (j % (uint32_t)(1.0 / group->sample_rate))\n\nWhen `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting\ninfinity to `uint32_t` is undefined behavior in C. On most platforms\nthis yields 0, making the subsequent modulo operation (`j % 0`) a\nfatal arithmetic trap.\n\nThis path was not previously reachable because an earlier bug caused\nall pseudo-merge candidates to be classified as \"stable\" (where the\nsampling rate is not used), regardless of their actual commit date. Now\nthat the date classification is fixed, the unstable path is exercised\nand the division by zero can fire.\n\nFix this by changing the validation to require a strict lower bound and\nthus reject 0.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/config/bitmap-pseudo-merge.adoc | 4 ++--\n pseudo-merge.c | 4 ++--\n t/t5333-pseudo-merge-bitmaps.sh | 2 +-\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/bitmap-pseudo-merge.adoc b/Documentation/config/bitmap-pseudo-merge.adoc\nindex 1f264eca99b..6bf52c80ba7 100644\n--- a/Documentation/config/bitmap-pseudo-merge.adoc\n+++ b/Documentation/config/bitmap-pseudo-merge.adoc\n@@ -47,8 +47,8 @@ will be updated more often than a reference pointing at an old commit.\n bitmapPseudoMerge..sampleRate::\n \tDetermines the proportion of non-bitmapped commits (among\n \treference tips) which are selected for inclusion in an\n-\tunstable pseudo-merge bitmap. Must be between `0` and `1`\n-\t(inclusive). The default is `1`.\n+\tunstable pseudo-merge bitmap. Must be greater than `0` and\n+\tless than or equal to `1`. The default is `1`.\n \n bitmapPseudoMerge..threshold::\n \tDetermines the minimum age of non-bitmapped commits (among\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex d79e5fb649a..75bed043602 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -169,8 +169,8 @@ static int pseudo_merge_config(const char *var, const char *value,\n \t\t}\n \t} else if (!strcmp(key, \"samplerate\")) {\n \t\tgroup->sample_rate = git_config_double(var, value, ctx->kvi);\n-\t\tif (!(0 <= group->sample_rate && group->sample_rate <= 1)) {\n-\t\t\twarning(_(\"%s must be between 0 and 1, using default\"), var);\n+\t\tif (!(0 < group->sample_rate && group->sample_rate <= 1)) {\n+\t\t\twarning(_(\"%s must be between 0 (exclusive) and 1, using default\"), var);\n \t\t\tgroup->sample_rate = DEFAULT_PSEUDO_MERGE_SAMPLE_RATE;\n \t\t}\n \t} else if (!strcmp(key, \"threshold\")) {\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 0032a16606b..5bfbbd4214e 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -639,7 +639,7 @@ test_expect_success 'pseudo-merge commits are correctly classified by date' '\n \t)\n '\n \n-test_expect_failure 'sampleRate=0 does not cause division by zero' '\n+test_expect_success 'sampleRate=0 does not cause division by zero' '\n \ttest_when_finished \"rm -fr pseudo-merge-sample-rate-zero\" &&\n \tgit init pseudo-merge-sample-rate-zero &&\n \t(\n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543112","messageId":"46d0ee2f168a404d5f8832f60c0b2cc241dd0cfa.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 8/9] Documentation: fix broken `sampleRate` in gitpacking(7)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:47:09Z","receivedAt":"2026-05-12T00:47:11Z","isPatch":true,"body":"The documentation explaining some sample configurations for bitmap\npseudo-merges incorrectly uses a sample rate outside of the allowed\n(0,1] range.\n\nThis dates back to faf558b23ef (pseudo-merge: implement support for\nselecting pseudo-merge commits, 2024-05-23), and was likely written when\nthe allowable range for this configuration was the integral values\nbetween (0,100].\n\nFix this to conform to the actual allowable range for this\nconfiguration.\n\nNoticed-by: Elijah Newren \nSigned-off-by: Taylor Blau \n---\n Documentation/gitpacking.adoc | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/gitpacking.adoc b/Documentation/gitpacking.adoc\nindex a56596e2d1d..e6de6ec8249 100644\n--- a/Documentation/gitpacking.adoc\n+++ b/Documentation/gitpacking.adoc\n@@ -150,7 +150,7 @@ with a configuration like so:\n \tpattern = \"refs/\"\n \tthreshold = now\n \tstableThreshold = never\n-\tsampleRate = 100\n+\tsampleRate = 1\n \tmaxMerges = 64\n ----\n \n@@ -177,7 +177,7 @@ like:\n \tpattern = \"refs/virtual/([0-9]+)/(heads|tags)/\"\n \tthreshold = now\n \tstableThreshold = never\n-\tsampleRate = 100\n+\tsampleRate = 1\n \tmaxMerges = 64\n ----\n \n-- \n2.54.0.76.g9b17dab2cf7\n\n"},{"id":"543113","messageId":"9b17dab2cf745b78eae4c0da2cc2c0e79b7c0b3c.1778546804.git.me@ttaylorr.com","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"[PATCH v3 9/9] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:47:12Z","receivedAt":"2026-05-12T00:47:14Z","isPatch":true,"body":"When \"bitmapPseudoMerge.*.pattern\" appears more than once for the same\ngroup, `pseudo_merge_config()` frees the old `regex_t *` pointer\nbut does not call `regfree()` on it first. This leaks whatever internal\nstate `regcomp()` allocated.\n\nThe final cleanup path in `pseudo_merge_group_release()` does call\n`regfree()` before `free()`, so only the intermediate replacement is\naffected.\n\nFix this by guarding the replacement with a NULL check and calling\n`regfree()` before `free()` when the pointer is non-NULL.\n\nSigned-off-by: Taylor Blau \n---\n pseudo-merge.c | 5 ++++-\n t/t5333-pseudo-merge-bitmaps.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 33 insertions(+), 1 deletion(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex 75bed043602..22b8600d689 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -150,7 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,\n \tif (!strcmp(key, \"pattern\")) {\n \t\tstruct strbuf re = STRBUF_INIT;\n \n-\t\tfree(group->pattern);\n+\t\tif (group->pattern) {\n+\t\t\tregfree(group->pattern);\n+\t\t\tfree(group->pattern);\n+\t\t}\n \t\tif (*value != '^')\n \t\t\tstrbuf_addch(&re, '^');\n \t\tstrbuf_addstr(&re, value);\ndiff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh\nindex 5bfbbd4214e..305d6771082 100755\n--- a/t/t5333-pseudo-merge-bitmaps.sh\n+++ b/t/t5333-pseudo-merge-bitmaps.sh\n@@ -662,4 +662,33 @@ test_expect_success 'sampleRate=0 does not cause division by zero' '\n \t)\n '\n \n+test_expect_success 'duplicate pseudo-merge pattern does not leak' '\n+\ttest_when_finished \"rm -fr pseudo-merge-dup-pattern\" &&\n+\tgit init pseudo-merge-dup-pattern &&\n+\t(\n+\t\tcd pseudo-merge-dup-pattern &&\n+\n+\t\ttest_commit_bulk 64 &&\n+\t\ttag_everything &&\n+\t\tgit repack -ad &&\n+\n+\t\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\n+\t\t# Set the same group'\\''s pattern twice. The second\n+\t\t# assignment should cleanly release the compiled regex\n+\t\t# from the first without leaking.\n+\t\tgit config bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config --add bitmapPseudoMerge.test.pattern \"refs/tags/\" &&\n+\t\tgit config bitmapPseudoMerge.test.maxMerges 1 &&\n+\t\tgit config bitmapPseudoMerge.test.threshold now &&\n+\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n+\n+\t\tgit rev-parse HEAD~63 >in &&\n+\t\ttest-tool bitmap write \"$(basename $pack)\" merges &&\n+\t\ttest_line_count = 1 merges\n+\t)\n+'\n+\n test_done\n-- \n2.54.0.76.g9b17dab2cf7\n"},{"id":"543114","messageId":"agJ458pXAXTsIpKi@nand.local","threadId":"65478","inReplyTo":"xmqqpl32u06q.fsf@gitster.g","subject":"Re: [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T00:48:39Z","receivedAt":"2026-05-12T00:48:41Z","isPatch":true,"body":"On Mon, May 11, 2026 at 11:53:01AM +0900, Junio C Hamano wrote:\n> The topic went dormant after this comment, and it seems that it is\n> so close to the finish line otherwise? I'll leave the topic marked\n> as \"Expecting (hopefully minor and final) reroll\" in the draft\n> \"What's cooking\" report I work from for now.\n\nMy apologies. I had put this aside while you were on vacation, and then\ngot busy with GitHub-specific topics in the interim. I just sent a new\nversion that addresses Elijah's comments, which should be ready for\nmerging down.\n\nThanks,\nTaylor\n"},{"id":"543124","messageId":"xmqqse7xpftn.fsf@gitster.g","threadId":"65478","inReplyTo":"cover.1778546804.git.me@ttaylorr.com","subject":"Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-12T01:38:44Z","receivedAt":"2026-05-12T01:38:46Z","isPatch":true,"body":"Taylor Blau writes:\n\n> [Note to the maintainer: this series has been rebased onto the current\n> tip of master, which is 7760f83b597 (Merge branch\n> 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].\n\nA note like this is very much appreciated, but please also state the\nreason why the rebase was necessary. \"Because the current tip of\n'master' has advanced\" is not a good reason. \"The previous\nsynthetic base was made by merging topic X and topic Y on\nthen-current 'master', but both have graduated\" is a so-so ok\nreason. \"Because the updated implementation of this series uses\nfacilities that appeared in recent 'master' that come from topics A\nand B, which the previous iteration did not use\" and \"Recent updates\nto 'master' brings in conflicting changes from topic C\" are\nexcellent reasons.\n\n> Range-diff against v2:\n> 1: c0df35f8ebd = 1: 9c7a829cbeb t/helper: add 'test-tool bitmap write' subcommand\n> 2: 11de3343726 = 2: d1ed4aadf75 t5333: demonstrate various pseudo-merge bugs\n> 3: 8d908ab415e = 3: bf3a9a07e5f pack-bitmap-write: sort pseudo-merge commit lookup table in pack order\n> 4: 07f70a07c20 ! 4: a1d341c92eb pack-bitmap: fix inverted binary search in `pseudo_merge_at()`\n> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during\n> +test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '\n> \ttest_when_finished \"rm -fr pseudo-merge-fill-in-multi\" &&\n> \tgit init pseudo-merge-fill-in-multi &&\n> -+\tgit init pseudo-merge-fill-in-multi &&\n\nOK.\n\n> \t(\n> - \t\tcd pseudo-merge-fill-in-multi &&\n> - \n> 5: 3ed0b39843f = 5: 06e3410d323 pack-bitmap: fix pseudo-merge lookup for shared commits\n> 6: 95f847211f3 = 6: 78cf7e6d80d pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`\n> 7: f8a01cfb893 = 7: 4dbf6686718 pack-bitmap: reject pseudo-merge \"sampleRate\" of 0\n> 8: c37156502c0 = 8: 46d0ee2f168 Documentation: fix broken `sampleRate` in gitpacking(7)\n> 9: b905fd5d0ae ! 9: 9b17dab2cf7 pack-bitmap: prevent pattern leak on pseudo-merge re-assignment\n> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus\n> '\n> \n> +test_expect_success 'duplicate pseudo-merge pattern does not leak' '\n> -+\tgit init pseudo-merge-dup-pattern &&\n> +\ttest_when_finished \"rm -fr pseudo-merge-dup-pattern\" &&\n> -+\n> ++\tgit init pseudo-merge-dup-pattern &&\n> +\t(\n> +\t\tcd pseudo-merge-dup-pattern &&\n> +\n> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus\n> +\t\tgit config bitmapPseudoMerge.test.threshold now &&\n> +\t\tgit config bitmapPseudoMerge.test.stableThreshold never &&\n> +\n> -+\t\tgit rev-parse HEAD~63 |\n> -+\t\ttest-tool bitmap write \"$(basename $pack)\" &&\n> ++\t\tgit rev-parse HEAD~63 >in &&\n> ++\t\ttest-tool bitmap write \"$(basename $pack)\" +\n> +\t\ttest_pseudo_merges >merges &&\n> +\t\ttest_line_count = 1 merges\n>\n> base-commit: 7760f83b59750c27df653c5c46d0f80e44cfe02c\n\nQueued. Thanks.\n"},{"id":"543127","messageId":"agKGh/zv8RF/E/uB@nand.local","threadId":"65478","inReplyTo":"xmqqse7xpftn.fsf@gitster.g","subject":"Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-05-12T01:46:47Z","receivedAt":"2026-05-12T01:46:49Z","isPatch":true,"body":"On Tue, May 12, 2026 at 10:38:44AM +0900, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > [Note to the maintainer: this series has been rebased onto the current\n> > tip of master, which is 7760f83b597 (Merge branch\n> > 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].\n>\n> A note like this is very much appreciated, but please also state the\n> reason why the rebase was necessary. \"Because the current tip of\n> 'master' has advanced\" is not a good reason. \"The previous\n> synthetic base was made by merging topic X and topic Y on\n> then-current 'master', but both have graduated\" is a so-so ok\n> reason. \"Because the updated implementation of this series uses\n> facilities that appeared in recent 'master' that come from topics A\n> and B, which the previous iteration did not use\" and \"Recent updates\n> to 'master' brings in conflicting changes from topic C\" are\n> excellent reasons.\n\nI think the reason here was \"bad habit that I am trying to break\" ;-).\n\n(Joking aside, I usually rebase my series locally before sending to\nensure they can still be merged in cleanly. I usually remember to toss\nthat rebased version aside and send the non-rebased version, but clearly\nforgot to do so here. Sorry about that.)\n\nThanks for queueing regardless.\n\nThanks,\nTaylor\n"},{"id":"543128","messageId":"xmqqo6ilpfbf.fsf@gitster.g","threadId":"65478","inReplyTo":"xmqqse7xpftn.fsf@gitster.g","subject":"Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-12T01:49:40Z","receivedAt":"2026-05-12T01:49:42Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> Taylor Blau writes:\n>\n>> [Note to the maintainer: this series has been rebased onto the current\n>> tip of master, which is 7760f83b597 (Merge branch\n>> 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].\n>\n> A note like this is very much appreciated, but please also state the\n> reason why the rebase was necessary. \"Because the current tip of\n> 'master' has advanced\" is not a good reason. \"The previous\n> synthetic base was made by merging topic X and topic Y on\n> then-current 'master', but both have graduated\" is a so-so ok\n> reason. \"Because the updated implementation of this series uses\n> facilities that appeared in recent 'master' that come from topics A\n> and B, which the previous iteration did not use\" and \"Recent updates\n> to 'master' brings in conflicting changes from topic C\" are\n> excellent reasons.\n\nForgot one important case. \"It turns out that this fix is important\nso it was rebased to be applicable to an older maintenance release M\"\nwould be very much appreciated as well.\n\nPerhaps after coming up with a few more good reasons, we should\ndescribe them in Documentation/SubmittingPatches somewhere.\n\n"}]}