* Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Junio C Hamano @ 2026-05-12 1:49 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King, Elijah Newren
In-Reply-To: <xmqqse7xpftn.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Taylor Blau <me@ttaylorr.com> writes:
>
>> [Note to the maintainer: this series has been rebased onto the current
>> tip of master, which is 7760f83b597 (Merge branch
>> 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].
>
> A note like this is very much appreciated, but please also state the
> reason why the rebase was necessary. "Because the current tip of
> 'master' has advanced" is not a good reason. "The previous
> synthetic base was made by merging topic X and topic Y on
> then-current 'master', but both have graduated" is a so-so ok
> reason. "Because the updated implementation of this series uses
> facilities that appeared in recent 'master' that come from topics A
> and B, which the previous iteration did not use" and "Recent updates
> to 'master' brings in conflicting changes from topic C" are
> excellent reasons.
Forgot one important case. "It turns out that this fix is important
so it was rebased to be applicable to an older maintenance release M"
would be very much appreciated as well.
Perhaps after coming up with a few more good reasons, we should
describe them in Documentation/SubmittingPatches somewhere.
^ permalink raw reply
* Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Taylor Blau @ 2026-05-12 1:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Elijah Newren
In-Reply-To: <xmqqse7xpftn.fsf@gitster.g>
On Tue, May 12, 2026 at 10:38:44AM +0900, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > [Note to the maintainer: this series has been rebased onto the current
> > tip of master, which is 7760f83b597 (Merge branch
> > 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].
>
> A note like this is very much appreciated, but please also state the
> reason why the rebase was necessary. "Because the current tip of
> 'master' has advanced" is not a good reason. "The previous
> synthetic base was made by merging topic X and topic Y on
> then-current 'master', but both have graduated" is a so-so ok
> reason. "Because the updated implementation of this series uses
> facilities that appeared in recent 'master' that come from topics A
> and B, which the previous iteration did not use" and "Recent updates
> to 'master' brings in conflicting changes from topic C" are
> excellent reasons.
I think the reason here was "bad habit that I am trying to break" ;-).
(Joking aside, I usually rebase my series locally before sending to
ensure they can still be merged in cleanly. I usually remember to toss
that rebased version aside and send the non-rebased version, but clearly
forgot to do so here. Sorry about that.)
Thanks for queueing regardless.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 00/12] pack-objects: integrate --path-walk and some --filter options
From: Taylor Blau @ 2026-05-12 1:43 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <pull.2101.v3.git.1778523189.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:12:57PM +0000, Derrick Stolee via GitGitGadget wrote:
> Derrick Stolee (9):
> t5620: make test work with path-walk var
> pack-objects: pass --objects with --path-walk
> t/perf: add pack-objects filter and path-walk benchmark
> path-walk: always emit directly-requested objects
> path-walk: support blobless filter
> backfill: die on incompatible filter options
> path-walk: support blob size limit filter
> path-walk: add pl_sparse_trees to control tree pruning
> pack-objects: support sparse:oid filter with path-walk
>
> Taylor Blau (3):
> path-walk: support `tree:0` filter
> path-walk: support `object:type` filter
> path-walk: support `combine` filter
Thanks so much for putting this together! I'm really excited to see this
all moving along, and am eager to get this deployed at GitHub ;-).
I left a handful of notes throughout the series. There are a couple of
typos I noted that I think merit a reroll, but most of my remaining
comments are more for my own edification than identifying anything to
change within your series.
Thanks in advance for taking a look at them. I think that the next
reroll ought to be pretty close to the finish line.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 10/12] path-walk: support `tree:0` filter
From: Taylor Blau @ 2026-05-12 1:41 UTC (permalink / raw)
To: Taylor Blau via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <c9efff0ea8d72d2d084957e1b099801b8d8727f3.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:13:07PM +0000, Taylor Blau via GitGitGadget wrote:
> @@ -534,6 +545,18 @@ static int setup_pending_objects(struct path_walk_info *info,
> free(tagged_blobs);
> }
> }
> + if (tagged_trees) {
> + if (tagged_trees->oids.nr) {
> + const char *tagged_tree_path = "/tagged-trees";
> + tagged_trees->type = OBJ_TREE;
> + tagged_trees->maybe_interesting = 1;
> + strmap_put(&ctx->paths_to_lists, tagged_tree_path, tagged_trees);
> + push_to_stack(ctx, tagged_tree_path);
> + } else {
> + oid_array_clear(&tagged_trees->oids);
> + free(tagged_trees);
> + }
> + }
> if (tags) {
> if (tags->oids.nr) {
> const char *tag_path = "/tags";
It looks like there is some prior art here for enumerating a sentinel
path for "/tags", but I am curious why we did the same for
directly-listed trees in the presence of --filter=tree:0.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Junio C Hamano @ 2026-05-12 1:38 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> [Note to the maintainer: this series has been rebased onto the current
> tip of master, which is 7760f83b597 (Merge branch
> 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].
A note like this is very much appreciated, but please also state the
reason why the rebase was necessary. "Because the current tip of
'master' has advanced" is not a good reason. "The previous
synthetic base was made by merging topic X and topic Y on
then-current 'master', but both have graduated" is a so-so ok
reason. "Because the updated implementation of this series uses
facilities that appeared in recent 'master' that come from topics A
and B, which the previous iteration did not use" and "Recent updates
to 'master' brings in conflicting changes from topic C" are
excellent reasons.
> Range-diff against v2:
> 1: c0df35f8ebd = 1: 9c7a829cbeb t/helper: add 'test-tool bitmap write' subcommand
> 2: 11de3343726 = 2: d1ed4aadf75 t5333: demonstrate various pseudo-merge bugs
> 3: 8d908ab415e = 3: bf3a9a07e5f pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
> 4: 07f70a07c20 ! 4: a1d341c92eb pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during
> +test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '
> test_when_finished "rm -fr pseudo-merge-fill-in-multi" &&
> git init pseudo-merge-fill-in-multi &&
> -+ git init pseudo-merge-fill-in-multi &&
OK.
> (
> - cd pseudo-merge-fill-in-multi &&
> -
> 5: 3ed0b39843f = 5: 06e3410d323 pack-bitmap: fix pseudo-merge lookup for shared commits
> 6: 95f847211f3 = 6: 78cf7e6d80d pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
> 7: f8a01cfb893 = 7: 4dbf6686718 pack-bitmap: reject pseudo-merge "sampleRate" of 0
> 8: c37156502c0 = 8: 46d0ee2f168 Documentation: fix broken `sampleRate` in gitpacking(7)
> 9: b905fd5d0ae ! 9: 9b17dab2cf7 pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus
> '
>
> +test_expect_success 'duplicate pseudo-merge pattern does not leak' '
> -+ git init pseudo-merge-dup-pattern &&
> + test_when_finished "rm -fr pseudo-merge-dup-pattern" &&
> -+
> ++ git init pseudo-merge-dup-pattern &&
> + (
> + cd pseudo-merge-dup-pattern &&
> +
> @@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus
> + git config bitmapPseudoMerge.test.threshold now &&
> + git config bitmapPseudoMerge.test.stableThreshold never &&
> +
> -+ git rev-parse HEAD~63 |
> -+ test-tool bitmap write "$(basename $pack)" &&
> ++ git rev-parse HEAD~63 >in &&
> ++ test-tool bitmap write "$(basename $pack)" <in &&
> +
> + test_pseudo_merges >merges &&
> + test_line_count = 1 merges
>
> base-commit: 7760f83b59750c27df653c5c46d0f80e44cfe02c
Queued. Thanks.
^ permalink raw reply
* Re: [PATCH v3 07/12] path-walk: support blob size limit filter
From: Taylor Blau @ 2026-05-12 1:33 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <a1ab70405d6209a11b2eb752c154361060f1675d.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:13:04PM +0000, Derrick Stolee via GitGitGadget wrote:
> [...] Blobs whose size cannot be determined (e.g. missing in a partial
> clone) are conservatively included, matching the existing filter
> behavior.
Makes sense, but...
> Notice that this inspection of object sizes requires the content to be
> present in the repository. The odb_read_object_info() call will download
> a missing blob on-demand.
... this says that we do download missing blobs on-demand. Should we be
(temporarily) disabling 'fetch_if_missing' for this phase, or using
odb_read_object_info_extended() with the OBJECT_INFO_SKIP_FETCH_OBJECT
bit set?
I don't know enough about 'git backfill' to know whether the current
behavior is more reasonable than the above suggestion, so please let me
know if I'm missing something here!
The rest looks good to me.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 06/12] backfill: die on incompatible filter options
From: Taylor Blau @ 2026-05-12 1:26 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <da191e29e68bd46a56adea35ba1e7a41ff2a15c5.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:13:03PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> The 'git backfill' command uses the path-walk API in a critical way: it
> uses the objects output from the command to find the batches of missing
> objects that should be requested from the server. Unlike 'git
> pack-objects', we cannot fall back to another mechanism.
>
> The previous change added the path_walk_filter_compatible() method that
> we can reuse here. Use it during argument validation in cmd_backfill().
Makes sense.
> @@ -96,9 +96,8 @@ static void reject_unsupported_rev_list_options(struct rev_info *revs)
> if (revs->explicit_diff_merges)
> die(_("'%s' cannot be used with 'git backfill'"),
> "--diff-merges");
> - if (revs->filter.choice)
> - die(_("'%s' cannot be used with 'git backfill'"),
> - "--filter");
> + if (!path_walk_filter_compatible(&revs->filter))
> + die(_("cannot backfill with these filter options"));
I was going to suggest that we indicate the type of object filter which
was incompatible, but that gets a little tricky if the incompatible
filter is a child of a LOFC_COMBINE filter.
Resolving that does not seem worth our while, so I think that what you
wrote here is more than sufficient.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 04/12] path-walk: always emit directly-requested objects
From: Taylor Blau @ 2026-05-12 1:23 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <50933cccf508f71f47d955d5aceacad0db612fc5.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:13:01PM +0000, Derrick Stolee via GitGitGadget wrote:
> We can tell that a path is part of the directly-referenced objects if its
> path name starts with '/' (other paths, including root trees never have this
> starting character). Create a path_is_for_direct_objects() to make this
> meaning clear, especially as we add more references in the future as we
> integrate the path-walk API with partial clone filter options.
I don't know that I have anything in the way of a better suggestion, but
I can't help but feel like the `path_is_for_direct_objects()` check is
somewhat brittle as-is.
I am not familiar enough with the path-walk.c internals to come up with
a good suggestion off the top of my head, but I figured I'd raise it
here in case you had thoughts on alternatives.
> diff --git a/path-walk.c b/path-walk.c
> index 6e426af433..59a7670c5b 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -248,6 +248,16 @@ static int add_tree_entries(struct path_walk_context *ctx,
> return 0;
> }
>
> +/*
> + * Paths starting with '/' (e.g., "/tags", "/tagged-blobs") hold objects that
> + * were directly requested by 'pending' objects rather than discovered during
> + * tree traversal.
> + */
> +static int path_is_for_direct_objects(const char *path)
> +{
> + return path[0] == '/';
If we do end up keeping this approach, should we have a NULL check on
path itself here? I think that could even be an ASSERT(), since
something has gone wrong if we have a NULL at this point, but I'd rather
die by an assertion rather than a segfault here if so.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 2/2] run-command: honor "gc.auto" for auto-maintenance
From: Junio C Hamano @ 2026-05-12 1:21 UTC (permalink / raw)
To: Jeff King
Cc: Patrick Steinhardt, git, Jean-Christophe Manciot,
Mikael Magnusson, Taylor Blau, Derrick Stolee
In-Reply-To: <20260511201800.GC22912@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, May 11, 2026 at 02:29:56PM +0200, Patrick Steinhardt wrote:
>
>> @@ -1946,8 +1946,10 @@ int prepare_auto_maintenance(struct repository *r, int quiet,
>> {
>> int enabled, auto_detach;
>>
>> - if (!repo_config_get_bool(r, "maintenance.auto", &enabled) &&
>> - !enabled)
>> + if (repo_config_get_bool(r, "maintenance.auto", &enabled) &&
>> + repo_config_get_bool(r, "gc.auto", &enabled))
>> + enabled = 1;
>> + if (!enabled)
>> return 0;
>
> gc.auto isn't a bool; it's the count of loose objects after which to run
> maintenance. So "0" works in both contexts, but will we complain if
> gc.auto is set to 100? I think maybe not, because we fall back to
> git_parse_int(), but it feels kind of fragile.
>
> The gc code uses repo_config_get_int() here.
>
> -Peff
Very good point. I was about to send the same message ;-)
^ permalink raw reply
* Re: [PATCH 1/2] builtin/maintenance: fix locking with "--detach"
From: Junio C Hamano @ 2026-05-12 1:19 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Jean-Christophe Manciot, Mikael Magnusson, Jeff King,
Taylor Blau, Derrick Stolee
In-Reply-To: <20260511-pks-maintenance-fix-lock-with-detach-v1-1-ccd7d62c9a40@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 3a71e314c9..09cb92ac97 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1810,10 +1810,32 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
> TASK_PHASE_FOREGROUND))
> result = 1;
>
> - /* Failure to daemonize is ok, we'll continue in foreground. */
> if (opts->detach > 0) {
> + pid_t child_pid;
> +
> trace2_region_enter("maintenance", "detach", the_repository);
> - daemonize();
> +
> + child_pid = daemonize_without_exit();
> + if (!child_pid) {
> + /*
> + * We're in the child process, so we take ownership of
> + * the lockfile.
> + */
> + lock_file_reassign_owner(&lk, getpid());
> + } else if (child_pid > 0) {
> + /*
> + * We're in the parent process, so we assign ownership
> + * of the lockfile to the child and then exit immediately.
> + */
> + lock_file_reassign_owner(&lk, child_pid);
> + exit(0);
The point of reassigning the owner to somebody else is so that we
won't clean them when we exit as the tempfile.c::remove_tempfile()
function checks the "owner" is "me" and refrains from unlinking
those that do not belong to us, so there is nothing wrong in this
code, but this somehow felt awkward. In a sense, child_pid here
does not have to be what fork() returned but anything that is not
our own pid. Perhaps "we assign ... to the child" -> "we relinquish
... to prevent us removing upon exiting" would convey the intention
better? I dunno.
> -int daemonize(void)
> +pid_t daemonize_without_exit(void)
> {
> #ifdef NO_POSIX_GOODIES
> errno = ENOSYS;
> return -1;
> #else
> - switch (fork()) {
> - case 0:
> - break;
> - case -1:
> - die_errno(_("fork failed"));
> - default:
> - exit(0);
> - }
> + pid_t pid = fork();
> + if (pid < 0)
> + return -1;
> + if (pid > 0)
> + return pid;
> +
> if (setsid() == -1)
> die_errno(_("setsid failed"));
> close(0);
> @@ -2180,6 +2178,21 @@ int daemonize(void)
> #endif
> }
>
> +int daemonize(void)
> +{
> +#ifdef NO_POSIX_GOODIES
> + errno = ENOSYS;
> + return -1;
> +#else
> + pid_t pid = daemonize_without_exit();
> + if (pid < 0)
> + die_errno(_("fork failed"));
> + if (pid > 0)
> + exit(0);
> + return 0;
> +#endif
> +}
I was hoping that we can do without the #ifdef in this caller as
daemonize_without_exit() already has exactly the same condtional
compilation. If the NO_POSIX_GOODIES side can just return silently
wit ENOSYS, shouldn't the callers be also fine if we return failure
instead of calling die_errno(_("fork failed")), I have to wonder.
But because (1) as long as we have to call die_errno() here, we must
keep the conditional compilation in daemonize() as well as
daemonize_without_exit(), and (2) changing what the callers get when
fork failed here is totally outside of this topic, I would say that
the code around here is good as-is.
^ permalink raw reply
* Re: [PATCH v3 03/12] t/perf: add pack-objects filter and path-walk benchmark
From: Taylor Blau @ 2026-05-12 1:11 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <77329cf8f45b948f3986d004be72035c75ce40da.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:13:00PM +0000, Derrick Stolee via GitGitGadget wrote:
> ---
> t/perf/p5315-pack-objects-filter.sh | 129 ++++++++++++++++++++++++++++
> 1 file changed, 129 insertions(+)
> create mode 100755 t/perf/p5315-pack-objects-filter.sh
This is great, thank you for adding these as part of the series! :-)
> diff --git a/t/perf/p5315-pack-objects-filter.sh b/t/perf/p5315-pack-objects-filter.sh
> new file mode 100755
> index 0000000000..b009039c89
> --- /dev/null
> +++ b/t/perf/p5315-pack-objects-filter.sh
> @@ -0,0 +1,129 @@
> +#!/bin/sh
> +
> +test_description='Tests pack-objects performance with filters and --path-walk'
> +. ./perf-lib.sh
> +
> +test_perf_large_repo
> +
> +test_expect_success 'setup filter inputs' '
> + # Sample a few depth-2 directories from the test repo to build
> + # a cone-mode sparse-checkout definition. The sampling picks
> + # directories at evenly-spaced positions so the choice is stable
> + # and scales to repos of any shape.
> +
> + git ls-tree -d --name-only HEAD >top-dirs &&
> + top_nr=$(wc -l <top-dirs) &&
> +
> + >depth2-dirs &&
> + while read tdir
> + do
> + git ls-tree -d --name-only "HEAD:$tdir" 2>/dev/null |
> + sed "s|^|$tdir/|" >>depth2-dirs || return 1
> + done <top-dirs &&
Not a huge deal, but I think we should avoid having ls-tree on the
left-hand side of the pipe here. I would perhaps write this as:
while read tdir
do
git ls-tree -d --name-only "HEAD:$tdir" || return 1
done <top-dirs >depth2-dirs.raw &&
sed -e "s|^|$tdir/|" depth2-dirs.raw >depth2-dirs &&
(Moving sed outside of the main loop cuts down on the process count, but
isn't a huge deal. I think the main thing I wanted to suggest here was
avoiding 'git ls-tree ... | sed'.)
The rest of the test looks great to me.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 02/12] pack-objects: pass --objects with --path-walk
From: Taylor Blau @ 2026-05-12 1:04 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <5cc63833800d12b09ebce882ad3e25f61c9c1940.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:12:59PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> When 'git pack-objects' has the --path-walk option enabled, it uses a
> different set of revision walk parameters than normal. For once,
s/once/one/ ? Not sure.
> --objects was previously assumed by the path-walk API and was not needed
s/was not/did not/ ? Also not sure.
> ---
> builtin/pack-objects.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
Looks good.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 01/12] t5620: make test work with path-walk var
From: Taylor Blau @ 2026-05-12 1:03 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <0840110116a967d39d8b63dbfe822ea9b2500088.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:12:58PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> The GIT_TEST_PACK_PATH_WALK test variable allows enabling the
> --path-walk option to 'git pack-objects' by default. This sometimes
> engages the warning that --path-walk is incompatible with the --filter
> option. These tests in t5620 fail due to this warning over stderr in
> this case. Disable this variable for this moment until these options
> work together.
Makes sense. I think it would be equally OK to set this at the
top-level similar to c51f5a6437c (t5326: test multi-pack bitmap
behavior, 2021-08-31), but this is equally fine. Especially so if there
are other tests in the script that we do want to exercise under
--path-walk.
Looks good.
Thanks,
Taylor
^ permalink raw reply
* Re: What's cooking in git.git (May 2026, #02)
From: Taylor Blau @ 2026-05-12 0:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqq4iketzh0.fsf@gitster.g>
On Mon, May 11, 2026 at 12:08:27PM +0900, Junio C Hamano wrote:
> * tb/pseudo-merge-bugfixes (2026-04-21) 9 commits
>
> (snip)
>
> Expecting (hopefully minor and final) reroll.
> cf. <CABPp-BGkfavqezk2SV3+K6iF8MLm8j_=ijHiPDLmv_U_o_Ykgg@mail.gmail.com>
> source: <cover.1776801694.git.me@ttaylorr.com>
I just sent[1] a reroll of this one, which should be ready to go.
> * tb/incremental-midx-part-3.3 (2026-04-29) 16 commits
>
> (snip)
>
> Will merge to 'next'?
> source: <cover.1777507303.git.me@ttaylorr.com>
I think that this one is as ready as it will be. Peff reviewed v2[2] and
noted a couple of minor things which I addressed in v3. Peff's review of
v3 didn't note any further issues, so I think this is ready to go.
(+CC Peff here in case I'm misrepresenting his review.)
Thanks,
Taylor
[1]: <cover.1778546804.git.me@ttaylorr.com>
[1]: <20260429081030.GA1268740@coredump.intra.peff.net>
[2]: <20260501064640.GB2038915@coredump.intra.peff.net>
^ permalink raw reply
* Re: [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Taylor Blau @ 2026-05-12 0:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren, git, Jeff King
In-Reply-To: <xmqqpl32u06q.fsf@gitster.g>
On Mon, May 11, 2026 at 11:53:01AM +0900, Junio C Hamano wrote:
> The topic went dormant after this comment, and it seems that it is
> so close to the finish line otherwise? I'll leave the topic marked
> as "Expecting (hopefully minor and final) reroll" in the draft
> "What's cooking" report I work from for now.
My apologies. I had put this aside while you were on vacation, and then
got busy with GitHub-specific topics in the interim. I just sent a new
version that addresses Elijah's comments, which should be ready for
merging down.
Thanks,
Taylor
^ permalink raw reply
* [PATCH v3 9/9] pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
From: Taylor Blau @ 2026-05-12 0:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
When "bitmapPseudoMerge.*.pattern" appears more than once for the same
group, `pseudo_merge_config()` frees the old `regex_t *` pointer
but does not call `regfree()` on it first. This leaks whatever internal
state `regcomp()` allocated.
The final cleanup path in `pseudo_merge_group_release()` does call
`regfree()` before `free()`, so only the intermediate replacement is
affected.
Fix this by guarding the replacement with a NULL check and calling
`regfree()` before `free()` when the pointer is non-NULL.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pseudo-merge.c | 5 ++++-
t/t5333-pseudo-merge-bitmaps.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/pseudo-merge.c b/pseudo-merge.c
index 75bed043602..22b8600d689 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -150,7 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,
if (!strcmp(key, "pattern")) {
struct strbuf re = STRBUF_INIT;
- free(group->pattern);
+ if (group->pattern) {
+ regfree(group->pattern);
+ free(group->pattern);
+ }
if (*value != '^')
strbuf_addch(&re, '^');
strbuf_addstr(&re, value);
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 5bfbbd4214e..305d6771082 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -662,4 +662,33 @@ test_expect_success 'sampleRate=0 does not cause division by zero' '
)
'
+test_expect_success 'duplicate pseudo-merge pattern does not leak' '
+ test_when_finished "rm -fr pseudo-merge-dup-pattern" &&
+ git init pseudo-merge-dup-pattern &&
+ (
+ cd pseudo-merge-dup-pattern &&
+
+ test_commit_bulk 64 &&
+ tag_everything &&
+ git repack -ad &&
+
+ pack=$(ls .git/objects/pack/pack-*.pack) &&
+
+ # Set the same group'\''s pattern twice. The second
+ # assignment should cleanly release the compiled regex
+ # from the first without leaking.
+ git config bitmapPseudoMerge.test.pattern "refs/tags/" &&
+ git config --add bitmapPseudoMerge.test.pattern "refs/tags/" &&
+ git config bitmapPseudoMerge.test.maxMerges 1 &&
+ git config bitmapPseudoMerge.test.threshold now &&
+ git config bitmapPseudoMerge.test.stableThreshold never &&
+
+ git rev-parse HEAD~63 >in &&
+ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test_pseudo_merges >merges &&
+ test_line_count = 1 merges
+ )
+'
+
test_done
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 8/9] Documentation: fix broken `sampleRate` in gitpacking(7)
From: Taylor Blau @ 2026-05-12 0:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
The documentation explaining some sample configurations for bitmap
pseudo-merges incorrectly uses a sample rate outside of the allowed
(0,1] range.
This dates back to faf558b23ef (pseudo-merge: implement support for
selecting pseudo-merge commits, 2024-05-23), and was likely written when
the allowable range for this configuration was the integral values
between (0,100].
Fix this to conform to the actual allowable range for this
configuration.
Noticed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/gitpacking.adoc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitpacking.adoc b/Documentation/gitpacking.adoc
index a56596e2d1d..e6de6ec8249 100644
--- a/Documentation/gitpacking.adoc
+++ b/Documentation/gitpacking.adoc
@@ -150,7 +150,7 @@ with a configuration like so:
pattern = "refs/"
threshold = now
stableThreshold = never
- sampleRate = 100
+ sampleRate = 1
maxMerges = 64
----
@@ -177,7 +177,7 @@ like:
pattern = "refs/virtual/([0-9]+)/(heads|tags)/"
threshold = now
stableThreshold = never
- sampleRate = 100
+ sampleRate = 1
maxMerges = 64
----
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 7/9] pack-bitmap: reject pseudo-merge "sampleRate" of 0
From: Taylor Blau @ 2026-05-12 0:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
The "bitmapPseudoMerge.*.sampleRate" configuration controls what
fraction of unstable commits are included in each pseudo-merge group.
The config validation accepts values in the range `[0, 1]`, but a value
of exactly 0 causes a division by zero in `select_pseudo_merges_1()`:
if (j % (uint32_t)(1.0 / group->sample_rate))
When `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting
infinity to `uint32_t` is undefined behavior in C. On most platforms
this yields 0, making the subsequent modulo operation (`j % 0`) a
fatal arithmetic trap.
This path was not previously reachable because an earlier bug caused
all pseudo-merge candidates to be classified as "stable" (where the
sampling rate is not used), regardless of their actual commit date. Now
that the date classification is fixed, the unstable path is exercised
and the division by zero can fire.
Fix this by changing the validation to require a strict lower bound and
thus reject 0.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/config/bitmap-pseudo-merge.adoc | 4 ++--
pseudo-merge.c | 4 ++--
t/t5333-pseudo-merge-bitmaps.sh | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/config/bitmap-pseudo-merge.adoc b/Documentation/config/bitmap-pseudo-merge.adoc
index 1f264eca99b..6bf52c80ba7 100644
--- a/Documentation/config/bitmap-pseudo-merge.adoc
+++ b/Documentation/config/bitmap-pseudo-merge.adoc
@@ -47,8 +47,8 @@ will be updated more often than a reference pointing at an old commit.
bitmapPseudoMerge.<name>.sampleRate::
Determines the proportion of non-bitmapped commits (among
reference tips) which are selected for inclusion in an
- unstable pseudo-merge bitmap. Must be between `0` and `1`
- (inclusive). The default is `1`.
+ unstable pseudo-merge bitmap. Must be greater than `0` and
+ less than or equal to `1`. The default is `1`.
bitmapPseudoMerge.<name>.threshold::
Determines the minimum age of non-bitmapped commits (among
diff --git a/pseudo-merge.c b/pseudo-merge.c
index d79e5fb649a..75bed043602 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -169,8 +169,8 @@ static int pseudo_merge_config(const char *var, const char *value,
}
} else if (!strcmp(key, "samplerate")) {
group->sample_rate = git_config_double(var, value, ctx->kvi);
- if (!(0 <= group->sample_rate && group->sample_rate <= 1)) {
- warning(_("%s must be between 0 and 1, using default"), var);
+ if (!(0 < group->sample_rate && group->sample_rate <= 1)) {
+ warning(_("%s must be between 0 (exclusive) and 1, using default"), var);
group->sample_rate = DEFAULT_PSEUDO_MERGE_SAMPLE_RATE;
}
} else if (!strcmp(key, "threshold")) {
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 0032a16606b..5bfbbd4214e 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -639,7 +639,7 @@ test_expect_success 'pseudo-merge commits are correctly classified by date' '
)
'
-test_expect_failure 'sampleRate=0 does not cause division by zero' '
+test_expect_success 'sampleRate=0 does not cause division by zero' '
test_when_finished "rm -fr pseudo-merge-sample-rate-zero" &&
git init pseudo-merge-sample-rate-zero &&
(
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 6/9] pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
From: Taylor Blau @ 2026-05-12 0:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
`find_pseudo_merge_group_for_ref()` uses the commit's date to classify
it as either "stable" (older than the stable threshold) or "unstable"
(otherwise).
However, to find the relevant commit from a given OID, the function
`find_pseudo_merge_group_for_ref()` uses `lookup_commit()` which does
not parse commits.
Because an unparsed commit has its "date" set to zero, every candidate
is placed in the "stable" bucket regardless of its actual committer
timestamp. This means the `bitmapPseudoMerge.*.threshold` and
`stableThreshold` configuration options have no effect: the
stable/unstable split is always determined by comparing against zero
rather than the real commit date.
The net result is that pseudo-merge groups are partitioned by
`stableSize` instead of the intended decay-based sizing, and the
`sampleRate` knob (which only applies to the unstable path) is never
exercised.
Fix this by calling `repo_parse_commit()` after `lookup_commit()`,
bailing out of the callback if parsing fails.
The corresponding test configures two pseudo-merge groups that both
match all tags. The "stable" group uses `threshold=1.month.ago`, and the
"all" group uses `threshold=now`. The test use our custom
"GIT_TEST_DATE_NOW" environment variable by setting it to the value of
"$test_tick" to align Git's notion of "now" (and therefore
"1.month.ago") with the `test_tick` timestamps, so the commits appear to
be younger than one month: only the "all" group matches them, producing
exactly one pseudo-merge.
Without the fix every commit has `date == 0`, which satisfies `date <=
threshold` for both groups (since 0 is older than one month ago), and
the "stable" group erroneously matches as well.
Now that commits are correctly classified as "unstable", the bug
described in the test exercising the "sampleRate=0" test is reachable,
and the test is marked as failing. It will be fixed in a following
commit.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pseudo-merge.c | 2 ++
t/t5333-pseudo-merge-bitmaps.sh | 22 ++++++++++++----------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/pseudo-merge.c b/pseudo-merge.c
index 34e1da00b4e..d79e5fb649a 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -236,6 +236,8 @@ static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_d
c = lookup_commit(the_repository, maybe_peeled);
if (!c)
return 0;
+ if (repo_parse_commit(the_repository, c))
+ return 0;
if (!packlist_find(writer->to_pack, maybe_peeled))
return 0;
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 90459da5e63..0032a16606b 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -592,32 +592,34 @@ test_expect_success 'apply pseudo-merges with overlapping groups during fill-in'
)
'
-test_expect_failure 'pseudo-merge commits are correctly classified by date' '
+test_expect_success 'pseudo-merge commits are correctly classified by date' '
test_when_finished "rm -fr pseudo-merge-date-classification" &&
git init pseudo-merge-date-classification &&
(
cd pseudo-merge-date-classification &&
test_commit_bulk 64 &&
+
tag_everything &&
git repack -ad &&
pack="$(ls .git/objects/pack/pack-*.pack)" &&
# Configure two pseudo-merge groups: one that only
- # matches "stable" refs (older than one month), and one
- # that matches all refs. With 64 freshly-created tags
- # (all younger than one month) the stable group should
- # have zero pseudo-merges and the catch-all group should
- # have one.
+ # matches "stable" refs (older than one month), and
+ # one that matches all refs. With 64 tags whose
+ # commits are all younger than one month, the
+ # "stable" group should have zero pseudo-merges and
+ # the "all" group should have one.
#
# Use GIT_TEST_DATE_NOW to align "now" (and therefore
# "1.month.ago") with the test_tick timestamps so that
# the commits are within the last month.
#
- # This exercises the date-based classification in
- # find_pseudo_merge_group_for_ref(), which requires
- # that commits are parsed before inspecting their date.
+ # Without parsing the commit, its date field would
+ # be zero, causing it to satisfy date <= threshold
+ # for the "stable" group as well, and both groups
+ # would produce pseudo-merges.
git config bitmapPseudoMerge.stable.pattern "refs/tags/" &&
git config bitmapPseudoMerge.stable.maxMerges 64 &&
git config bitmapPseudoMerge.stable.stableThreshold never &&
@@ -637,7 +639,7 @@ test_expect_failure 'pseudo-merge commits are correctly classified by date' '
)
'
-test_expect_success 'sampleRate=0 does not cause division by zero' '
+test_expect_failure 'sampleRate=0 does not cause division by zero' '
test_when_finished "rm -fr pseudo-merge-sample-rate-zero" &&
git init pseudo-merge-sample-rate-zero &&
(
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 5/9] pack-bitmap: fix pseudo-merge lookup for shared commits
From: Taylor Blau @ 2026-05-12 0:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
When a commit appears in more than one pseudo-merge group, its entry in
the commit lookup table has the high bit set in its offset field,
indicating that the offset points to an "extended" table containing the
set of pseudo-merges for that commit.
There are three bugs in this path:
* The `next_ext` offset in `write_pseudo_merges()` undercounts the
per-entry size of the lookup table (8 vs. 12 bytes).
* `nth_pseudo_merge_ext()` calls `read_pseudo_merge_commit_at()` on a
pseudo-merge bitmap offset, misinterpreting it as a 12-byte commit
table entry.
* The error check after `pseudo_merge_ext_at()` in
`apply_pseudo_merges_for_commit()` tests `< -1` instead of `< 0`,
silently swallowing errors from `error()`.
The first bug is on the write side: each commit lookup entry contains a
4- and 8-byte unsigned value for a total of 12 bytes, but the
calculation assumes that the entry only contains 8 bytes of data. This
makes `next_ext` too small, so the extended-table offsets that get
written point into the middle of the non-extended lookup table rather
than past it. The reader then interprets non-extended lookup data as
extended entries, producing garbage.
The second bug is on the read side and is independently fatal: even with
a correctly positioned extended table, `nth_pseudo_merge_ext()` feeds
the offset it reads (which points at pseudo-merge bitmap data) to
`read_pseudo_merge_commit_at()`. That function tries to parse 12 bytes
as a `pseudo_merge_commit` struct, clobbering `merge->pseudo_merge_ofs`
with whatever happens to be at that location. The caller only needs
`pseudo_merge_ofs`, so the fix is to store the offset directly rather
than re-parsing a commit table entry. The `commit_pos` field is left
untouched, retaining the value that `find_pseudo_merge()` set earlier.
The third bug is latent. With the first two fixes applied, the extended
table is correctly written and read, so `pseudo_merge_ext_at()` does not
fail during normal operation. The `< -1` vs `< 0` distinction only
matters when the bitmap file is corrupt or truncated, in which case the
error would be silently ignored and the code would proceed with
uninitialized data.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 2 +-
pseudo-merge.c | 4 ++--
t/t5333-pseudo-merge-bitmaps.sh | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 86ed6a5d78c..1c8070f99c0 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -877,7 +877,7 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
next_ext = st_add(hashfile_total(f),
st_mult(kh_size(writer->pseudo_merge_commits),
- sizeof(uint64_t)));
+ sizeof(uint32_t) + sizeof(uint64_t)));
table_start = hashfile_total(f);
diff --git a/pseudo-merge.c b/pseudo-merge.c
index fb71c761792..34e1da00b4e 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -600,7 +600,7 @@ static int nth_pseudo_merge_ext(const struct pseudo_merge_map *pm,
return error(_("out-of-bounds read: (%"PRIuMAX" >= %"PRIuMAX")"),
(uintmax_t)ofs, (uintmax_t)pm->map_size);
- read_pseudo_merge_commit_at(merge, pm->map + ofs);
+ merge->pseudo_merge_ofs = ofs;
return 0;
}
@@ -671,7 +671,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,
off_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63);
uint32_t i;
- if (pseudo_merge_ext_at(pm, &ext, ofs) < -1) {
+ if (pseudo_merge_ext_at(pm, &ext, ofs) < 0) {
warning(_("could not read extended pseudo-merge table "
"for commit %s"),
oid_to_hex(&commit->object.oid));
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 5411fbf1e04..90459da5e63 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -549,7 +549,7 @@ test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '
)
'
-test_expect_failure 'apply pseudo-merges with overlapping groups during fill-in' '
+test_expect_success 'apply pseudo-merges with overlapping groups during fill-in' '
test_when_finished "rm -fr pseudo-merge-fill-in-overlap" &&
git init pseudo-merge-fill-in-overlap &&
(
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 4/9] pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
From: Taylor Blau @ 2026-05-12 0:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
The binary search in `pseudo_merge_at()` has its "lo" and "hi" updates
swapped: when the midpoint's offset is less than the target, it sets `hi
= mi` (searching left) instead of `lo = mi + 1` (searching right), and
vice versa.
This means that lookups for pseudo-merges whose offset is not near the
midpoint of the pseudo-merge table are likely to fail.
In practice, with a single pseudo-merge group this is masked because the
lone entry is always at the midpoint. With multiple groups, the inverted
comparisons cause lookups to search in the wrong direction, potentially
missing entries.
Swap the "lo" and "hi" assignments to search in the correct direction,
making it possible to apply pseudo-merges during fill-in when more than
one pseudo-merge exists in a group.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pseudo-merge.c | 4 ++--
t/t5333-pseudo-merge-bitmaps.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/pseudo-merge.c b/pseudo-merge.c
index ff18b6c3642..fb71c761792 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -559,9 +559,9 @@ static struct pseudo_merge *pseudo_merge_at(const struct pseudo_merge_map *pm,
if (got == want)
return use_pseudo_merge(pm, &pm->v[mi]);
else if (got < want)
- hi = mi;
- else
lo = mi + 1;
+ else
+ hi = mi;
}
warning(_("could not find pseudo-merge for commit %s at offset %"PRIuMAX),
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 3d7a7668121..5411fbf1e04 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -496,7 +496,7 @@ test_expect_success 'apply pseudo-merges during fill-in traversal' '
)
'
-test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '
+test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '
test_when_finished "rm -fr pseudo-merge-fill-in-multi" &&
git init pseudo-merge-fill-in-multi &&
(
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 3/9] pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
From: Taylor Blau @ 2026-05-12 0:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
The pseudo-merge commit lookup table stores each commit's position in
the pack- or pseudo-pack order, and is used to perform a binary search
in order to determine which pseudo-merge(s) a given commit belongs to.
However, the table was previously sorted in lexical order (via
`oid_array_sort()`), causing the binary search to fail.
While this causes pseudo-merge bitmaps to be de-facto broken for fill-in
traversal, there are a couple of important points to keep in mind:
* Pseudo-merge application during the initial phases of a bitmap-based
traversal are applied via `cascade_pseudo_merges_1()`. This function
enumerates the known pseudo-merges and determines if its parents are
a subset of the traversal roots.
This is a different path than the fill-in traversal, where we are
looking for any pseudo-merges which may be satisfied after visiting
some commit along an object walk, which involves the aforementioned
(broken) binary search.
As a consequence, any pseudo-merges we apply at this stage are done
so correctly.
* While this bug makes applying pseudo-merges during fill-in traversal
effectively broken, it does not produce wrong results. Instead of
applying the *wrong* pseudo-merge, we will simply fail to find
satisfied pseudo-merges, leaving the traversal to use the existing
fill-in routines.
Fix this by sorting the table by bit position before writing, matching
the order that the reader's binary search expects.
This does produce a change the on-disk format insofar as the actual code
now complies with the documented format (for more details, refer to:
Documentation/technical/bitmap-format.adoc). Given that this never
worked in the first place, such a change should be OK to perform.
If an out-of-tree implementation of pseudo-merges happened to generate
bitmaps that comply with the documented format, they will continue to be
read and interpreted as normal.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 21 ++++++++++++++++++++-
t/t5333-pseudo-merge-bitmaps.sh | 2 +-
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 8338d7217ef..86ed6a5d78c 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -819,6 +819,20 @@ static void write_selected_commits_v1(struct bitmap_writer *writer,
}
}
+static int pseudo_merge_commit_pos_cmp(const void *_va, const void *_vb,
+ void *_data)
+{
+ struct bitmap_writer *writer = _data;
+ uint32_t pos_a = find_object_pos(writer, _va, NULL);
+ uint32_t pos_b = find_object_pos(writer, _vb, NULL);
+
+ if (pos_a < pos_b)
+ return -1;
+ if (pos_a > pos_b)
+ return 1;
+ return 0;
+}
+
static void write_pseudo_merges(struct bitmap_writer *writer,
struct hashfile *f)
{
@@ -876,7 +890,12 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
oid_array_append(&commits, &kh_key(writer->pseudo_merge_commits, i));
}
- oid_array_sort(&commits);
+ /*
+ * Sort the commits by their bit position so that the lookup
+ * table can be binary searched by the reader (see
+ * find_pseudo_merge()).
+ */
+ QSORT_S(commits.oid, commits.nr, pseudo_merge_commit_pos_cmp, writer);
/* write lookup table (non-extended) */
for (i = 0; i < commits.nr; i++) {
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 0e9638c31c3..3d7a7668121 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -462,7 +462,7 @@ test_expect_success 'use pseudo-merge in boundary traversal' '
)
'
-test_expect_failure 'apply pseudo-merges during fill-in traversal' '
+test_expect_success 'apply pseudo-merges during fill-in traversal' '
test_when_finished "rm -fr pseudo-merge-fill-in-traversal" &&
git init pseudo-merge-fill-in-traversal &&
(
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 2/9] t5333: demonstrate various pseudo-merge bugs
From: Taylor Blau @ 2026-05-12 0:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
Using the test helper introduced via the previous commit, add various
failing tests demonstrating bugs in the pseudo-merge implementation.
These are all marked as failing with one exception. The "sampleRate=0"
test describes a latent bug, which is only reachable through a code path
that is itself masked by a separate bug. A future commit will fix that
bug, and, in turn, cause the aforementioned test to fail. Accordingly,
that commit will mark the test as failing, and it will be re-marked as
passing in a separate commit which fixes the once-latent bug.
For the rest: the following commits will explain and fix the underlying
bugs in detail.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/t5333-pseudo-merge-bitmaps.sh | 198 ++++++++++++++++++++++++++++++++
1 file changed, 198 insertions(+)
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 1f7a5d82ee4..0e9638c31c3 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -462,4 +462,202 @@ test_expect_success 'use pseudo-merge in boundary traversal' '
)
'
+test_expect_failure 'apply pseudo-merges during fill-in traversal' '
+ test_when_finished "rm -fr pseudo-merge-fill-in-traversal" &&
+ git init pseudo-merge-fill-in-traversal &&
+ (
+ cd pseudo-merge-fill-in-traversal &&
+
+ git config bitmapPseudoMerge.test.pattern refs/tags/ &&
+ git config bitmapPseudoMerge.test.maxMerges 1 &&
+ git config bitmapPseudoMerge.test.stableThreshold never &&
+
+ test_commit_bulk 64 &&
+ tag_everything &&
+ git repack -ad &&
+
+ pack=$(ls .git/objects/pack/pack-*.pack) &&
+ git rev-parse HEAD~63 >in &&
+ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test_pseudo_merges >merges &&
+ test_line_count = 1 merges &&
+
+ test_commit stale &&
+
+ git rev-list --count --objects HEAD >expect &&
+
+ : >trace2.txt &&
+ GIT_TRACE2_EVENT=$PWD/trace2.txt \
+ git rev-list --count --objects --use-bitmap-index HEAD >actual &&
+ test_pseudo_merges_satisfied 1 <trace2.txt &&
+
+ test_cmp expect actual
+ )
+'
+
+test_expect_failure 'apply pseudo-merges from multiple groups during fill-in' '
+ test_when_finished "rm -fr pseudo-merge-fill-in-multi" &&
+ git init pseudo-merge-fill-in-multi &&
+ (
+ cd pseudo-merge-fill-in-multi &&
+
+ test_commit base &&
+ base=$(git rev-parse HEAD) &&
+
+ for side in left right
+ do
+ git checkout -B $side base &&
+
+ test_commit_bulk --id=$side 64 &&
+ git rev-list --no-object-names HEAD --not $base >in &&
+ while read oid
+ do
+ echo "create refs/group-$side/$oid $oid" || return 1
+ done <in | git update-ref --stdin || return 1
+ done &&
+
+ git checkout left &&
+ git merge right &&
+ git repack -ad &&
+
+ git config bitmapPseudoMerge.left.pattern "refs/group-left/" &&
+ git config bitmapPseudoMerge.left.maxMerges 1 &&
+ git config bitmapPseudoMerge.left.stableThreshold never &&
+
+ git config bitmapPseudoMerge.right.pattern "refs/group-right/" &&
+ git config bitmapPseudoMerge.right.maxMerges 1 &&
+ git config bitmapPseudoMerge.right.stableThreshold never &&
+
+ pack="$(ls .git/objects/pack/pack-*.pack)" &&
+ git rev-parse "$base" >in &&
+ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test_pseudo_merges >merges &&
+ test_line_count = 2 merges &&
+
+ test_commit stale &&
+
+ git rev-list --count --objects HEAD >expect &&
+
+ : >trace2.txt &&
+ GIT_TRACE2_EVENT=$PWD/trace2.txt \
+ git rev-list --count --objects --use-bitmap-index HEAD >actual &&
+ test_pseudo_merges_satisfied 2 <trace2.txt &&
+
+ test_cmp expect actual
+ )
+'
+
+test_expect_failure 'apply pseudo-merges with overlapping groups during fill-in' '
+ test_when_finished "rm -fr pseudo-merge-fill-in-overlap" &&
+ git init pseudo-merge-fill-in-overlap &&
+ (
+ cd pseudo-merge-fill-in-overlap &&
+
+ test_commit_bulk 64 &&
+ tag_everything &&
+ git repack -ad &&
+
+ pack="$(ls .git/objects/pack/pack-*.pack)" &&
+
+ # Use two pseudo-merge group patterns that both match
+ # refs/tags/, so every tagged commit belongs to both
+ # groups. This exercises the extended lookup table
+ # path in apply_pseudo_merges_for_commit().
+ git config bitmapPseudoMerge.all.pattern "refs/tags/" &&
+ git config bitmapPseudoMerge.all.maxMerges 1 &&
+ git config bitmapPseudoMerge.all.stableThreshold never &&
+
+ git config bitmapPseudoMerge.tags.pattern "refs/tags/" &&
+ git config bitmapPseudoMerge.tags.maxMerges 1 &&
+ git config bitmapPseudoMerge.tags.stableThreshold never &&
+
+ git rev-parse HEAD~63 >in &&
+ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test_pseudo_merges >merges &&
+ test_line_count = 2 merges &&
+
+ test_commit stale &&
+
+ git rev-list --count --objects HEAD >expect &&
+
+ : >trace2.txt &&
+ GIT_TRACE2_EVENT=$PWD/trace2.txt \
+ git rev-list --count --objects --use-bitmap-index HEAD >actual &&
+ test_pseudo_merges_satisfied 2 <trace2.txt &&
+
+ test_cmp expect actual
+ )
+'
+
+test_expect_failure 'pseudo-merge commits are correctly classified by date' '
+ test_when_finished "rm -fr pseudo-merge-date-classification" &&
+ git init pseudo-merge-date-classification &&
+ (
+ cd pseudo-merge-date-classification &&
+
+ test_commit_bulk 64 &&
+ tag_everything &&
+ git repack -ad &&
+
+ pack="$(ls .git/objects/pack/pack-*.pack)" &&
+
+ # Configure two pseudo-merge groups: one that only
+ # matches "stable" refs (older than one month), and one
+ # that matches all refs. With 64 freshly-created tags
+ # (all younger than one month) the stable group should
+ # have zero pseudo-merges and the catch-all group should
+ # have one.
+ #
+ # Use GIT_TEST_DATE_NOW to align "now" (and therefore
+ # "1.month.ago") with the test_tick timestamps so that
+ # the commits are within the last month.
+ #
+ # This exercises the date-based classification in
+ # find_pseudo_merge_group_for_ref(), which requires
+ # that commits are parsed before inspecting their date.
+ git config bitmapPseudoMerge.stable.pattern "refs/tags/" &&
+ git config bitmapPseudoMerge.stable.maxMerges 64 &&
+ git config bitmapPseudoMerge.stable.stableThreshold never &&
+ git config bitmapPseudoMerge.stable.threshold 1.month.ago &&
+
+ git config bitmapPseudoMerge.all.pattern "refs/tags/" &&
+ git config bitmapPseudoMerge.all.maxMerges 1 &&
+ git config bitmapPseudoMerge.all.stableThreshold never &&
+ git config bitmapPseudoMerge.all.threshold now &&
+
+ git rev-parse HEAD~63 >in &&
+ GIT_TEST_DATE_NOW=$test_tick \
+ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test_pseudo_merges >merges &&
+ test_line_count = 1 merges
+ )
+'
+
+test_expect_success 'sampleRate=0 does not cause division by zero' '
+ test_when_finished "rm -fr pseudo-merge-sample-rate-zero" &&
+ git init pseudo-merge-sample-rate-zero &&
+ (
+ cd pseudo-merge-sample-rate-zero &&
+
+ test_commit_bulk 64 &&
+ tag_everything &&
+ git repack -ad &&
+
+ pack="$(ls .git/objects/pack/pack-*.pack)" &&
+
+ git config bitmapPseudoMerge.test.pattern "refs/tags/" &&
+ git config bitmapPseudoMerge.test.maxMerges 1 &&
+ git config bitmapPseudoMerge.test.sampleRate 0 &&
+ git config bitmapPseudoMerge.test.threshold now &&
+ git config bitmapPseudoMerge.test.stableThreshold never &&
+
+ git rev-parse HEAD~63 >in &&
+ test-tool bitmap write "$(basename $pack)" <in
+ )
+'
+
test_done
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 1/9] t/helper: add 'test-tool bitmap write' subcommand
From: Taylor Blau @ 2026-05-12 0:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1778546804.git.me@ttaylorr.com>
In f16eb1c091 (pseudo-merge: fix disk reads from find_pseudo_merge(),
2026-03-31), we noted that `apply_pseudo_merges_for_commit()` is never
triggered by the existing test suite, and that this bears further
investigation.
This patch is the first one to begin that investigation. The following
patches will expose and fix a variety of bugs in the implementation of
pseudo-merge bitmaps.
In order to do so, however, many of these tests require very precise
selection of which commits receive bitmaps and which do not. To date,
there isn't a standard approach to easily facilitate this. Address this
by introducing a `test-tool bitmap write` subcommand that writes a
bitmap for a given packfile, reading the set of commits which should
receive individual bitmaps from stdin like so:
test-tool bitmap write <pack-basename> </path/to/commits.list
, where "<pack-basename>" is the filename for a specific packfile (e.g.,
"pack-abc123.pack"), and "/path/to/commits.list" is a list of commit
OIDs which will receive bitmaps.
The helper respects `bitmapPseudoMerge.*` configuration for creating
pseudo-merge bitmaps alongside the regular commit bitmaps.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/helper/test-bitmap.c | 113 +++++++++++++++++++++++++++++++++++++++-
t/t5310-pack-bitmaps.sh | 24 +++++++++
2 files changed, 136 insertions(+), 1 deletion(-)
diff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c
index 16a01669e41..381e9b58b2c 100644
--- a/t/helper/test-bitmap.c
+++ b/t/helper/test-bitmap.c
@@ -2,7 +2,10 @@
#include "test-tool.h"
#include "git-compat-util.h"
+#include "hex.h"
+#include "odb.h"
#include "pack-bitmap.h"
+#include "pseudo-merge.h"
#include "setup.h"
static int bitmap_list_commits(void)
@@ -35,6 +38,111 @@ static int bitmap_dump_pseudo_merge_objects(uint32_t n)
return test_bitmap_pseudo_merge_objects(the_repository, n);
}
+static int add_packed_object(const struct object_id *oid,
+ struct packed_git *pack,
+ uint32_t pos,
+ void *_data)
+{
+ struct packing_data *packed = _data;
+ struct object_entry *entry;
+ struct object_info oi = OBJECT_INFO_INIT;
+ enum object_type type;
+
+ oi.typep = &type;
+
+ entry = packlist_alloc(packed, oid);
+ entry->idx.offset = nth_packed_object_offset(pack, pos);
+ if (packed_object_info(pack, entry->idx.offset, &oi) < 0)
+ die("could not get type of object %s",
+ oid_to_hex(oid));
+ oe_set_type(entry, type);
+ oe_set_in_pack(packed, entry, pack);
+
+ return 0;
+}
+
+static int idx_oid_cmp(const void *va, const void *vb)
+{
+ const struct pack_idx_entry *a = *(const struct pack_idx_entry **)va;
+ const struct pack_idx_entry *b = *(const struct pack_idx_entry **)vb;
+
+ return oidcmp(&a->oid, &b->oid);
+}
+
+static int bitmap_write(const char *basename)
+{
+ struct packed_git *p = NULL;
+ struct packing_data packed = { 0 };
+ struct bitmap_writer writer;
+ struct pack_idx_entry **index;
+ struct strbuf buf = STRBUF_INIT;
+ uint32_t i;
+
+ prepare_repo_settings(the_repository);
+ repo_for_each_pack(the_repository, p) {
+ if (!strcmp(pack_basename(p), basename))
+ break;
+ }
+
+ if (!p)
+ die("could not find pack '%s'", basename);
+
+ if (open_pack_index(p))
+ die("cannot open pack index for '%s'", p->pack_name);
+
+ prepare_packing_data(the_repository, &packed);
+
+ for_each_object_in_pack(p, add_packed_object, &packed,
+ ODB_FOR_EACH_OBJECT_PACK_ORDER);
+
+ /*
+ * Build the index array now that data.packed.objects[] is
+ * fully allocated (packlist_alloc() may have reallocated it
+ * during the loop above).
+ */
+ ALLOC_ARRAY(index, p->num_objects);
+ for (i = 0; i < p->num_objects; i++)
+ index[i] = &packed.objects[i].idx;
+
+ bitmap_writer_init(&writer, the_repository, &packed, NULL);
+ bitmap_writer_build_type_index(&writer, index);
+
+ while (strbuf_getline_lf(&buf, stdin) != EOF) {
+ struct object_id oid;
+ struct commit *c;
+
+ if (get_oid_hex(buf.buf, &oid))
+ die("invalid OID: %s", buf.buf);
+
+ c = lookup_commit(the_repository, &oid);
+ if (!c || repo_parse_commit(the_repository, c))
+ die("could not parse commit %s", buf.buf);
+
+ bitmap_writer_push_commit(&writer, c, 0);
+ }
+
+ select_pseudo_merges(&writer);
+ if (bitmap_writer_build(&writer) < 0)
+ die("failed to build bitmaps");
+
+ bitmap_writer_set_checksum(&writer, p->hash);
+
+ QSORT(index, p->num_objects, idx_oid_cmp);
+
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, p->pack_name);
+ strbuf_strip_suffix(&buf, ".pack");
+ strbuf_addstr(&buf, ".bitmap");
+ bitmap_writer_finish(&writer, index, buf.buf, 0);
+
+ bitmap_writer_free(&writer);
+ strbuf_release(&buf);
+ free(index);
+ clear_packing_data(&packed);
+
+ return 0;
+}
+
int cmd__bitmap(int argc, const char **argv)
{
setup_git_directory();
@@ -51,13 +159,16 @@ int cmd__bitmap(int argc, const char **argv)
return bitmap_dump_pseudo_merge_commits(atoi(argv[2]));
if (argc == 3 && !strcmp(argv[1], "dump-pseudo-merge-objects"))
return bitmap_dump_pseudo_merge_objects(atoi(argv[2]));
+ if (argc == 3 && !strcmp(argv[1], "write"))
+ return bitmap_write(argv[2]);
usage("\ttest-tool bitmap list-commits\n"
"\ttest-tool bitmap list-commits-with-offset\n"
"\ttest-tool bitmap dump-hashes\n"
"\ttest-tool bitmap dump-pseudo-merges\n"
"\ttest-tool bitmap dump-pseudo-merge-commits <n>\n"
- "\ttest-tool bitmap dump-pseudo-merge-objects <n>");
+ "\ttest-tool bitmap dump-pseudo-merge-objects <n>\n"
+ "\ttest-tool bitmap write <pack-basename> < <commit-list>");
return -1;
}
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index f693cb56691..efeb71593bf 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -648,4 +648,28 @@ test_expect_success 'truncated bitmap fails gracefully (lookup table)' '
test_grep corrupted.bitmap.index stderr
'
+test_expect_success 'test-tool bitmap write determines bitmap selection' '
+ test_when_finished "rm -fr bitmap-write-helper" &&
+ git init bitmap-write-helper &&
+ (
+ cd bitmap-write-helper &&
+
+ test_commit_bulk 64 &&
+ git repack -ad &&
+
+ pack="$(ls .git/objects/pack/pack-*.pack)" &&
+
+ git rev-parse HEAD >in &&
+ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test-tool bitmap list-commits >bitmaps.raw &&
+ sort bitmaps.raw >bitmaps &&
+ test_cmp in bitmaps &&
+
+ git rev-list --count --objects --use-bitmap-index HEAD >actual &&
+ git rev-list --count --objects HEAD >expect &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply related
* [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Taylor Blau @ 2026-05-12 0:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1776124588.git.me@ttaylorr.com>
[Note to the maintainer: this series has been rebased onto the current
tip of master, which is 7760f83b597 (Merge branch
'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].
This is a (very) small reroll of my series to fix several bugs in the
pseudo-merge bitmap implementation. The only changes since last time are:
- consistently ordering `test_when_finished "rm -fr ..."` above `git
init` in tests, and
- avoiding a single Git invocation on the left-hand side of a pipe in
the final patch
Otherwise, the series is entirely unchanged save for the above and the
rebase.
As usual, a range-diff is included below for convenience. The original
cover letter is as follows:
========================================================================
This series fixes several bugs in the pseudo-merge bitmap implementation
that caused the pseudo-merge application path to be effectively broken
during fill-in traversal.
Peff noticed that this code path was never triggered by the existing
test suite, and investigating that observation uncovered a handful of
bugs, some compounding.
The first two patches introduce test infrastructure: a 'bitmap write'
test helper that gives tests precise control over which commits receive
individual bitmaps, and a set of "test_expect_failure" tests
demonstrating each bug.
The next four patches fix the bugs in the per-commit pseudo-merge
lookup:
- The pseudo-merge commit lookup table was sorted by OID rather than
by bit position, causing the reader's binary search to fail.
- The binary search in pseudo_merge_at() had its lo/hi updates
swapped.
- The extended pseudo-merge lookup path had three compounding bugs: a
wrong entry-size calculation in the writer, a misinterpretation of
extended table entries in the reader, and a silently-swallowed error
check.
The final two patches fix issues in pseudo-merge group selection:
- find_pseudo_merge_group_for_ref() did not parse commits before
inspecting their dates, so all candidates had date == 0 and were
unconditionally placed in the "stable" bucket.
- The config validation for bitmapPseudoMerge.*.sampleRate accepted 0,
which leads to a division by zero once the date classification is
fixed and the unstable code path is exercised.
There is also a small fix for a regex leak when the pattern key is
overridden in config.
Thanks in advance for your review!
Taylor Blau (9):
t/helper: add 'test-tool bitmap write' subcommand
t5333: demonstrate various pseudo-merge bugs
pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
pack-bitmap: fix pseudo-merge lookup for shared commits
pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
pack-bitmap: reject pseudo-merge "sampleRate" of 0
Documentation: fix broken `sampleRate` in gitpacking(7)
pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
Documentation/config/bitmap-pseudo-merge.adoc | 4 +-
Documentation/gitpacking.adoc | 4 +-
pack-bitmap-write.c | 23 +-
pseudo-merge.c | 19 +-
t/helper/test-bitmap.c | 113 ++++++++-
t/t5310-pack-bitmaps.sh | 24 ++
t/t5333-pseudo-merge-bitmaps.sh | 229 ++++++++++++++++++
7 files changed, 402 insertions(+), 14 deletions(-)
Range-diff against v2:
1: c0df35f8ebd = 1: 9c7a829cbeb t/helper: add 'test-tool bitmap write' subcommand
2: 11de3343726 = 2: d1ed4aadf75 t5333: demonstrate various pseudo-merge bugs
3: 8d908ab415e = 3: bf3a9a07e5f pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
4: 07f70a07c20 ! 4: a1d341c92eb pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
@@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'apply pseudo-merges during
+test_expect_success 'apply pseudo-merges from multiple groups during fill-in' '
test_when_finished "rm -fr pseudo-merge-fill-in-multi" &&
git init pseudo-merge-fill-in-multi &&
-+ git init pseudo-merge-fill-in-multi &&
(
- cd pseudo-merge-fill-in-multi &&
-
5: 3ed0b39843f = 5: 06e3410d323 pack-bitmap: fix pseudo-merge lookup for shared commits
6: 95f847211f3 = 6: 78cf7e6d80d pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
7: f8a01cfb893 = 7: 4dbf6686718 pack-bitmap: reject pseudo-merge "sampleRate" of 0
8: c37156502c0 = 8: 46d0ee2f168 Documentation: fix broken `sampleRate` in gitpacking(7)
9: b905fd5d0ae ! 9: 9b17dab2cf7 pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
@@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus
'
+test_expect_success 'duplicate pseudo-merge pattern does not leak' '
-+ git init pseudo-merge-dup-pattern &&
+ test_when_finished "rm -fr pseudo-merge-dup-pattern" &&
-+
++ git init pseudo-merge-dup-pattern &&
+ (
+ cd pseudo-merge-dup-pattern &&
+
@@ t/t5333-pseudo-merge-bitmaps.sh: test_expect_success 'sampleRate=0 does not caus
+ git config bitmapPseudoMerge.test.threshold now &&
+ git config bitmapPseudoMerge.test.stableThreshold never &&
+
-+ git rev-parse HEAD~63 |
-+ test-tool bitmap write "$(basename $pack)" &&
++ git rev-parse HEAD~63 >in &&
++ test-tool bitmap write "$(basename $pack)" <in &&
+
+ test_pseudo_merges >merges &&
+ test_line_count = 1 merges
base-commit: 7760f83b59750c27df653c5c46d0f80e44cfe02c
--
2.54.0.76.g9b17dab2cf7
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox