* [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
* Re: [PATCH v4 2/2] commit-reach: early exit paint_down_to_common for single merge-base
From: Junio C Hamano @ 2026-05-12 0:40 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget
Cc: git, Patrick Steinhardt, Kristofer Karlsson
In-Reply-To: <19f1605067e26c8e393c6c2e341844bcb3dc1b41.1778504352.git.gitgitgadget@gmail.com>
"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Kristofer Karlsson <krka@spotify.com>
>
> Commits not in the commit-graph get GENERATION_NUMBER_INFINITY and
> sort to the top of the priority queue. After those, commits with
> finite generation numbers are popped in non-increasing order.
> When MERGE_BASE_FIND_ALL is not set the first doubly-painted commit
> with a finite generation is therefore a best merge-base: no commit
> still in the queue can be a descendant of it. Skip the expensive
> STALE drain in this case.
>
> Add MERGE_BASE_FIND_ALL to the merge_base_flags enum. Callers that
> need every merge-base (repo_get_merge_bases_many, repo_get_merge_bases,
> repo_in_merge_bases_many, remove_redundant_no_gen) pass the flag to
> preserve existing behavior. git merge-base (without --all) passes 0,
> triggering the early exit.
>
> On a 2.2M-commit merge-heavy monorepo with commit-graph:
>
> HEAD vs ~500: 5,229ms -> 24ms
> HEAD vs ~1000: 4,214ms -> 39ms
> HEAD vs ~5000: 3,799ms -> 46ms
> HEAD vs ~10000: 3,827ms -> 61ms
>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> builtin/merge-base.c | 3 ++-
> commit-reach.c | 19 +++++++++++++++----
> commit-reach.h | 7 ++++++-
> t/t6600-test-reach.sh | 40 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 63 insertions(+), 6 deletions(-)
Very nicely done and well described.
> diff --git a/builtin/merge-base.c b/builtin/merge-base.c
> index 9b50b4660e..a87011c6cd 100644
> --- a/builtin/merge-base.c
> +++ b/builtin/merge-base.c
> @@ -11,11 +11,12 @@
>
> static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
> {
> + enum merge_base_flags flags = show_all ? MERGE_BASE_FIND_ALL : 0;
Curious that only this variable, among 6 that this two-patch series
introduces for the type, is called "flags" while all others are
called "mb_flags". No need to change it; the comment is mostly to
show I did read the two patches with reasonable attention to the
detail ;-).
Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH v8] checkout: extend --track with a "fetch" mode to refresh start-point
From: Junio C Hamano @ 2026-05-12 0:32 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk,
Marc Branchaud, Phillip Wood, Harald Nordgren
In-Reply-To: <pull.2281.v8.git.git.1778507225500.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
> index 43ccf47cf6..28f17f427e 100644
> --- a/Documentation/git-checkout.adoc
> +++ b/Documentation/git-checkout.adoc
> @@ -158,11 +158,22 @@ of it").
> resets _<branch>_ to the start point instead of failing.
>
> `-t`::
> -`--track[=(direct|inherit)]`::
> +`--track[=(direct|inherit|fetch)[,...]]`::
> When creating a new branch, set up "upstream" configuration. See
> `--track` in linkgit:git-branch[1] for details. As a convenience,
> --track without -b implies branch creation.
> +
> +The argument is a comma-separated list. `direct` (the default) and
> +`inherit` select the tracking mode and are mutually exclusive. Adding
> +`fetch` requests that the remote be fetched before _<start-point>_ is
> +resolved, so the new branch starts from a fresh tip: when
> +_<start-point>_ is in _<remote>/<branch>_ form, only that branch is
> +updated; when _<start-point>_ is a bare remote name (e.g. `origin`),
> +only the remote's default branch is updated.
The latter is because "checkout -t -b new remote/origin" makes "new"
track "remote/origin/HEAD", which makes sense. And of course this
fetch can fail.
> +static int resolve_fetch_target(const char *arg, char **remote_out,
> + char **src_ref_out)
> +{
> + const char *slash;
> + char *remote_name;
> + struct remote *remote;
> + struct refspec_item query = { 0 };
> + struct strbuf dst = STRBUF_INIT;
> + const char *rest;
> +
> + *remote_out = NULL;
> + *src_ref_out = NULL;
> +
> + if (!arg || !*arg)
> + return -1;
> +
> + slash = strchr(arg, '/');
> + if (slash == arg)
> + return -1;
> + remote_name = slash ? xstrndup(arg, slash - arg) : xstrdup(arg);
> +
> + remote = remote_get(remote_name);
> + if (!remote || !remote_is_configured(remote, 1)) {
> + free(remote_name);
> + return -1;
> + }
> +
> + rest = (slash && slash[1]) ? slash + 1 : NULL;
There is no slash when asking for "origin", and the control goes
into the "if (!rest)" block.
> + if (!rest) {
> + struct object_id oid;
> + const char *head_target;
> + const char *short_target;
> +
> + strbuf_addf(&dst, "refs/remotes/%s/HEAD", remote_name);
> + head_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> + dst.buf,
> + RESOLVE_REF_READING |
> + RESOLVE_REF_NO_RECURSE,
> + &oid, NULL);
> + strbuf_reset(&dst);
> + if (head_target &&
> + skip_prefix(head_target, "refs/remotes/", &short_target) &&
> + skip_prefix(short_target, remote_name, &short_target) &&
> + *short_target == '/')
> + rest = short_target + 1;
> + }
If refs/remotes/origin/HEAD points at refs/remotes/origin/main, rest
gets "main". Otherwise (i.e. unexpected contents in HEAD or lacking
HEAD), rest remains NULL. And then we have the "if (rest)" block.
> + if (rest) {
> + strbuf_addf(&dst, "refs/remotes/%s/%s", remote_name, rest);
> + query.dst = dst.buf;
> + if (!remote_find_tracking(remote, &query) && query.src) {
> + *src_ref_out = xstrdup(query.src);
> + free(query.src);
> + } else {
> + *src_ref_out = xstrdup(rest);
> + }
> + }
> +
> + strbuf_release(&dst);
> + *remote_out = remote_name;
> + return 0;
> +}
It is not a new problem but I do not remember how we explicitly
forbid "hierarchical" remote names. refs/remotes/a/b/c might be
their branch "b/c" at remote we call "a", or branch "c" at remote
"a/b". Unless we forbid slashes in remote names, the "there is no
slash so we got only the name of the remote to mean its HEAD" logic
would not work well, so we may want to double check.
> +static void fetch_remote_for_start_point(const char *arg)
> +{
> + char *remote_name = NULL;
> + char *src_ref = NULL;
> + struct child_process cmd = CHILD_PROCESS_INIT;
> + struct strbuf dst_ref = STRBUF_INIT;
> + int have_existing_ref = 0;
> +
> + if (resolve_fetch_target(arg, &remote_name, &src_ref))
> + return;
> +
> + {
> + struct object_id oid;
> +
> + if (strchr(arg, '/'))
> + strbuf_addf(&dst_ref, "refs/remotes/%s", arg);
> + else
> + strbuf_addf(&dst_ref, "refs/remotes/%s/HEAD", arg);
> + if (!refs_read_ref(get_main_ref_store(the_repository),
> + dst_ref.buf, &oid))
> + have_existing_ref = 1;
> + }
I do not quite see the point of this extra block. Can we do without
it (and move the def of oid up near the beginning of the function,
of course)?
Even better, as resolve_fetch_target() already looks at "arg" and
poked at the remote-tracking ref hierarchy, wouldn't it make more
sense to make that helper function responsible for finding out if
there already is a usable, albeit potentially stale, ref?
> + strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
> + if (src_ref)
> + strvec_push(&cmd.args, src_ref);
> + cmd.git_cmd = 1;
> + if (run_command(&cmd)) {
> + if (have_existing_ref)
> + warning(_("failed to fetch start-point '%s'; "
> + "using existing '%s'"),
> + arg, dst_ref.buf);
> + else
> + die(_("failed to fetch start-point '%s'"), arg);
> + }
> +
> + free(remote_name);
> + free(src_ref);
> + strbuf_release(&dst_ref);
> +}
> @@ -1244,7 +1398,6 @@ static int git_checkout_config(const char *var, const char *value,
> opts->dwim_new_local_branch = git_config_bool(var, value);
> return 0;
> }
> -
> if (starts_with(var, "submodule."))
> return git_default_submodule_config(var, value, NULL);
Unrelated patch noise?
^ permalink raw reply
* Re: [PATCH v2 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Taylor Blau @ 2026-05-12 0:10 UTC (permalink / raw)
To: Elijah Newren; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <CABPp-BGkfavqezk2SV3+K6iF8MLm8j_=ijHiPDLmv_U_o_Ykgg@mail.gmail.com>
On Tue, Apr 21, 2026 at 06:37:45PM -0700, Elijah Newren wrote:
> Here you fixed the order, but...
>
> > ++ git init pseudo-merge-fill-in-multi &&
>
> ...then you immediately run git init a second time? I'm guessing this
> was a stray edit made while trying to fix the order; could we get rid
> of the duplicate?
Oof, good catch. I'm not sure how that snuck in there, but it's fixed on
my end.
> > (
> > + cd pseudo-merge-fill-in-multi &&
> > +
>
> Looks like you addressed all the feedback so far from v1. There does
> appear to be a new accidental double-init that I noted above in patch
> 4, but I didn't spot any other issues.
Besides that, I found one more spot that needed some love, which is the
new "duplicate pseudo-merge pattern does not leak" test added at the
very end of the series, which had a wrong ordering, and piped the output
of 'git rev-parse' directly into a test helper.
I'll send a fixed up round out now.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH] add -p: introduce 'w' command to view hunk with --word-diff
From: Junio C Hamano @ 2026-05-12 0:04 UTC (permalink / raw)
To: D. Ben Knoble
Cc: Phillip Wood, Javier Bassi, git, Abraham Samuel Adekunle,
Rene Scharfe, Elijah Newren, Ruben Justo, Patrick Steinhardt
In-Reply-To: <CALnO6CCR9bQ71LR_buEAzX4UcR4sERnaVLc6_19sJUfXyi-HPA@mail.gmail.com>
"D. Ben Knoble" <ben.knoble@gmail.com> writes:
>> > :show <opts>
> ...
> - ":" is an exception to single-key mode and always allows more text
> to compose a longer-form "command". The first command could be "print"
> which takes options of this kind.
>
> Since ":" would be new, exempting it from single-key mode shouldn't be
> too jarring I guess.
OK. And we can use ':blah' to rarely used actions and give them
descriptive names. Makes sense. word-diff sounds like a good
candidate but it is not immediately obvious to me if machinery to
set up the "colon is special and handles longhand commands with
possible arguments" is worth doing.
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Jacob Keller @ 2026-05-11 23:58 UTC (permalink / raw)
To: Jeff King
Cc: Taylor Blau, jean-christophe manciot, Patrick Steinhardt, git,
Derrick Stolee
In-Reply-To: <20260511203502.GA25510@coredump.intra.peff.net>
On 5/11/2026 1:35 PM, Jeff King wrote:
> On Mon, May 11, 2026 at 01:21:37PM -0700, Jacob Keller wrote:
>
>>> - If both claim ownership, it's mostly OK, because they'll both try to
>>> unlink() which is idempotent-ish. Though there is a bad sequence
>>> where we delete somebody _else's_ lock, like:
>>>
>>> 1. Parent forks, but has not yet reassigned.
>>>
>>> 2. Child calls reassign to take ownership. Now both have
>>> ownership.
>>>
>>> 3. Signal kills both parent and child, so they enter cleanup
>>> code.
>>>
>>> 4. One of them (let's say the parent) deletes the lockfile.
>>>
>>> 5. Some other unrelated process (let's call it "git other") takes
>>> the lock.
>>>
>>> 6. The child deletes the lockfile.
>>>
>>> And at that point "git other" thinks it holds the lock, but it
>>> doesn't. It's quite an unlikely sequence in practice, though, I'd
>>> think.
>>>
>>> - If neither claims ownership and a signal kills both, then nobody
>>> cleans up the lock and it is left in place. This is annoying, but
>>> also something that can happen occasionally anyway (kill -9, etc).
>>>
>>> I don't have an easy suggestion for making it more atomic, though. You
>>> could choose one or the other direction using some synchronization
>>> between the two (e.g., child reassigns only after parent signals over a
>>> pipe that it has relinquished), but it's all kind of ugly.
>>>
>> Ya, that seems really ugly. My first thought was some way to disable
>> signals temporarily, but I am guessing that either has no good way to do
>> it or would introduce even more issues. Plus there is always kill -9...
>
> I guess the parent could relinquish control by reassigning to "0" before
> even calling fork(), and then taking it back if fork() fails. And then
> worst case is that nobody cleans up the lock (if the child is killed
> before taking control), but that's the least-bad outcome from a
> correctness perspective (though still annoying).
>
> -Peff
This seems simpler than the suggestion of a pipe signal, and the worst
result is only the version that is possible no matter what if there is a
well timed kill -9.
^ permalink raw reply
* Re: [PATCH v6 0/5] branch: prune-merged
From: Junio C Hamano @ 2026-05-11 23:20 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> * --prune-merged now measures merged-ness against the remote's default
> branch instead of the candidate's upstream — so the decision no longer
> depends on which branch happens to be checked out locally.
I may be misreading the above and misunderstood you, but if you mean
that the feature now checks with remote/origin/master when I have a
local branch that were forked from remote/origin/todo and set to
merge new changes from there, I do not think it is a good change.
The two remote-tracking branches may not even share any commit.
The "what it tracks or HEAD" logic I raised as questionable is in
the function builtin/branch.c:branch_merged() that is called from
the function check_branch_commit() you updated and used in the
implementation of prune-merged. It does branch_get_upstream() to
find the tip of remote-tracking branch that the target branch builds
upon, and performs comparison (which is very sensible). I do not
think you want to change it to check with remotes/<remote>/HEAD
instead, as the upstream of the local branch may not be building on
their HEAD at all. But when the upstream is not found, the code
makes the reference_rev variable fall back to head_rev, and checks
if the commit at the tip of the target branch is already merged
there.
It is still not clear to me if we want to optionally disable this
fallback to HEAD, but a quick scan of branch_merged() tells me that
it is prepared to see NULL in head_rev.
^ permalink raw reply
* Re: git hard-codes use of /bin/sh for aliases
From: Thorsten Glaser @ 2026-05-11 22:46 UTC (permalink / raw)
To: brian m. carlson; +Cc: git
In-Reply-To: <agJaS0xuMaLm6UZv@fruit.crustytoothpaste.net>
On Mon, 11 May 2026, brian m. carlson wrote:
>On Linux, I have mksh, but my work machine on macOS does not, so
>specifying `core.sh = /bin/mksh` would not work on all my systems. If
But it could be made possible by pointing it to ~/.etc/bin/mksh
on the Macintosh…
>I'll also note that Git doesn't handle aliases specially. Git has a
>with-shell and a without-shell mode for running commands, and we use the
>former for aliases.
Ah, okay.
I was merely wondering that it was not configurable for aliases.
When looking at the aliases in detail, I found that I already invoked
mksh -c '…' in those that used mksh-specific code, and that the one
that failed merely uses a recent POSIX sh construct that /bin/sh on
a customer system does not yet implement and I could reword it.
bye,
//Thorsten
--
Thorsten Glaser
Linux / Unix Developer
Tel.: +49 160 91168501
E-Mail: tglaser@b1-systems.de
B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / https://www.b1-systems.de/
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt, HRB 3537
^ permalink raw reply
* Re: git hard-codes use of /bin/sh for aliases
From: brian m. carlson @ 2026-05-11 22:38 UTC (permalink / raw)
To: Thorsten Glaser; +Cc: git
In-Reply-To: <6791cb26-16af-dcda-1dae-82b85da6fb0f@b1-systems.de>
[-- Attachment #1: Type: text/plain, Size: 2829 bytes --]
On 2026-05-11 at 21:13:18, Thorsten Glaser wrote:
> Hi,
>
> git aliases don’t use the user’s shell but a hardcoded /bin/sh,
> and this cannot be made configurable.
>
> This, unfortunately, breaks quite some aliases and they cannot
> be made portable by adding something like core.sh = /bin/mksh
> to the .gitconfig.
>
> I’d love if this could be made configurable.
Yes, I believe they use `/bin/sh`, except on Windows, where they may
look up things in the PATH. It is very much intentional that they use
sh because it means that aliases work in a consistent way across
platforms and that they work at all.
On Linux, I have mksh, but my work machine on macOS does not, so
specifying `core.sh = /bin/mksh` would not work on all my systems. If
you have system-level aliases as well as user-level aliases, then you
need to pick a particular shell and letting the user choose will likely
break the system-level aliases. This is not a hypothetical: my last
company shipped system-level aliases on development VMs while users were
free to customize their user-level aliases (and shell).
This will absolutely be a problem on Windows, where you have CMD,
multiple versions of PowerShell, and bash, all with different syntax.
Some versions of PowerShell also corrupt non-Unicode data sent through
pipelines as well, which means they're effectively useless for
general-purpose scripting.
It can also lead to odd behaviour on some shells because, for instance,
zsh runs a config file with `-c`, `.zshenv`, that can make the shell
very much less POSIX compliant, so behaviour can differ substantially
from sh.
I'll also note that Git doesn't handle aliases specially. Git has a
with-shell and a without-shell mode for running commands, and we use the
former for aliases. We definitely do not want the with-shell mode to
generically allow arbitrary shells because we cannot handle non-POSIX
shells and will not quote correctly for them (almost certainly leading
to security vulnerabilities). We also format the shell command as "%s
\"$@\"", which also basically requires a POSIX shell to work, and we
don't want to work around people's use of non-POSIX shells; that way
madness lies.
However, because mksh is POSIXy, you could build Git on your system with
`SHELL_PATH=/bin/mksh`, which would use `/bin/mksh` everywhere that the
shell is invoked. That feature exists primarily for systems where `sh`
is not POSIXy or doesn't support `local` (which I know mksh does), but
there's no reason you can't use it yourself.
There has been some discussion about this issue in the past (in the
context of PowerShell) at
https://lore.kernel.org/git/CAAXzdLXt4+-34+OhS=Jn=-VeORN3Y2jMzzg9+bhyn88aN4hm0A@mail.gmail.com/ .
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply
* [ANNOUNCE] Tickets available for Git Merge 2026
From: Taylor Blau @ 2026-05-11 22:17 UTC (permalink / raw)
To: git; +Cc: Scott Chacon, Patrick Steinhardt, Emily Shaffer
Tickets are now on sale for Git Merge 2026!
To purchase tickets, you can visit the conference website at:
https://git-merge.com
Tickets are available for $99 USD, with an option to attend the JJ day
exclusively ($49 USD) or in conjunction with the main conference ($129
USD).
If you are a Git or JJ contributor, please e-mail me off-list for a
discount code that will allow you to attend for free. For Git
contributors specifically, this option will also allow you to register
for the Contributor's Summit, held on the 18th. If you aren't sure if
you qualify, please reach out and ask!
Purchase of a ticket (excluding the "JJ day only" option) grants you
access to both days of the main conference, including food/drinks/beer,
as well as the after-party.
As a reminder[1], the details for the main event are as follows:
When: September 17-18th, 2026
Where: Suspenso, Lisbon, Portugal (https://suspenso.pt/)
What: Talks on the 17th, everything else on the 18th
(The JJ day will be held on September 16th, 2026, at the same location.)
The call for proposals (CFP) is now open, and we're using the same CFP
submission form for both JJ-specific and Git Merge talks.
The CFP submission form can be found at:
https://bit.ly/git-merge-2026-cfp
, and will be available until the CFP closes on June 30th. At that
point, we'll go through the talk submissions and notify prospective
speakers of the outcome.
As before, if you would like financial assistance with travel costs,
please reach out to the Git PLC at <git@sfconservancy.org>. We will
accept requests until May 15, 2026, and will inform applicants of the
outcome shortly thereafter.
See you in Lisbon!
Thanks,
Scott Chacon (GitButler)
Taylor Blau (GitHub)
[1]: https://lore.kernel.org/git/aeenTA8efhY6hNvt@nand.local/
^ permalink raw reply
* git hard-codes use of /bin/sh for aliases
From: Thorsten Glaser @ 2026-05-11 21:13 UTC (permalink / raw)
To: git
Hi,
git aliases don’t use the user’s shell but a hardcoded /bin/sh,
and this cannot be made configurable.
This, unfortunately, breaks quite some aliases and they cannot
be made portable by adding something like core.sh = /bin/mksh
to the .gitconfig.
I’d love if this could be made configurable.
bye,
//Thorsten
--
Thorsten Glaser
Linux / Unix Developer
Tel.: +49 160 91168501
E-Mail: tglaser@b1-systems.de
B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / https://www.b1-systems.de/
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt, HRB 3537
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Jeff King @ 2026-05-11 20:35 UTC (permalink / raw)
To: Jacob Keller
Cc: Taylor Blau, jean-christophe manciot, Patrick Steinhardt, git,
Derrick Stolee
In-Reply-To: <590781db-7c19-4aa8-8497-e16e5eb5eba1@intel.com>
On Mon, May 11, 2026 at 01:21:37PM -0700, Jacob Keller wrote:
> > - If both claim ownership, it's mostly OK, because they'll both try to
> > unlink() which is idempotent-ish. Though there is a bad sequence
> > where we delete somebody _else's_ lock, like:
> >
> > 1. Parent forks, but has not yet reassigned.
> >
> > 2. Child calls reassign to take ownership. Now both have
> > ownership.
> >
> > 3. Signal kills both parent and child, so they enter cleanup
> > code.
> >
> > 4. One of them (let's say the parent) deletes the lockfile.
> >
> > 5. Some other unrelated process (let's call it "git other") takes
> > the lock.
> >
> > 6. The child deletes the lockfile.
> >
> > And at that point "git other" thinks it holds the lock, but it
> > doesn't. It's quite an unlikely sequence in practice, though, I'd
> > think.
> >
> > - If neither claims ownership and a signal kills both, then nobody
> > cleans up the lock and it is left in place. This is annoying, but
> > also something that can happen occasionally anyway (kill -9, etc).
> >
> > I don't have an easy suggestion for making it more atomic, though. You
> > could choose one or the other direction using some synchronization
> > between the two (e.g., child reassigns only after parent signals over a
> > pipe that it has relinquished), but it's all kind of ugly.
> >
> Ya, that seems really ugly. My first thought was some way to disable
> signals temporarily, but I am guessing that either has no good way to do
> it or would introduce even more issues. Plus there is always kill -9...
I guess the parent could relinquish control by reassigning to "0" before
even calling fork(), and then taking it back if fork() fails. And then
worst case is that nobody cleans up the lock (if the child is killed
before taking control), but that's the least-bad outcome from a
correctness perspective (though still annoying).
-Peff
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Jeff King @ 2026-05-11 20:22 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Taylor Blau, Derrick Stolee, jean-christophe manciot, git
In-Reply-To: <agF6Q-z1SuTL1xXs@pks.im>
On Mon, May 11, 2026 at 08:42:11AM +0200, Patrick Steinhardt wrote:
> In any case, I also agree with Stolee that it may have unintended
> consequences to unconditionally reassign tempfiles to the child process
> when daemonizing. It is okay for all current callers, but I'm not sure
> whether future callers would expect this behaviour.
>
> An alternative would be to explicitly reassign the lockfile's ownership
> in git-maintenance(1). Something like the below patch.
I really think this should be an automatic part of daemonize(), for the
reasons I gave earlier in the thread. It's really a lurking problem for
any cleanup handler, but lockfiles are the one that is most likely to
bite us.
In fact, I thought "git gc --detach" without "--skip-foreground-tasks"
had the same bug, but it narrowly avoids it by dropping and reacquiring
the lock (!) on either side of the daemonize() call.
So I think you are more likely to avoid unpleasant surprises by baking
the behavior into daemonize() rather than requiring each caller to do it
manually.
Plus it keeps daemonize() a bit more abstract. It's a noop on Windows,
but you can imagine an implementation that does weird platform-specific
stuff and doesn't switch pids at all.
-Peff
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Jacob Keller @ 2026-05-11 20:21 UTC (permalink / raw)
To: Jeff King, Taylor Blau
Cc: jean-christophe manciot, Patrick Steinhardt, git, Derrick Stolee
In-Reply-To: <20260511200112.GA22912@coredump.intra.peff.net>
On 5/11/2026 1:01 PM, Jeff King wrote:
> On Sat, May 09, 2026 at 05:52:29PM -0400, Taylor Blau wrote:
>
>> Dropping and re-acquiring the lock is possible, but it's racy since
>> there is a gap during the critical window while we fork(). I believe
>> that the only airtight way to do this is to update the owner field of
>> the tempfiles we want to pass down during daemonization.
>
> It's racy in the sense that we might not get the lock back, but I don't
> think that's _wrong_ exactly. We just care that some instance runs the
> second half eventually.
>
> It is technically vulnerable to starvation, if processes keep taking the
> lock, doing the foreground jobs, and then losing the race to take it a
> second time. But besides the fact that you'd eventually win by chance,
> unless you have an infinite supply of processes coming along to run the
> foreground half, the final invocation will run the background tasks.
>
> I agree it feels pretty hacky. It is, however, how "git gc" seems to
> work (notice that it does a manual delete_tempfile() on the lock right
> before daemonizing).
>
>> (As an aside, do we want to do that for all tempfiles? It might be nice
>> to have a "->reassign_on_fork" flag or something on the tempfile struct
>> in case there are instances where the parent wants to retain ownership
>> of the tempfile after fork()-ing, but I can't think of any off the top
>> of my head. If we do introduce such a field, it should probably default
>> to "true" to avoid any foot-guns.)
>
> I think the answer is yes, we want it for all tempfiles. Because it is
> not a property of the individual tempfiles, but rather of the caller who
> is forking. The point of that owner field is so that when we spawn
> children, we don't end up with duplicate or unexpected cleanups.
>
> But daemonize() is not about spawning a child or otherwise splitting the
> process into two. There is still a single contiguous flow of execution,
> and the fact that we switch pids and call exit() is an implementation
> detail. So we would still want to remain the "owner" of any cleanup.
>
> And that is true for any other atexit() handlers besides the tempfile
> ones, too. If we called run_processes_parallel(), and then while that
> child was running we called daemonize(), we'd not want to kill the child
> then, but our atexit handler would do so! But we don't tend to leave
> processes running while doing stuff like daemonize(), which is why we
> never bothered to implement an owner field there in the first place.
>
>>> Ultimately fixing the lock bug will solve that. Though if doing so is
>>> too complicated for a quick maint release, I'm tempted to say we should
>>> consider reverting 452b12c2e0 for a potential v2.54.1 (as there were a
>>> few other regression fixes so far, I assume we'll have one soon-ish).
>>
>> I think something like the following (untested) would do the trick:
>
> Yeah, that's what I was envisioning. Note that there's a small race
> here:
>
>> + switch ((pid = fork())) {
>> case 0:
>> + reassign_tempfile_ownership(ppid, getpid());
>> break;
>> case -1:
>> die_errno(_("fork failed"));
>> default:
>> + reassign_tempfile_ownership(ppid, pid);
>> exit(0);
>> }
>
> The parent is giving up ownership and the child is taking it. So there
> may be a moment where both claim ownership, or a moment where the parent
> has relinquished but the child has not yet taken. We're not going to
> call exit() in the middle there, but a signal could kill us.
>
> And then either:
>
> - If both claim ownership, it's mostly OK, because they'll both try to
> unlink() which is idempotent-ish. Though there is a bad sequence
> where we delete somebody _else's_ lock, like:
>
> 1. Parent forks, but has not yet reassigned.
>
> 2. Child calls reassign to take ownership. Now both have
> ownership.
>
> 3. Signal kills both parent and child, so they enter cleanup
> code.
>
> 4. One of them (let's say the parent) deletes the lockfile.
>
> 5. Some other unrelated process (let's call it "git other") takes
> the lock.
>
> 6. The child deletes the lockfile.
>
> And at that point "git other" thinks it holds the lock, but it
> doesn't. It's quite an unlikely sequence in practice, though, I'd
> think.
>
> - If neither claims ownership and a signal kills both, then nobody
> cleans up the lock and it is left in place. This is annoying, but
> also something that can happen occasionally anyway (kill -9, etc).
>
> I don't have an easy suggestion for making it more atomic, though. You
> could choose one or the other direction using some synchronization
> between the two (e.g., child reassigns only after parent signals over a
> pipe that it has relinquished), but it's all kind of ugly.
>
Ya, that seems really ugly. My first thought was some way to disable
signals temporarily, but I am guessing that either has no good way to do
it or would introduce even more issues. Plus there is always kill -9...
> So it may be reasonable to just shrug and assume it's unlikely enough
> not to worry about.
>
> -Peff
>
Yea, I agree that this might qualify as unlikely enough to not worry.
^ permalink raw reply
* Re: [PATCH 2/2] run-command: honor "gc.auto" for auto-maintenance
From: Jeff King @ 2026-05-11 20:18 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Jean-Christophe Manciot, Mikael Magnusson, Taylor Blau,
Derrick Stolee
In-Reply-To: <20260511-pks-maintenance-fix-lock-with-detach-v1-2-ccd7d62c9a40@pks.im>
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
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Jeff King @ 2026-05-11 20:10 UTC (permalink / raw)
To: Derrick Stolee
Cc: Taylor Blau, jean-christophe manciot, Patrick Steinhardt, git
In-Reply-To: <9ddfd37d-7d71-4359-b9be-d993fbfd138c@gmail.com>
On Sun, May 10, 2026 at 12:08:14PM -0400, Derrick Stolee wrote:
> > So in that sense I would prefer to "fix forward" here rather than to
> > mask over the bug. But even the relatively short diff above is not so
> > straightforward to reason through, review, or test, so I'm open to other
> > ideas on how to proceed here.
>
> I initially worried about cross-platform support, thinking that we
> needed to pass file descriptors / handles and Windows always has
> issues with file handles. But we aren't actually keeping a handle
> open but instead a record that we created the lock and should delete
> it when everything resolves.
The daemonize() function is a noop on Windows anyway, because we don't
have fork(). It's controlled by the NO_POSIX_GOODIES knob.
> For me to be convinced that this forward fix is the right direction,
> I'd need to see a test that proves the detached process will clean up
> the locks on a normal process end and an early exit.
I think it's hard to trigger an early exit, since the only thing we do
while holding the lock is run the maintenance tasks, and those are
almost entirely just using run-command to run subprocesses. So I don't
see a single path that can lead to a die().
Which leaves signals. It is easy-ish to test manually with a
long-running maintenance (say, the "gc" task on linux.git), verifying
that maintenance.lock is there, killing the detached git-maintenance
process with SIGTERM, and then confirming that the lock was cleaned up.
It looks like Taylor found a way to make an arbitrarily slow task using
the prefetch process. So I think his test could be modified to issue a
kill at the right moment and verify cleanup (rather than waiting for a
normal exit, which calls rollback_lock_file() and never checks the owner
field at all).
-Peff
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Jeff King @ 2026-05-11 20:01 UTC (permalink / raw)
To: Taylor Blau
Cc: jean-christophe manciot, Patrick Steinhardt, git, Derrick Stolee
In-Reply-To: <af+snTGFeoUUyfPU@nand.local>
On Sat, May 09, 2026 at 05:52:29PM -0400, Taylor Blau wrote:
> Dropping and re-acquiring the lock is possible, but it's racy since
> there is a gap during the critical window while we fork(). I believe
> that the only airtight way to do this is to update the owner field of
> the tempfiles we want to pass down during daemonization.
It's racy in the sense that we might not get the lock back, but I don't
think that's _wrong_ exactly. We just care that some instance runs the
second half eventually.
It is technically vulnerable to starvation, if processes keep taking the
lock, doing the foreground jobs, and then losing the race to take it a
second time. But besides the fact that you'd eventually win by chance,
unless you have an infinite supply of processes coming along to run the
foreground half, the final invocation will run the background tasks.
I agree it feels pretty hacky. It is, however, how "git gc" seems to
work (notice that it does a manual delete_tempfile() on the lock right
before daemonizing).
> (As an aside, do we want to do that for all tempfiles? It might be nice
> to have a "->reassign_on_fork" flag or something on the tempfile struct
> in case there are instances where the parent wants to retain ownership
> of the tempfile after fork()-ing, but I can't think of any off the top
> of my head. If we do introduce such a field, it should probably default
> to "true" to avoid any foot-guns.)
I think the answer is yes, we want it for all tempfiles. Because it is
not a property of the individual tempfiles, but rather of the caller who
is forking. The point of that owner field is so that when we spawn
children, we don't end up with duplicate or unexpected cleanups.
But daemonize() is not about spawning a child or otherwise splitting the
process into two. There is still a single contiguous flow of execution,
and the fact that we switch pids and call exit() is an implementation
detail. So we would still want to remain the "owner" of any cleanup.
And that is true for any other atexit() handlers besides the tempfile
ones, too. If we called run_processes_parallel(), and then while that
child was running we called daemonize(), we'd not want to kill the child
then, but our atexit handler would do so! But we don't tend to leave
processes running while doing stuff like daemonize(), which is why we
never bothered to implement an owner field there in the first place.
> > Ultimately fixing the lock bug will solve that. Though if doing so is
> > too complicated for a quick maint release, I'm tempted to say we should
> > consider reverting 452b12c2e0 for a potential v2.54.1 (as there were a
> > few other regression fixes so far, I assume we'll have one soon-ish).
>
> I think something like the following (untested) would do the trick:
Yeah, that's what I was envisioning. Note that there's a small race
here:
> + switch ((pid = fork())) {
> case 0:
> + reassign_tempfile_ownership(ppid, getpid());
> break;
> case -1:
> die_errno(_("fork failed"));
> default:
> + reassign_tempfile_ownership(ppid, pid);
> exit(0);
> }
The parent is giving up ownership and the child is taking it. So there
may be a moment where both claim ownership, or a moment where the parent
has relinquished but the child has not yet taken. We're not going to
call exit() in the middle there, but a signal could kill us.
And then either:
- If both claim ownership, it's mostly OK, because they'll both try to
unlink() which is idempotent-ish. Though there is a bad sequence
where we delete somebody _else's_ lock, like:
1. Parent forks, but has not yet reassigned.
2. Child calls reassign to take ownership. Now both have
ownership.
3. Signal kills both parent and child, so they enter cleanup
code.
4. One of them (let's say the parent) deletes the lockfile.
5. Some other unrelated process (let's call it "git other") takes
the lock.
6. The child deletes the lockfile.
And at that point "git other" thinks it holds the lock, but it
doesn't. It's quite an unlikely sequence in practice, though, I'd
think.
- If neither claims ownership and a signal kills both, then nobody
cleans up the lock and it is left in place. This is annoying, but
also something that can happen occasionally anyway (kill -9, etc).
I don't have an easy suggestion for making it more atomic, though. You
could choose one or the other direction using some synchronization
between the two (e.g., child reassigns only after parent signals over a
pipe that it has relinquished), but it's all kind of ugly.
So it may be reasonable to just shrug and assume it's unlikely enough
not to worry about.
-Peff
^ permalink raw reply
* Re: [PATCH v2] ignore: note info/exclude lives in GIT_COMMON_DIR, not GIT_DIR
From: D. Ben Knoble @ 2026-05-11 19:55 UTC (permalink / raw)
To: phillip.wood
Cc: git, brian m . carlson, Patrick Steinhardt, Taylor Blau,
Caleb White, Calvin Wan, Junio C Hamano, Elijah Newren,
Andrew Berry, Jeff King, Derrick Stolee
In-Reply-To: <fd05eba1-d821-4cbe-ada6-ccab97308e56@gmail.com>
On Mon, May 11, 2026 at 6:30 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 08/05/2026 15:14, D. Ben Knoble wrote:
> > gitignore(5) says that the per-repository ignore file is
> > $GIT_DIR/info/exclude, but in a worktree that is not the case:
> >
> > git rev-parse --git-path info/exclude
> > /path/to/main/worktree/.git/info/exclude
> > git rev-parse --git-common-dir
> > /path/to/main/worktree/.git
> >
> > We actually use $GIT_COMMON_DIR/info/exclude. Adjust the documentation
> > to say so.
>
> Thanks for making the documentation match reality. Are there some more
> instances than need to be changed? If I run
>
> git grep -n GIT_DIR/info origin/master Documentation/gitignore.adoc
>
> I see
>
> origin/master:Documentation/gitignore.adoc:10:$XDG_CONFIG_HOME/git/ignore, $GIT_DIR/info/exclude, .gitignore
> origin/master:Documentation/gitignore.adoc:37: * Patterns read from `$GIT_DIR/info/exclude`.
> origin/master:Documentation/gitignore.adoc:53: the `$GIT_DIR/info/exclude` file.
> origin/master:Documentation/gitignore.adoc:100: such as $GIT_DIR/info/exclude and core.excludesFile, are treated as if
> origin/master:Documentation/gitignore.adoc:149:`$GIT_DIR/info/exclude`. Patterns in the exclude file are used in addition to
> origin/master:Documentation/gitignore.adoc:150:those in `$GIT_DIR/info/exclude`.
>
> We also have a ton of other files under Documentation that mention
> $GIT_DIR/info/... all of which, apart from the release notes, I
> think should probably be using $GIT_COMMON_DIR but we can always do
> that separately
>
> Thanks
>
> Phillip
Good catch, thanks. Let me adjust the ignore-related ones and leave
the rest for now.
^ permalink raw reply
* Re: [PATCH v3 05/12] path-walk: support blobless filter
From: Derrick Stolee @ 2026-05-11 19:44 UTC (permalink / raw)
To: Taylor Blau, Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps
In-Reply-To: <agIiL3jBvBBLHJWD@nand.local>
On 5/11/2026 2:38 PM, Taylor Blau wrote:
> On Mon, May 11, 2026 at 06:13:02PM +0000, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
>> index b78175fbe1..917045d5c3 100644
>> --- a/Documentation/git-pack-objects.adoc
>> +++ b/Documentation/git-pack-objects.adoc
>> @@ -402,9 +402,9 @@ will be automatically changed to version `1`.
>> of filenames that cause collisions in Git's default name-hash
>> algorithm.
>> +
>> -Incompatible with `--delta-islands`, `--shallow`, or `--filter`. The
>> -`--use-bitmap-index` option will be ignored in the presence of
>> -`--path-walk.`
>> +Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
>> +ignored in the presence of `--path-walk`. Whe `--path-walk` option
>> +supports the `--filter=<spec>` form `blob:none`.
>
> s/Whe/The/ ?
Yes, of course. This typo will follow the remaining patches in the context
lines. Will fix in v4.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v2 0/9] doc: interpret-trailers: explain key format
From: D. Ben Knoble @ 2026-05-11 19:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Kristoffer Haugsbakk, git, Christian Couder, jackmanb,
Linus Arver
In-Reply-To: <xmqq1pfivfa3.fsf@gitster.g>
Overall looks good to me. Repeating a few points throughout the doc
might create headaches if format restrictions are changed, but I think
they are essential points worth repeating for now.
On Sun, May 10, 2026 at 10:41 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>
> > On Mon, Apr 13, 2026, at 12:20, kristofferhaugsbakk@fastmail.com wrote:
> >> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> >>[snip]
> >
> > Sorry to Ben here who I forgot to set on copy. :/
> >
> > +Cc now.
>
> It has been quite a while and I have no recollection if there were
> still necessary adjustments or not. Is everybody happy with the
> final text?
>
> https://lore.kernel.org/git/V2_CV_doc_int-tr_key_format.613@msgid.xyz/
>
>
> Documentation/git-interpret-trailers.adoc | 68 +++++++++++++++++++++++--------
> 1 file changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/git-interpret-trailers.adoc b/Documentation/git-interpret-trailers.adoc
> index 77b4f63b05..b42f957d66 100644
> --- a/Documentation/git-interpret-trailers.adoc
> +++ b/Documentation/git-interpret-trailers.adoc
> @@ -3,7 +3,7 @@ git-interpret-trailers(1)
>
> NAME
> ----
> -git-interpret-trailers - Add or parse structured information in commit messages
> +git-interpret-trailers - Add or parse metadata in commit messages
>
> SYNOPSIS
> --------
> @@ -14,9 +14,15 @@ git interpret-trailers [--in-place] [--trim-empty]
>
> DESCRIPTION
> -----------
> -Add or parse _trailer_ lines that look similar to RFC 822 e-mail
> -headers, at the end of the otherwise free-form part of a commit
> -message. For example, in the following commit message
> +Add or parse trailers metadata at the end of the otherwise
> +free-form part of a commit message, or any other kind of text.
> +
> +A _trailer_ in its simplest form is a key-value pair with a colon as a
> +separator. The _key_ consists of ASCII alphanumeric characters and
> +hyphens (`-`). A _trailer block_ consists of one or more trailers. The
> +trailer block needs to be preceded by a blank line, where a _blank line_
> +is either an empty or a whitespace-only line. For example, in the
> +following commit message
>
> ------------------------------------------------
> subject
> @@ -81,19 +87,25 @@ trailer.sign.key "Signed-off-by: "
> in your configuration, you only need to specify `--trailer="sign: foo"`
> on the command line instead of `--trailer="Signed-off-by: foo"`.
>
> -By default the new trailer will appear at the end of all the existing
> -trailers. If there is no existing trailer, the new trailer will appear
> -at the end of the input. A blank line will be added before the new
> -trailer if there isn't one already.
> +By default the new trailer will appear at the end of the trailer block.
> +A trailer block will be created with only that trailer if a trailer
> +block does not already exist. Recall that a trailer block needs to be
> +preceded by a blank line, so a blank line (specifically an empty line)
> +will be inserted before the new trailer block in that case.
[not strictly related to this patch, but while we're here…]
Even in context, I find the original (and new) paragraph somewhat
jarring. In "the new trailer," there's no antecedent for "the
trailer", so which new trailer are we talking about? The previous
paragraph is about "<key-alias>es" for --trailer="<key>: value".
We _could_ move this paragraph up one, so that it follows the
paragraph on trailers being appended when given with --trailer.
Either way, adjusting "the new trailer" to "a new trailer" might feel
better to me. Other suggestions welcome.
> -Existing trailers are extracted from the input by looking for
> -a group of one or more lines that (i) is all trailers, or (ii) contains at
> -least one Git-generated or user-configured trailer and consists of at
> +Existing trailers are extracted from the input by looking for the
> +trailer block. Concretely, that is a group of one or more lines that (i)
> +is all trailers, or (ii) contains at least one Git-generated or
> +user-configured trailer and consists of at
> least 25% trailers.
> -The group must be preceded by one or more empty (or whitespace-only) lines.
> -The group must either be at the end of the input or be the last
> -non-whitespace lines before a line that starts with `---` (followed by a
> -space or the end of the line).
> +The trailer block is by definition at the end the the message. The end
> +of the message in turn is either (i) at the end of the input, or (ii)
> +the last non-whitespace lines before a line that starts with `---`
> +(followed by a space or the end of the line).
> +
> +This command ignores comment lines (see `core.commentString` in
> +linkgit:git-config[1]). This is for use with the `prepare-commit-msg`
> +and `commit-msg` hooks.
>
> When reading trailers, there can be no whitespace before or inside the
> _<key>_, but any number of regular space and tab characters are allowed
> @@ -107,9 +119,6 @@ key: This is a very long value, with spaces and
> newlines in it.
> ------------------------------------------------
>
> -Note that trailers do not follow (nor are they intended to follow) many of the
> -rules for RFC 822 headers. For example they do not follow the encoding rule.
> -
> OPTIONS
> -------
> `--in-place`::
> @@ -402,6 +411,29 @@ mv "\$1.new" "\$1"
> $ chmod +x .git/hooks/commit-msg
> ------------
>
> +* Here we try to to use three different trailer keys. But it fails
> + because two of them are not recognized as trailer keys.
> ++
> +----
> +$ cat msg.txt
> +subject
> +
> +Skapad-på: some-branch
> +Hash-in-v6.11: 45c12d3269fe48f22834320c782ffe86c3560f2c
> +Reviewed-by: Alice <alice@example.com>
> +$ git interpret-trailers --only-trailers <msg.txt
> +$
> +----
> ++
> +Recall that a trailer key has to consist of only ASCII alphanumeric
> +characters and hyphens, and this does not hold for the two first
> +supposed trailer keys. And now none are recognized as trailers because
> +the candidate trailer block has at least one non-trailer line, even
> +though `Reviewed-by` is a valid trailer key. Recall that a trailer block
> +has to either (i) be all trailers, or (ii) consist of at least one
> +Git-generated or user-configured trailer (and some other conditions).
> +And (ii) is not satisfied since we have not configured any trailer keys.
> +
> SEE ALSO
> --------
> linkgit:git-commit[1], linkgit:git-format-patch[1], linkgit:git-config[1]
--
D. Ben Knoble
^ permalink raw reply
* [PATCH v2] shallow: fix relative deepen on non-shallow repositories
From: Samo Pogačnik @ 2026-05-11 19:20 UTC (permalink / raw)
To: git; +Cc: l.s.r, owen, Samo Pogačnik, Junio C Hamano
In-Reply-To: <xmqqy0hqqreq.fsf@gitster.g>
The commit "3ef68ff40e (shallow: handling fetch relative-deepen,
2026-02-15)" introduced a bug where using --deepen=<n> on a non-
shallow repository incorrectly treated the value as an absolute
depth, resulting in a shallow fetch and truncated history.
This patch prevents any modification when a relative deepen is
requested on a non-shallow repository.
A test is added to ensure that history is not changed when
--deepen is used on a non-shallow repository.
Reported-by: Owen Stephens <owen@owenstephens.co.uk>
Signed-off-by: Samo Pogačnik <samo_pogacnik@t-2.net>
---
Changes since v1:
- Fixed commit reference in the commit message.
shallow.c | 6 +++++-
t/t5537-fetch-shallow.sh | 10 ++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/shallow.c b/shallow.c
index a8ad92e303..610ff3d13b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -245,7 +245,11 @@ struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag)
{
if (shallows && deepen_relative) {
- depth += get_shallows_depth(heads, shallows);
+ int cur_shallow_depth = get_shallows_depth(heads, shallows);
+ if (cur_shallow_depth)
+ depth += cur_shallow_depth;
+ else
+ return NULL;
}
return get_shallows_or_depth(heads, NULL, NULL,
depth, shallow_flag, not_shallow_flag);
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 6588ce6226..9982dd2aa6 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -251,6 +251,16 @@ test_expect_success '.git/shallow is edited by repack' '
origin "+refs/heads/*:refs/remotes/origin/*"
'
+test_expect_success 'fetch --deepen does not truncate' '
+ git clone --no-local .git full-clone &&
+ git -C full-clone rev-parse --is-shallow-repository >expect &&
+ git -C full-clone log --oneline >>expect &&
+ git -C full-clone fetch --deepen=1 &&
+ git -C full-clone rev-parse --is-shallow-repository >actual &&
+ git -C full-clone log --oneline >>actual &&
+ test_cmp expect actual
+'
+
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] add -p: introduce 'w' command to view hunk with --word-diff
From: D. Ben Knoble @ 2026-05-11 19:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phillip Wood, Javier Bassi, git, Abraham Samuel Adekunle,
Rene Scharfe, Elijah Newren, Ruben Justo, Patrick Steinhardt
In-Reply-To: <xmqqqznix0ko.fsf@gitster.g>
On Sun, May 10, 2026 at 8:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
>
> > I concur, though I wonder what other diff options would be useful. I
> > can imagine a world where "add --patch" allows to reshow the patch
> > with any set of custom options. Spitballing…
> >
> > :show <opts>
> >
> > …could work, since we have room to allow long-form commands still.
> > This could be extended to arbitrary git-* commands, and ":!cmd" could
> > be used for shelling out.
> > (Can you tell I use Vim?)
> >
> > Anyway, what I like about this is we don't have to worry about running
> > out of (or thinking up new mnemonics for) single letters for different
> > combinations of diff options, so it's a bit more extensible.
>
>
> If you mean "don't waste a new letter, like 'w', every time you come
> up with a use case to show the patch hunk differently",
Precisely
> I had the
> same reaction to suggest reusing 'p' but allow options after it,
> just like the output from "git diff" can be modified by giving it an
> option "--word-diff". But how would it work for single-key folks (I
> am not one of them so I have less sympathy than I should in this
> case, but still...).
Ah, I hadn't remembered we already had p/P. Perhaps
- in non-single-key mode, p/P can take options.
and/or
- ":" is an exception to single-key mode and always allows more text
to compose a longer-form "command". The first command could be "print"
which takes options of this kind.
Since ":" would be new, exempting it from single-key mode shouldn't be
too jarring I guess.
--
D. Ben Knoble
^ permalink raw reply
* Re: [PATCH v3 05/12] path-walk: support blobless filter
From: Taylor Blau @ 2026-05-11 18:38 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: <b2deb7f81939d6ed03f0ea5c8be5ec515c1c99ab.1778523189.git.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 06:13:02PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
> index b78175fbe1..917045d5c3 100644
> --- a/Documentation/git-pack-objects.adoc
> +++ b/Documentation/git-pack-objects.adoc
> @@ -402,9 +402,9 @@ will be automatically changed to version `1`.
> of filenames that cause collisions in Git's default name-hash
> algorithm.
> +
> -Incompatible with `--delta-islands`, `--shallow`, or `--filter`. The
> -`--use-bitmap-index` option will be ignored in the presence of
> -`--path-walk.`
> +Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
> +ignored in the presence of `--path-walk`. Whe `--path-walk` option
> +supports the `--filter=<spec>` form `blob:none`.
s/Whe/The/ ?
Thanks,
Taylor
^ permalink raw reply
* [PATCH v3 12/12] path-walk: support `combine` filter
From: Taylor Blau via GitGitGadget @ 2026-05-11 18:13 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Taylor Blau
In-Reply-To: <pull.2101.v3.git.1778523189.gitgitgadget@gmail.com>
From: Taylor Blau <me@ttaylorr.com>
The `combine` filter takes the intersection of its children, that is:
objects are shown only when all child filters would admit the object.
The preceding patches added support for many individual filter types.
Enable users to compose these filters by implementing support for the
`combine` filter type.
Mapping intersection onto path_walk_info works because every supported
child filter is a monotonic restriction:
- `blob:none`, `tree:0` unconditionally clear `info->blobs` and (for
`tree:0`) `info->trees`; clearing an already-cleared flag is a
no-op.
- `object:type=X` is now expressed as an AND of each type flag with the
filtered type, so applying multiple such filters only refines the
existing set rather than overwrites it.
- `blob:limit=N` has to compose too: the intersection of "size < L1"
and "size < L2" is "size < min(L1, L2)".
Update the `LOFC_BLOB_LIMIT` handler to take the running minimum when
`info->blob_limit` is already set, so a combined filter with, e.g.,
both "blob:limit=10" and "blob:limit=5" produces a limit of 5
regardless of ordering.
- `sparse:oid` is left unchanged. A `combine` filter that includes a
`sparse:oid` is allowed at most once, since the existing handler
refuses to overwrite `info->pl`. Two `sparse:oid` filters in a single
`combine` would be unusual and are rejected with a warning, matching
the standalone `sparse:oid` behavior.
Implementation-wise, the existing `prepare_filters()` called
`list_objects_filter_release()` inside each case branch. That works fine for
top-level filters, but `combine` filters need to recurse over its child
filters without releasing each one in turn (since the parent's release
iterates the sub array). Split `prepare_filters()` into a recursive helper
that performs only the mutation, plus a thin wrapper that calls the helper
and then releases the top-level filter once.
The `LOFC_COMBINE` case in the helper just walks `sub_nr` and recurses;
child filters are released by the wrapper's single
`list_objects_filter_release()` call on the parent (which itself recursively
releases each sub-filter, the same way it always has).
If any sub-filter is unsupported (e.g. "tree:1", "sparse:<path>", or a
not-yet-supported choice), the recursion bubbles a failure up and the
existing pack-objects/backfill fallback paths kick in.
Add coverage in t6601:
- "combine:blob:none+tree:0" collapses to "tree:0"
- "combine:object:type=blob+blob:limit=3" yields only the blobs
smaller than three bytes
- "combine:object:type=blob+object:type=tree" intersects to empty
- "combine:tree:1+blob:none" reports the "tree:1" error.
Update Documentation/git-pack-objects.adoc to add combine to the
list of supported --filter forms.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git-pack-objects.adoc | 3 +-
path-walk.c | 25 ++++++++--
t/t6601-path-walk.sh | 71 +++++++++++++++++++++++++++++
3 files changed, 93 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index e96d2c00b3..2fd79f13ad 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -405,7 +405,8 @@ will be automatically changed to version `1`.
Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
ignored in the presence of `--path-walk`. Whe `--path-walk` option
supports the `--filter=<spec>` forms `blob:none`, `blob:limit=<n>`,
-`tree:0`, `object:type=<type>`, and `sparse:<oid>`.
+`tree:0`, `object:type=<type>`, and `sparse:<oid>`. These supported filter
+types can be combined with the `combine:<spec>+<spec>` form.
DELTA ISLANDS
diff --git a/path-walk.c b/path-walk.c
index 839505839b..3c67b359f9 100644
--- a/path-walk.c
+++ b/path-walk.c
@@ -573,8 +573,8 @@ static int setup_pending_objects(struct path_walk_info *info,
return 0;
}
-static int prepare_filters(struct path_walk_info *info,
- struct list_objects_filter_options *options)
+static int prepare_filters_one(struct path_walk_info *info,
+ struct list_objects_filter_options *options)
{
switch (options->choice) {
case LOFC_DISABLED:
@@ -591,7 +591,8 @@ static int prepare_filters(struct path_walk_info *info,
if (info) {
if (!options->blob_limit_value)
info->blobs = 0;
- else
+ else if (!info->blob_limit ||
+ info->blob_limit > options->blob_limit_value)
info->blob_limit = options->blob_limit_value;
list_objects_filter_release(options);
}
@@ -606,7 +607,6 @@ static int prepare_filters(struct path_walk_info *info,
if (info) {
info->trees = 0;
info->blobs = 0;
- list_objects_filter_release(options);
}
return 1;
@@ -658,8 +658,13 @@ static int prepare_filters(struct path_walk_info *info,
warning(_("sparse filter is not cone-mode compatible"));
return 0;
}
+ }
+ return 1;
- list_objects_filter_release(options);
+ case LOFC_COMBINE:
+ for (size_t i = 0; i < options->sub_nr; i++) {
+ if (!prepare_filters_one(info, &options->sub[i]))
+ return 0;
}
return 1;
@@ -670,6 +675,16 @@ static int prepare_filters(struct path_walk_info *info,
}
}
+static int prepare_filters(struct path_walk_info *info,
+ struct list_objects_filter_options *options)
+{
+ if (!prepare_filters_one(info, options))
+ return 0;
+ if (info)
+ list_objects_filter_release(options);
+ return 1;
+}
+
int path_walk_filter_compatible(struct list_objects_filter_options *options)
{
return prepare_filters(NULL, options);
diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh
index 0515aaddbf..643d630d2a 100755
--- a/t/t6601-path-walk.sh
+++ b/t/t6601-path-walk.sh
@@ -729,6 +729,77 @@ test_expect_success 'all, object:type=blob filter' '
test_cmp_sorted expect out
'
+test_expect_success 'all, combine:blob:none+tree:0 filter' '
+ test-tool path-walk \
+ --filter=combine:blob:none+tree:0 -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:commit::$(git rev-parse topic)
+ 0:commit::$(git rev-parse base)
+ 0:commit::$(git rev-parse base~1)
+ 0:commit::$(git rev-parse base~2)
+ 1:tag:/tags:$(git rev-parse refs/tags/first)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.1)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.2)
+ 1:tag:/tags:$(git rev-parse refs/tags/third)
+ 1:tag:/tags:$(git rev-parse refs/tags/fourth)
+ 1:tag:/tags:$(git rev-parse refs/tags/tree-tag)
+ 1:tag:/tags:$(git rev-parse refs/tags/blob-tag)
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag^{})
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag2^{})
+ 3:tree:/tagged-trees:$(git rev-parse refs/tags/tree-tag^{tree})
+ 3:tree:/tagged-trees:$(git rev-parse refs/tags/tree-tag2)
+ blobs:2
+ commits:4
+ tags:7
+ trees:2
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'all, combine:object:type=blob+blob:limit=3 filter' '
+ test-tool path-walk \
+ --filter=combine:object:type=blob+blob:limit=3 \
+ -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag^{})
+ 0:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag2^{})
+ 1:blob:a:$(git rev-parse base~2:a)
+ 2:blob:left/b:$(git rev-parse base~2:left/b)
+ 3:blob:right/c:$(git rev-parse base~2:right/c)
+ 4:blob:right/d:$(git rev-parse base~1:right/d)
+ blobs:6
+ commits:0
+ tags:0
+ trees:0
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'all, combine of disjoint object:types is empty' '
+ test-tool path-walk \
+ --filter=combine:object:type=blob+object:type=tree \
+ -- --all >out &&
+
+ cat >expect <<-EOF &&
+ blobs:0
+ commits:0
+ tags:0
+ trees:0
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'combine: rejects unsupported subfilters' '
+ test_must_fail test-tool path-walk \
+ --filter=combine:tree:1+blob:none -- --all 2>err &&
+ test_grep "tree:1 filter not supported by the path-walk API" err
+'
+
test_expect_success 'setup sparse filter blob' '
# Cone-mode patterns: include root, exclude all dirs, include left/
cat >patterns <<-\EOF &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 11/12] path-walk: support `object:type` filter
From: Taylor Blau via GitGitGadget @ 2026-05-11 18:13 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Taylor Blau
In-Reply-To: <pull.2101.v3.git.1778523189.gitgitgadget@gmail.com>
From: Taylor Blau <me@ttaylorr.com>
The `object:type` filter accepts only objects of a single type; it is
the second member of the object-info-only filter family that bitmap
traversal already supports.
Like `blob:none` and `tree:0`, it can be evaluated with nothing more
than the object's type, which is exactly the granularity path-walk's
existing info->{commits,trees,blobs,tags} flags already control.
Map `LOFC_OBJECT_TYPE` in `prepare_filters()` by AND-ing each flag
against the filtered type. A single `object:type=X` filter
applied to the default info (all flags = 1) leaves `info->X = 1` and
all the others 0, which is what we want.
Using an AND rather than straight assignment prepares us for a
subsequent change to implement combined object filters.
The path-walk machinery is mostly already wired for the per-type
distinction:
- `walk_path()` calls `path_fn` for a batch only when the corresponding
`info->X` flag is set, so unwanted types are silently not reported.
- `add_tree_entries()` skips tree entries of type `OBJ_BLOB` when
`info->blobs` is unset, so we don't even allocate paths for them.
- The commit-walk loop short-circuits the root-tree fetch when
`!info->trees && !info->blobs`, so commit-only filters don't descend
into trees at all.
But there are a couple of side effects of the "trees off, blobs on" case
that need fixing:
1. 'setup_pending_objects()' previously skipped pending trees as soon
as `info->trees` was zero. For 'object:type=blob' the call site
needs those pending trees: a lightweight tag pointing to a tree, or
an annotated tag whose peeled target is a tree, can both reach
blobs that are otherwise unreachable from any commit's root tree.
Loosen the gate to "if (!info->trees && !info->blobs) continue" and
similarly retrieve the root_tree_list whenever either trees or
blobs are wanted.
2. The revision machinery's `handle_commit()` drops pending trees when
`revs->tree_objects` is zero (see the 'OBJ_TREE' handler in
revision.c), so by the time path-walk sees the pending list
after `prepare_revision_walk()` the tree-bearing pendings would
already be gone. Fix this by setting
revs->tree_objects = info->trees || info->blobs
so pending trees survive `prepare_revision_walk()` whenever we
need to walk into them. Path-walk still resets tree_objects to
zero immediately after `prepare_revision_walk()` returns, so the
rev-walk itself never enumerates trees redundantly with
path-walk's own descent.
Add coverage in t6601 for each of the four `object:type` values. The
'object:type=blob' test in particular asserts that file2 and child/file
(both reachable only through tag-pointed trees) show up in the output,
exercising the pending-tree fix.
Update Documentation/git-pack-objects.adoc to add object:type to
the list of supported --filter forms.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git-pack-objects.adoc | 2 +-
path-walk.c | 13 ++++-
path-walk.h | 6 +++
t/t6601-path-walk.sh | 84 +++++++++++++++++++++++++++++
4 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 4ebda0fa9e..e96d2c00b3 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -405,7 +405,7 @@ will be automatically changed to version `1`.
Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
ignored in the presence of `--path-walk`. Whe `--path-walk` option
supports the `--filter=<spec>` forms `blob:none`, `blob:limit=<n>`,
-`tree:0`, and `sparse:<oid>`.
+`tree:0`, `object:type=<type>`, and `sparse:<oid>`.
DELTA ISLANDS
diff --git a/path-walk.c b/path-walk.c
index f6f95da550..839505839b 100644
--- a/path-walk.c
+++ b/path-walk.c
@@ -381,7 +381,7 @@ static int walk_path(struct path_walk_context *ctx,
ret = ctx->info->path_fn(path, &filtered, list->type,
ctx->info->path_fn_data);
oid_array_clear(&filtered);
- } else if (path_is_for_direct_objects(path) ||
+ } else if ((!ctx->info->strict_types && path_is_for_direct_objects(path)) ||
(list->type == OBJ_TREE && ctx->info->trees) ||
(list->type == OBJ_BLOB && ctx->info->blobs) ||
(list->type == OBJ_TAG && ctx->info->tags)) {
@@ -610,6 +610,17 @@ static int prepare_filters(struct path_walk_info *info,
}
return 1;
+ case LOFC_OBJECT_TYPE:
+ if (info) {
+ info->commits &= options->object_type == OBJ_COMMIT;
+ info->tags &= options->object_type == OBJ_TAG;
+ info->trees &= options->object_type == OBJ_TREE;
+ info->blobs &= options->object_type == OBJ_BLOB;
+ info->strict_types = 1;
+ list_objects_filter_release(options);
+ }
+ return 1;
+
case LOFC_SPARSE_OID:
if (info) {
struct object_id sparse_oid;
diff --git a/path-walk.h b/path-walk.h
index 7e57ae5f65..a2652b2d46 100644
--- a/path-walk.h
+++ b/path-walk.h
@@ -47,6 +47,12 @@ struct path_walk_info {
int blobs;
int tags;
+ /**
+ * If 'strict_types' is 0, then direct object requests will no longer
+ * override the object type restrictions.
+ */
+ int strict_types;
+
/**
* If non-zero, specifies a maximum blob size. Blobs with a
* size equal to or greater than this limit will not be
diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh
index deaeab9a3e..0515aaddbf 100755
--- a/t/t6601-path-walk.sh
+++ b/t/t6601-path-walk.sh
@@ -645,6 +645,90 @@ test_expect_success 'tree:1 filter is rejected' '
test_grep "tree:1 filter not supported by the path-walk API" err
'
+test_expect_success 'all, object:type=commit filter' '
+ test-tool path-walk --filter=object:type=commit -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:commit::$(git rev-parse topic)
+ 0:commit::$(git rev-parse base)
+ 0:commit::$(git rev-parse base~1)
+ 0:commit::$(git rev-parse base~2)
+ blobs:0
+ commits:4
+ tags:0
+ trees:0
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'all, object:type=tag filter' '
+ test-tool path-walk --filter=object:type=tag -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:tag:/tags:$(git rev-parse refs/tags/first)
+ 0:tag:/tags:$(git rev-parse refs/tags/second.1)
+ 0:tag:/tags:$(git rev-parse refs/tags/second.2)
+ 0:tag:/tags:$(git rev-parse refs/tags/third)
+ 0:tag:/tags:$(git rev-parse refs/tags/fourth)
+ 0:tag:/tags:$(git rev-parse refs/tags/tree-tag)
+ 0:tag:/tags:$(git rev-parse refs/tags/blob-tag)
+ blobs:0
+ commits:0
+ tags:7
+ trees:0
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'all, object:type=tree filter' '
+ test-tool path-walk --filter=object:type=tree -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:tree::$(git rev-parse topic^{tree})
+ 0:tree::$(git rev-parse base^{tree})
+ 0:tree::$(git rev-parse base~1^{tree})
+ 0:tree::$(git rev-parse base~2^{tree})
+ 0:tree::$(git rev-parse refs/tags/tree-tag^{})
+ 0:tree::$(git rev-parse refs/tags/tree-tag2^{})
+ 1:tree:a/:$(git rev-parse base:a)
+ 2:tree:child/:$(git rev-parse refs/tags/tree-tag:child)
+ 3:tree:left/:$(git rev-parse base:left)
+ 3:tree:left/:$(git rev-parse base~2:left)
+ 4:tree:right/:$(git rev-parse topic:right)
+ 4:tree:right/:$(git rev-parse base~1:right)
+ 4:tree:right/:$(git rev-parse base~2:right)
+ blobs:0
+ commits:0
+ tags:0
+ trees:13
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'all, object:type=blob filter' '
+ test-tool path-walk --filter=object:type=blob -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag^{})
+ 0:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag2^{})
+ 1:blob:a:$(git rev-parse base~2:a)
+ 2:blob:left/b:$(git rev-parse base:left/b)
+ 2:blob:left/b:$(git rev-parse base~2:left/b)
+ 3:blob:right/c:$(git rev-parse base~2:right/c)
+ 3:blob:right/c:$(git rev-parse topic:right/c)
+ 4:blob:right/d:$(git rev-parse base~1:right/d)
+ blobs:8
+ commits:0
+ tags:0
+ trees:0
+ EOF
+
+ test_cmp_sorted expect out
+'
+
test_expect_success 'setup sparse filter blob' '
# Cone-mode patterns: include root, exclude all dirs, include left/
cat >patterns <<-\EOF &&
--
gitgitgadget
^ permalink raw reply related
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