* [PATCH v2 0/4] doc: replay: fix config link
From: kristofferhaugsbakk @ 2026-06-03 16:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Siddharth Asthana, git
In-Reply-To: <CV_doc_replay_config.709@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Topic name (applied): kh/doc-replay-config
Topic summary: link to the config for git-replay(1) (one variable) in
git-replay(1) and git-config(1). Also improve the doc for that config
variable and `--ref-action`.
§ Changes in v2
See the notes on the patches for more points and details.
• Keep the description list for `replay.refAction` (Junio)
• Add a comment on both description lists about the fact that
the two are similar
[1/4] doc: link to config for git-replay(1)
[2/4] doc: replay: improve config description
[3/4] doc: replay: use a nested description list
[4/4] doc: replay: move “default” to the right-hand side
Documentation/config.adoc | 2 ++
Documentation/config/replay.adoc | 19 +++++++++++++------
Documentation/git-replay.adoc | 16 ++++++++++++----
3 files changed, 27 insertions(+), 10 deletions(-)
Interdiff against v1:
diff --git a/Documentation/config/replay.adoc b/Documentation/config/replay.adoc
index 42e521694d1..40d1695782a 100644
--- a/Documentation/config/replay.adoc
+++ b/Documentation/config/replay.adoc
@@ -1,5 +1,15 @@
replay.refAction::
- Specifies the default mode for handling reference updates. Either `update` or `print`.
+ Specifies the default mode for handling reference updates.
+ The value can be:
++
+--
+////
+These use the first sentences from the description list in git-replay(1).
+////
+`update`;; (default) Update refs directly using an atomic transaction.
+`print`;; Output update-ref commands for pipeline use.
+--
++
ifdef::git-replay[]
See `--ref-action`.
endif::git-replay[]
diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
index 39ecc2e1876..ea4d14baddb 100644
--- a/Documentation/git-replay.adoc
+++ b/Documentation/git-replay.adoc
@@ -80,6 +80,9 @@ incompatible with `--contained` (which is a modifier for `--onto` only).
Control how references are updated. The mode can be:
+
--
+////
+Expanded description list compared to 'replay.refAction'.
+////
`update`;; (default) Update refs directly using an atomic transaction.
All refs are updated or none are (all-or-nothing behavior).
`print`;; Output update-ref commands for pipeline use. This is the
Range-diff against v1:
1: ef8212a076a = 1: ef8212a076a doc: link to config for git-replay(1)
2: 7e915e331b5 ! 2: b60e2e02826 doc: replay: simplify replay.refAction description
@@ Metadata
Author: Kristoffer Haugsbakk <code@khaugsbakk.name>
## Commit message ##
- doc: replay: simplify replay.refAction description
+ doc: replay: improve config description
- We don’t need to list what each argument does since the documentation
- for `--ref-action` does that. So let’s simplify the `replay.refAction`
- description by referring to git-replay(1).
+ First of all, this bullet list for `--ref-action` introduces a term with
+ a colon. This is exactly what a description list is, structurally. Let’s
+ be sylistically consistent and use the description list markup
+ construct. Let’s also drop the harmless but unneeded indentation.
- Also make sure to not self-link for the git-replay(1) inclusion.
+ Second, let’s replace the inline-verbatim `git replay` with a link
+ to git-replay(1), since we are naming the command. But make that
+ conditional so that we avoid a self-link inside git-replay(1).[1]
+
+ † 1: See e.g. e7b3a768 (doc: git-init: rework config item
+ init.templateDir, 2024-03-10) for another example of
+ avoiding self-linking
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
@@ Documentation/config/replay.adoc
replay.refAction::
- Specifies the default mode for handling reference updates in
- `git replay`. The value can be:
--+
----
++ Specifies the default mode for handling reference updates.
++ The value can be:
+ +
+ --
- * `update`: Update refs directly using an atomic transaction (default behavior).
- * `print`: Output update-ref commands for pipeline use.
----
--+
++`update`;; Update refs directly using an atomic transaction (default behavior).
++`print`;; Output update-ref commands for pipeline use.
+ --
+ +
-This setting can be overridden with the `--ref-action` command-line option.
-When not configured, `git replay` defaults to `update` mode.
-+ Specifies the default mode for handling reference updates. Either `update` or `print`.
+ifdef::git-replay[]
+See `--ref-action`.
+endif::git-replay[]
3: 30952387f35 ! 3: d13cd39cb36 doc: replay: use a nested definition list
@@ Metadata
Author: Kristoffer Haugsbakk <code@khaugsbakk.name>
## Commit message ##
- doc: replay: use a nested definition list
+ doc: replay: use a nested description list
This bullet list for `--ref-action` introduces a term with a colon.
- This is exactly what a definition list is, structurally. Let’s be
- sylistically consistent and use the definition list markup construct.
+ This is exactly what a description list is, structurally. Let’s be
+ sylistically consistent and use the desc. list markup construct.[1]
We can reuse the `::` delimiter since we use an open block.
- But for consistency use the typical nested definition list
+ But for consistency use the typical nested description list
delimiter, namely `;;`.
Also drop the harmless but unneeded indentation.
+ † 1: Same explanation as in the previous commit
+
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
## Documentation/git-replay.adoc ##
4: 71a61bc0ed4 < -: ----------- doc: replay: move “default” to the right-hand-side
-: ----------- > 4: 17804ea7afa doc: replay: move “default” to the right-hand side
base-commit: a89346e34a937f001e5d397ee62224e3e9852040
--
2.54.0.22.g9e26862b904
^ permalink raw reply related
* Re: [PATCH] t3070: skip ls-files tests with backslash patterns on Windows
From: Junio C Hamano @ 2026-06-03 14:23 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Kristofer Karlsson via GitGitGadget, git, Johannes Schindelin
In-Reply-To: <CAL71e4MLyEEWtrHvB2K+stZUq6s+5sQUpSjmos3F9aVo3ej=Fw@mail.gmail.com>
Kristofer Karlsson <krka@spotify.com> writes:
> On Thu, 28 May 2026 at 22:26, Junio C Hamano <gitster@pobox.com> wrote:
>> Two questions.
>>
>> * Has this been broken on Windows since October, or has something
>> external change on Windows recently? I do not know. Anybody
>> knows?
>>
>> * Is this change a workaround that sweeps ugly breakage under the
>> rug, or is backslash inherently unusable as an excape character
>> when handling paths on Windows (which I am afraid would make
>> wildmatch fairly useless there)?
>>
>
> I am fairly new to the git ecosystem as a developer (not as a user),
> so I am not sure how long this has been broken. The backslash patterns
> in the ls-files test path predate 8a6d158a - patterns like 'foo\*'
> and '[\-_]' have been there since de8bada2bf (2018) - so it may
> have been failing for a while before anyone noticed.
Hmph.
> My thinking was that it would be good in general if the CI results
> were green and did not include false positives for errors that we
> know cannot work on this platform. The risk is that people stop
> looking into CI failures in detail because they start to assume it
> is the same old backslash problem.
Oh, no question about that.
> That said, there is also a risk that the real underlying issue does
> not get fixed. I am hoping it is sufficient that the BSLASHPSPEC
> prereq and the case *\\* filter make it obvious to anyone reading
> the test what we are skipping over and why.
>
>> Will queue. Thanks.
>
> Thanks! It felt a bit heavyweight to add noise to the list for trivial CI test
> changes but I suppose the process is the same even if it does not
> affect the production code.
Sure. I just found it a little disturbing to declare that there
won't be ways on Windows to quote special letters with backslash
when writing wildmatch patterns. But if some Windows folks got
motivated enough, they are capable of lifting the prereq when they
fix the underlying code as well, so it probably is not something I
should be worrying too much about.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 3/3] b4: introduce configuration for the Git project
From: Toon Claes @ 2026-06-03 13:58 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-3-a8aea0aa2c23@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> We're about to extend our documentation to recommend b4 for sending
> patch series to the mailing list. Prepare for this by introducing a b4
> configuration so that the tool knows to honor our preferences. For now,
> this configuration does two things:
>
> - It configures "send-same-thread = shallow", which tells b4 to always
> send subsequent versions of the same patch series as a reply to the
> cover letter of the first version.
>
> - It configures "prep-cover-template", which tells b4 to use a custom
> template for the cover letter. The most important change compared to
> the default template is that our custom template also includes a
> range-diff.
>
> There's potentially more things that we may want to configure going
> forward, like for example auto-configuration of folks to Cc on certain
> patches. But these two tweaks feel like a good place to start.
>
> Note that these values only serve as defaults, and users may want to
> tweak those defaults based on their own preference. Luckily, users can
> do that without having to touch `.b4-config` at all, as b4 allows them
> to override values via Git configuration:
>
> ```
> $ git config set b4.prep-cover-template /does/not/exist
> $ b4 send --dry-run
> ERROR: prep-cover-template says to use x, but it does not exist
> ```
>
> So this gives users an easy way to override our defaults without having
> to touch ".b4-config", which would dirty the tree.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> .b4-config | 6 ++++++
> .b4-cover-template | 11 +++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/.b4-config b/.b4-config
> new file mode 100644
> index 0000000000..fd4fb56b6d
> --- /dev/null
> +++ b/.b4-config
> @@ -0,0 +1,6 @@
> +# Note that these are default values that you can tweak via the typical
> +# git-config(1) machinery. You thus shouldn't ever have to change this file.
> +# See also https://b4.docs.kernel.org/en/latest/config.html.
> +[b4]
> +send-same-thread = shallow
Is it worth to note this requires v0.15 or higher?
That version was released only 2 months ago, I can imagine many distros
still ship an older version, what happens if a version doesn't support
this setting yet?
> +prep-cover-template = ./.b4-cover-template
> diff --git a/.b4-cover-template b/.b4-cover-template
> new file mode 100644
> index 0000000000..ab864933b5
> --- /dev/null
> +++ b/.b4-cover-template
> @@ -0,0 +1,11 @@
> +${cover}
> +
> +---
> +${shortlog}
> +
> +${diffstat}
> +
> +${range_diff}
> +---
> +base-commit: ${base_commit}
> +${prerequisites}
>
> --
> 2.54.0.1064.gd145956f57.dirty
>
>
--
Cheers,
Toon
^ permalink raw reply
* Re: Suggetsions for collaboration workflows in large repos
From: Toon Claes @ 2026-06-03 13:44 UTC (permalink / raw)
To: Matthew Hughes, git
In-Reply-To: <ahnUeESE1x802Z9N@desktop>
Matthew Hughes <matthewhughes934@gmail.com> writes:
> On Fri, May 29, 2026 at 05:31:17PM +0100, Matthew Hughes wrote:
>> I thought about doing something like tracking
>> `refs/heads*/some-colleague-branch` from the remote, since with the wildcard
>> `*` I at least won't the fatal error on the missing reference during fetch, but
>> that risks my config containing an ever growing list of such wildcards, or a
>> bunch of manual work occasionally cleaning up old ones (or maybe that could be
>> automated).
I feel your problem, although a lot less in the project I'm working on
lately. I have these refspecs by the way:
fetch = +refs/heads/master:refs/remotes/origin/master
fetch = +refs/heads/toon-*:refs/remotes/origin/toon-*
> I hacked some scripts to automate this. Firstly, one for fetching:
>
> 1. Fetches the branch
> 2. Adds a fetch config with wildcard hacks so `git fetch` brings in updates for
> that branch (the refspec should match _exactly_ that branch and never
> anything more)
> 3. Adds a separate ref to record that we're tracking this branch (so something
> knows to clean it up later)
>
> #!/usr/bin/env bash
>
> set -o errexit -o pipefail -o nounset
>
> # save command as e.g. git-fetch-other
> CMD_NAME="$(basename "$0" | sed 's/git-//g')"
> if [ $# -lt 1 ]
> then
> echo "usage: git $CMD_NAME branch-name [ remote-name ]" >&2
> exit 1
> fi
>
> BRANCH_NAME="$1"
> REMOTE_NAME="${2:-origin}"
> FETCH_CONFIG_NAME="remote.$REMOTE_NAME.fetch"
>
> git fetch "$REMOTE_NAME" "$BRANCH_NAME"
> git checkout -b "$BRANCH_NAME"
>
> # we want to record that we are tracking this branch, to do this create
> # a new ref whose name tells us what we're tracking, but whose value is
> # unimportant. So as a placeholder value just use the hash of an empty tree
> # taken from https://git.kernel.org/pub/scm/git/git.git/commit/?id=9c8a294a1ae1335511475db9c0eb8841c0ec9738
> EMPTY_TREE_REF="$(git hash-object -t tree /dev/null)"
>
> # refspec used to track the branch: we expect branches to be deleted from the
> # upstream when merged so tracking exactly:
> # "+refs/heads/$BRANCH_NAME:refs/remotes/$REMOTE_NAME/$BRANCH_NAME" will error
> # when we go to fetch that exact ref after its removed upstream.
> # so HACK around this: add wildcards that we still expect to only ever match
> # this exact branch (but doesn't have the issue of git complaining when it
> # tries to fetch an _exact_ ref)
> TRACKING_REFSPEC="+refs/heads*/$BRANCH_NAME:refs/remotes*/$REMOTE_NAME/$BRANCH_NAME"
>
> # record that we're tracking this branch. First check we've not already
> # recorded this, then ...
> if ! git config get --local --fixed-value --value "$TRACKING_REFSPEC" "$FETCH_CONFIG_NAME" >/dev/null
> then
> # ... set the config to track it for fetching, and ...
> git config set --comment "$CMD_NAME: tracking at $(date -I)" --local --append "$FETCH_CONFIG_NAME" "$TRACKING_REFSPEC"
> # ... record that we have special cased this tracking
> git update-ref "refs/tracked/$REMOTE_NAME/$BRANCH_NAME" "$EMPTY_TREE_REF"
> fi
It seems to be a bit more advanced than the alias I have:
cofetch = !sh -c 'git fetch $1 $2:remotes/$1/$2 && git switch -c $2 remotes/$1/$2' -
You need to pass it the remote and the branch name (in reverse order of
yours, which makes sense if you want the remote to be optional).
> And the cleanup script (needs to be run periodically):
>
> 1. Collects all the remote branches we know about
> 2. Checks all the references from step 3. above and checks if any branches
> defined there are missing remotes (I have fetch.prune=true to keep the remote
> tracking references up-to-date)
> 3. If they are, drops the tracking config for that branch
>
> #!/usr/bin/env bash
>
> set -o errexit -o pipefail -o nounset
>
> REMOTE_NAME="${1:-origin}"
> TRACKED_REF_PREFIX="refs/tracked/$REMOTE_NAME"
> REMOTE_REF_PREFIX="refs/remotes/$REMOTE_NAME"
>
> declare -A remote_branch_lookup
> while read -r remote_ref
> do
> # strip prefix, e.g. 'refs/remotes/origin/some-branch' -> 'some-branch'
> branch_name="${remote_ref#$REMOTE_REF_PREFIX/}"
> remote_branch_lookup["$branch_name"]=1
> done < <(git for-each-ref --format='%(refname)' "$REMOTE_REF_PREFIX/")
>
> while read -r tracking_info
> do
> tracked_branch="${tracking_info#$TRACKED_REF_PREFIX/}"
> if ! [[ -v "remote_branch_lookup[$tracked_branch]" ]]
> then
> echo "branch $tracked_branch has been removed from the remote, untracking it"
> git update-ref -d "$TRACKED_REF_PREFIX/$tracked_branch"
>
> tracking_refspec="+refs/heads*/$tracked_branch:refs/remotes*/$REMOTE_NAME/$tracked_branch"
> git config unset --local --fixed-value --value "$tracking_refspec" "remote.$REMOTE_NAME.fetch"
> fi
> done < <(git for-each-ref --format='%(refname)' "$TRACKED_REF_PREFIX/")
>
> So functionally I think this allows for the workflow I want, but does feel like
> a big ol' hack :>
I agree it feels hacky, but I don't really see how we can generalize it
more so it will become a standard feature in git?
I was thinking you can already pass `-c remote.origin.fetch=<refspec>`
(multiple times) to git-clone(1), but in practice it doesn't seem to
work because that config is additive, so it adds the refspec, instead of
overwriting, so you're getting:
fatal: multiple updates for ref 'refs/remotes/origin/main' not allowed
And you cannot combine it with `--single-branch`, although you could do
a single branch clone and then add additional refspecs later.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Tuomas Ahola @ 2026-06-03 13:30 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Patrick Steinhardt, Weijie Yuan, git, Junio C Hamano
In-Reply-To: <aiAK9eLvew+mgWt+@szeder.dev>
SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> > On Wed, Jun 03, 2026 at 10:12:22AM +0800, Weijie Yuan wrote:
> > > On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> > > > Huh? Doesn't MyFirstContribution speak *against* shallow threading?
> > > >
> > > > [...] make sure to replace it with the correct Message-ID for your
> > > > **previous cover letter** - that is, if you're sending v2, use the Message-ID
> > > > from v1; if you're sending v3, use the Message-ID from v2.
> > >
> > > I don't get it. Doesn't shallow threading means every following patches
> > > are replying to the cover letter? Replying to the previous one is
> > > --chain-reply-to, if I'm not mistaken.
> >
> > Shallow threading basically means that all patches are sent as a
> > response to the current cover letter, and the current cover letter is
> > always attached to the cover letter of the _first_ version.
>
> No, in Git shallow threading means that all patches are sent as a
> respose to the current cover letter, period. It has nothing to do
> with whether the current cover letter is sent as a reply to the cover
> letter of the first or the previous version.
>
That seems to be the established meaning of shallow threading, e.g. in
`git format-patch --thread=shallow`. Unfortunately there is a slight
terminology clash here.
Indeed, in B4 the config option `b4.send-same-thread = shallow` *is*
about whether the cover letter is a reply to v1 or v(n-1).
> > So this quote is definitely at odds with the configuration I have
> > proposed. It's actually quite surprising to me that we recommend deep
> > threading -- I personally find it extremely hard to navigate as the
> > nesting eventually gets way too deep.
>
> Deep threading means that every mail is a reply to the previous one.
> Again, it has nothing to do with the relation of the current cover
> letter and the previous cover letters.
>
> Therefore, we do not recommend deep threading.
>
In the usual meaning of the word that is the case. Most certainly
we don't recommend that kind of deep threading, but that wasn't the
question we were discussing here.
^ permalink raw reply
* Re: git hook question
From: Adrian Ratiu @ 2026-06-03 13:07 UTC (permalink / raw)
To: Jeff King, Wesley Schwengle; +Cc: git
In-Reply-To: <20260529210049.GC2628906@coredump.intra.peff.net>
On Fri, 29 May 2026, Jeff King <peff@peff.net> wrote:
> [re-adding list cc; let's let everyone benefit from the discussion]
>
> On Fri, May 29, 2026 at 04:14:33PM -0400, Wesley Schwengle wrote:
>
>> > I don't think the hooks themselves should need to be aware. If somebody
>> > is calling "git hook run pre-push" without providing arguments, they are
>> > breaking the contract to the hooks. You can get away with it if you know
>> > your particular hooks do not care about those arguments, but in the
>> > general case, what should a pre-push hook that _does_ care about the
>> > remote name do when it doesn't get any arguments? It's an error.
>>
>> Are they? The manual says this:
>>
>> git hook run has been designed to make it easy for tools which wrap Git to
>> configure and execute hooks using the Git hook infrastructure. It is
>> possible to provide arguments and stdin via the command line, as well as
>> specifying parallel or series execution if the user has provided multiple
>> hooks.
>>
>> Assuming your wrapper wants to support a hook named
>> "mywrapper-start-tests", you can have your users specify their hooks like
>> so:
>>
>> [hook "setup-test-dashboard"]
>> event = mywrapper-start-tests
>> command = ~/mywrapper/setup-dashboard.py --tap
>>
>> Then, in your mywrapper tool, you can invoke any users' configured
>> hooks by running:
>>
>> git hook run --allow-unknown-hook-name mywrapper-start-tests \
>> # providing something to stdin
>> --stdin some-tempfile-123 \
>> # execute multiple hooks in parallel
>> --jobs 3 \
>> # plus some arguments of your own...
>> -- \
>> --testname bar \
>> baz
>>
>> There is nothing about the contract of the hook, in fact, the way it is
>> written there isn't really a contract.
>
> This is a made-up hook, so it is up to the person defining
> mywrapper-start-tests to define that contract. And in this example,
> implicitly it takes whatever is in some-tempfile-123 on stdin, and
> --testname as an argument. What those mean would need to be communicated
> between the script invoking "git hook" and whoever is configuring hooks.
>
> I agree that is not made very clear in the documentation, though.
>
>> > So whether you are getting input as arguments or over stdin, it's
>> > probably something the hook needs to deal with (or at least think
>> > about).
>>
>> Right. I see where this is going. That means I think the examples in the
>> manual are incorrect, no, that's harsh, it could be stated more clearly in
>> git-hook(1).
>>
>> Examples like this:
>>
>> > [hook "linter"]
>> > event = pre-commit
>> > command = ~/bin/linter --cpp20
>>
>> seem to indicate: Any script can be run as a hook, the fact it needs to
>> respect the native hook structure isn't mentioned. This is mentioned:
>
> That example is OK-ish, in the sense that pre-commit does not take any
> arguments or receive anything on stdin. So you really can invoke
> whatever program you like (though it needs to understand how to use Git
> commands to look at what is staged in the index). So the details of
> "~/bin/linter" are doing a lot of the heavy lifting here, which is left
> unsaid.
>
> But the later example that adds "event = pre-push" is actively
> misleading. How does the ~/bin/linter script even know in which context
> it's being run? In the real world you are more likely to invoke a script
> that is aware it is a Git hook and can react accordingly.
>
> So I suspect there is a lot of room for expanding the documentation and
> explaining some of these gotchas. +cc Adrian, who wrote these docs, for
> visibility.
Yes, there is a lot of room for improvements everywhere, especially in
the documentation.
Patches are very much welcome to expand on or correct hook-related
issues. :)
BTW the git hook command is also just a very basic tool for testing, it
needs much attention and more additions. It is obviously not
feature-complete or bug-free.
Some historical context for the curious:
This area of work was blocked for almost a decade because people tried to
find a perfect/complete solution in one go, with complex patch series
reaching even 36-38 review iterations for a single series which went
nowhere, was regressing, was hard to review, you get the idea.
So I tried to enable a simplified incremental development approach,
reusing existing APIs & mechanisms, to allow more people to contribute
smaller patches which are also easier to review, test and so on.
P.S:
This also reminds me, I don't think it's documented anywhere that the
proc-receive hook is not using hook.[ch], so it cannot be specified via
configs yet like pre-receive and other similar server hooks.
I actually have a collegue at Collabora working on converting
proc-receive so we can remove some deprecated APIs and also clean up
some external hook_exists() calls which are now redundant because they
are handled by the unified hook.c implementation.
^ permalink raw reply
* Re: [PATCH v2 01/18] odb/source-loose: move loose source into "odb/" subsystem
From: Karthik Nayak @ 2026-06-03 13:07 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano
In-Reply-To: <20260601-b4-pks-odb-source-loose-v2-1-90ff159430af@pks.im>
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> In subsequent patches we'll be turning `struct odb_source_loose` into a
> proper `struct odb_source`. As a first step towards this goal, move its
s/its/this?
> struct out of "object-file.c" and into "odb/source-loose.c".
>
> This detaches the implementation of the loose object source from the
> generic object file code, following the same convention already used by
> the "files" and "in-memory" sources.
>
> No functional changes are intended.
>
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/8] setup: centralize object database creation
From: Karthik Nayak @ 2026-06-03 13:04 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-0-2fa5b385c13e@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this small patch series refactors the logic for how we discover and
> configure repositories. Most importantly, this involves the following
> two steps:
>
> 1. We unify the logic to apply the repository format, which is
> currently open-coded across multiple sites. These sites have
> already diverged, where some repository extensions are not
> consistently applied.
>
> 2. We then centralize creation of the object database to happen at the
> same time we apply the repository format.
>
> The end result is that we apply the repository format exactly once, and
> that's also the point in time where we can finalize the setup of the
> repo's data structures as we know about all details of the repo at that
> time. Ultimately, this makes it trivial to introduce the "objectStorage"
> extension, even though that's not part of this patch series.
>
> The series is built on top of aec3f58750 (Sync with 'maint', 2026-05-21)
> with ps/setup-wo-the-repository at df69f40c34 (setup: stop using
> `the_repository` in `init_db()`, 2026-05-19) merged into it.
>
Apart from some questions/comments, the series looks good. Thanks
- Karthik
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH] git-gui: silence install recipes under "make -s"
From: Johannes Sixt @ 2026-06-03 12:58 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2318.git.git.1780477489662.gitgitgadget@gmail.com>
Am 03.06.26 um 11:04 schrieb Harald Nordgren via GitGitGadget:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> The split install/uninstall recipes embed "echo" calls that fire
> even under "make -s", so install still prints "DEST /path" and
> "INSTALL 644 about.tcl" banners. The existing "-s" block only
> clears QUIET_GEN.
Good catch.
> Wrap the whole "ifndef V" block in the canonical "-s" guard from
> shared.mak, and drop the now-redundant narrow block.
Can we please mention shared.mak in a way that doesn't assume that this
patch was made in the Git repository?
> +ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
> ifndef V
> QUIET = @
> QUIET_GEN = $(QUIET)echo ' ' GEN '$@' &&
> @@ -89,6 +90,7 @@ ifndef V
> REMOVE_F0 = dst=
> REMOVE_F1 = && echo ' ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
> endif
> +endif
> -ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)
I would have expected that the old and the new condition expressions
only differ in the ifeq vs. ifneq, but they are different in more than
that. Assuming that the new expression is correct, was the old one
incorrect?
> -QUIET_GEN =
> -endif
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 5/8] setup: stop creating the object database in `setup_git_env()`
From: Karthik Nayak @ 2026-06-03 12:57 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-5-2fa5b385c13e@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5708 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> In the preceding commit we have stopped creating the object database in
> `repo_set_gitdir()`. But the logic is still somewhat confusing as we
> still end up creating it conditionally in `setup_git_dir()`, which is
> called multiple times.
>
> Drop the conditional logic and instead create the object database in all
> places where we have discovered and configured a repository.
>
> This leads to even more duplication than we already had in the preceding
> commit, but an alert reader may notice that we now (almost) always call
> `odb_new()` directly before having called `apply_repository_format()`.
> The only exception to this is `setup_git_directory_gently()`, where we
> also call the function when _not_ applying the repository format. This
> will be fixed in the next commit, and once that's done we can then unify
> creation of the object database into `apply_repository_format()`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> setup.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 3bd3f6c592..0dc9fe4565 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1035,8 +1035,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
> }
>
> static void setup_git_env_internal(struct repository *repo,
> - const char *git_dir,
> - bool skip_initializing_odb)
> + const char *git_dir)
> {
> char *git_replace_ref_base;
> const char *shallow_file;
> @@ -1053,10 +1052,6 @@ static void setup_git_env_internal(struct repository *repo,
> repo_set_gitdir(repo, git_dir, &args);
> strvec_clear(&to_free);
>
> - if (!skip_initializing_odb)
> - repo->objects = odb_new(repo, getenv_safe(&to_free, DB_ENVIRONMENT),
> - getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> -
> if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
> disable_replace_refs();
> replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
> @@ -1072,10 +1067,10 @@ static void setup_git_env_internal(struct repository *repo,
> fetch_if_missing = 0;
> }
>
> -static void set_git_dir_1(struct repository *repo, const char *path, bool skip_initializing_odb)
> +static void set_git_dir_1(struct repository *repo, const char *path)
> {
> xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
> - setup_git_env_internal(repo, path, skip_initializing_odb);
> + setup_git_env_internal(repo, path);
> }
>
> static void update_relative_gitdir(const char *name UNUSED,
> @@ -1089,7 +1084,7 @@ static void update_relative_gitdir(const char *name UNUSED,
> trace_printf_key(&trace_setup_key,
> "setup: move $GIT_DIR to '%s'",
> path);
> - set_git_dir_1(repo, path, true);
> + set_git_dir_1(repo, path);
Since we were providing `true`, we didn't have to initialize the odb
here. Makes sense.
> free(path);
> }
>
> @@ -1102,7 +1097,7 @@ static void set_git_dir(struct repository *repo, const char *path, int make_real
> path = realpath.buf;
> }
>
> - set_git_dir_1(repo, path, false);
> + set_git_dir_1(repo, path);
>
Huh. I was expecting the odb to be setup here.
> if (!is_absolute_path(path))
> chdir_notify_register(NULL, update_relative_gitdir, repo);
>
> @@ -1879,8 +1874,15 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
> }
>
> if (is_git_directory(".")) {
> + struct strvec to_free = STRVEC_INIT;
> +
> set_git_dir(repo, ".", 0);
> + repo->objects = odb_new(repo,
> + getenv_safe(&to_free, DB_ENVIRONMENT),
> + getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> check_and_apply_repository_format(repo, NULL);
> +
> + strvec_clear(&to_free);
> return path;
> }
>
> @@ -2032,13 +2034,19 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
> startup_info->have_repository ||
> /* GIT_DIR_EXPLICIT */
> getenv(GIT_DIR_ENVIRONMENT)) {
> + struct strvec to_free = STRVEC_INIT;
> +
> if (!repo->gitdir) {
> const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
> if (!gitdir)
> gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
> - setup_git_env_internal(repo, gitdir, false);
> + setup_git_env_internal(repo, gitdir);
> }
>
> + repo->objects = odb_new(repo,
> + getenv_safe(&to_free, DB_ENVIRONMENT),
> + getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> +
>
Okay, now it makes sense. we move the ODB creations to layers above.
> if (startup_info->have_repository) {
> struct strbuf err = STRBUF_INIT;
>
> @@ -2048,6 +2056,8 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
> clear_repository_format(&repo_fmt);
> strbuf_release(&err);
> }
> +
> + strvec_clear(&to_free);
> }
> /*
> * Since precompose_string_if_needed() needs to look at
> @@ -2796,6 +2806,7 @@ int init_db(struct repository *repo,
> int exist_ok = flags & INIT_DB_EXIST_OK;
> char *original_git_dir = real_pathdup(git_dir, 1);
> struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
> + struct strvec to_free = STRVEC_INIT;
>
> if (real_git_dir) {
> struct stat st;
> @@ -2816,6 +2827,9 @@ int init_db(struct repository *repo,
> }
> startup_info->have_repository = 1;
>
> + repo->objects = odb_new(repo, getenv_safe(&to_free, DB_ENVIRONMENT),
> + getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> +
> /*
> * Check to see if the repository version is right.
> * Note that a newly created repository does not have
> @@ -2879,6 +2893,7 @@ int init_db(struct repository *repo,
> }
>
> clear_repository_format(&repo_fmt);
> + strvec_clear(&to_free);
> free(original_git_dir);
> return 0;
> }
>
> --
> 2.54.0.926.g75ba10bac6.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2 4/8] repository: stop initializing the object database in `repo_set_gitdir()`
From: Karthik Nayak @ 2026-06-03 12:49 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-4-2fa5b385c13e@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2655 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
[snip]
> diff --git a/repository.c b/repository.c
> index 58a13f7c4f..2c2395105f 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -181,12 +181,6 @@ void repo_set_gitdir(struct repository *repo,
> free(old_gitdir);
>
> repo_set_commondir(repo, o->commondir);
> -
> - if (!repo->objects)
> - repo->objects = odb_new(repo, o->object_dir, o->alternate_db);
> - else if (!o->skip_initializing_odb)
> - BUG("cannot reinitialize an already-initialized object directory");
> -
This always confuses me, so we were creating the odb even if
`o->skip_initializing_odb` was set to true, if `repo->objects` didn't
exist. Weird.
> repo->disable_ref_updates = o->disable_ref_updates;
>
> expand_base_dir(&repo->graft_file, o->graft_file,
> @@ -302,6 +296,8 @@ int repo_init(struct repository *repo,
> goto error;
> }
>
> + repo->objects = odb_new(repo, NULL, NULL);
> +
> if (worktree)
> repo_set_worktree(repo, worktree);
>
> diff --git a/repository.h b/repository.h
> index c3ec0f4b79..36e2db2633 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -221,12 +221,9 @@ const char *repo_get_work_tree(struct repository *repo);
> */
> struct set_gitdir_args {
> const char *commondir;
> - const char *object_dir;
> const char *graft_file;
> const char *index_file;
> - const char *alternate_db;
> bool disable_ref_updates;
> - bool skip_initializing_odb;
> };
>
> void repo_set_gitdir(struct repository *repo, const char *root,
> diff --git a/setup.c b/setup.c
> index c5015923f1..3bd3f6c592 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1045,17 +1045,18 @@ static void setup_git_env_internal(struct repository *repo,
> struct strvec to_free = STRVEC_INIT;
>
> args.commondir = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT);
> - args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
> args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
> args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
> - args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
> if (getenv(GIT_QUARANTINE_ENVIRONMENT))
> args.disable_ref_updates = true;
> - args.skip_initializing_odb = skip_initializing_odb;
>
> repo_set_gitdir(repo, git_dir, &args);
> strvec_clear(&to_free);
>
> + if (!skip_initializing_odb)
> + repo->objects = odb_new(repo, getenv_safe(&to_free, DB_ENVIRONMENT),
> + getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> +
Now this makes a lot more sense.
> if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
> disable_replace_refs();
> replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
>
> --
> 2.54.0.926.g75ba10bac6.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/8] setup: deduplicate logic to apply repository format
From: Karthik Nayak @ 2026-06-03 12:43 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-3-2fa5b385c13e@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4439 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> After having discovered the repository format we then apply it to the
> repository so that it knows to use the proper repository extensions. The
> logic to apply the format is duplicated across three callsites, which
> makes it rather painfull to add new extensions.
>
> Introduce a new function `apply_repository_format()` that takes a repo
> and applies a given format to it and adapt all callsites to use it.
> While at it, rename `check_repository_format()` to clarify that it
> doesn't only _check_ the format, but that it also applies it.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> repository.c | 31 +++++++-------------
> setup.c | 93 ++++++++++++++++++++++++++++++++----------------------------
> setup.h | 9 ++++++
> 3 files changed, 70 insertions(+), 63 deletions(-)
>
> diff --git a/repository.c b/repository.c
> index db57b8308b..58a13f7c4f 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -262,8 +262,8 @@ void repo_set_worktree(struct repository *repo, const char *path)
> trace2_def_repo(repo);
> }
>
> -static int read_and_verify_repository_format(struct repository_format *format,
> - const char *commondir)
> +static int read_repository_format_from_commondir(struct repository_format *format,
> + const char *commondir)
Nit: The commit explicitly calls out one rename, but this one wasn't.
> {
> int ret = 0;
> struct strbuf sb = STRBUF_INIT;
> @@ -272,11 +272,6 @@ static int read_and_verify_repository_format(struct repository_format *format,
> read_repository_format(format, sb.buf);
> strbuf_reset(&sb);
>
> - if (verify_repository_format(format, &sb) < 0) {
> - warning("%s", sb.buf);
> - ret = -1;
> - }
> -
So we remove this, so that the callee would independently verify the
format I assume.
Edit: seems like we call verify_repository_format() within
apply_repository_format() and the latter is called by the callee.
> strbuf_release(&sb);
> return ret;
> }
> @@ -290,6 +285,8 @@ int repo_init(struct repository *repo,
> const char *worktree)
> {
> struct repository_format format = REPOSITORY_FORMAT_INIT;
> + struct strbuf err = STRBUF_INIT;
> +
> memset(repo, 0, sizeof(*repo));
>
> initialize_repository(repo);
> @@ -297,21 +294,13 @@ int repo_init(struct repository *repo,
> if (repo_init_gitdir(repo, gitdir))
> goto error;
>
> - if (read_and_verify_repository_format(&format, repo->commondir))
> + if (read_repository_format_from_commondir(&format, repo->commondir))
> goto error;
>
> - repo_set_hash_algo(repo, format.hash_algo);
> - repo_set_compat_hash_algo(repo, format.compat_hash_algo);
> - repo_set_ref_storage_format(repo, format.ref_storage_format,
> - format.ref_storage_payload);
> - repo->repository_format_worktree_config = format.worktree_config;
> - repo->repository_format_relative_worktrees = format.relative_worktrees;
> - repo->repository_format_precious_objects = format.precious_objects;
> - repo->repository_format_submodule_path_cfg = format.submodule_path_cfg;
> -
> - /* take ownership of format.partial_clone */
I see that we now do an xstrdup for format.partial_clone, meaning we
have our own memory segment to care about. Do we have to worry about
format.partial_clone not being free'd?
> - repo->repository_format_partial_clone = format.partial_clone;
> - format.partial_clone = NULL;
> + if (apply_repository_format(repo, &format, &err) < 0) {
> + warning("%s", err.buf);
> + goto error;
> + }
>
> if (worktree)
> repo_set_worktree(repo, worktree);
[snip]
> diff --git a/setup.h b/setup.h
> index 9409326fe4..5ed92f53fa 100644
> --- a/setup.h
> +++ b/setup.h
> @@ -221,6 +221,15 @@ void clear_repository_format(struct repository_format *format);
> int verify_repository_format(const struct repository_format *format,
> struct strbuf *err);
>
> +/*
> + * Apply the given repository format to the repo. This initializes extensions
> + * and basic data structures required for normal operation. Returns 0 on
> + * success, a negative error code otherwise.
> + */
Nit: perhaps we should also mention that we verify the format?
> +int apply_repository_format(struct repository *repo,
> + const struct repository_format *format,
> + struct strbuf *err);
> +
> const char *get_template_dir(const char *option_template);
>
> #define INIT_DB_QUIET (1 << 0)
>
> --
> 2.54.0.926.g75ba10bac6.dirty
p
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: Git for Windows Failing to Clone
From: Johannes Schindelin @ 2026-06-03 12:42 UTC (permalink / raw)
To: Dylan Carlyle; +Cc: git
In-Reply-To: <CAJKusd6WJUUVhbyN_-XHkGWVYeNe_=K2U3tZoezPWFG3+OG_zw@mail.gmail.com>
Hi Dylan,
On Wed, 3 Jun 2026, Dylan Carlyle wrote:
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue):
>
> Ran git clone user@ip_addres:repo
>
> What did you expect to happen? (Expected behavior):
>
> The repo to be cloned
>
> What happened instead? (Actual behavior):
>
> remote: Enumerating objects: 57873, done.
> remote: Counting objects: 100% (57873/57873), done.
> remote: Compressing objects: 100% (32002/32002), done.
> fatal: pack has bad object at offset 460179591: inflate returned 1
> fatal: fetch-pack: invalid index-pack output
>
> What's different between what you expected and what actually happened?
>
> The clone never finishes on Windows.
>
> Anything else you want to add:
>
> Git version on the remote server is 2.47.3
> This works fine from Linux but fails on Windows.
I guess that this is virtually identical to
https://github.com/git-for-windows/git/issues/6265
Since it works from Linux, but not from Windows, I strongly suspect the
problem to be related to that vexing 2GB/4GB problem induced by Git's
continued use of `unsigned long` instead of `size_t`, which I am slowly
(_very_ slowly) trying to address.
You can find out whether that effort might help you, by running `git repo
structure` on the original repository (I'd expect a blob whose unpacked
size is larger than 4GB).
Ciao,
Johannes
>
> [System Info]
> git version:
> git version 2.54.0.windows.1
> cpu: x86_64
> built from commit: 2b8a3ab140826ac423c2845ef81d4c6ac4f7bf3c
> sizeof-long: 4
> sizeof-size_t: 8
> shell-path: D:/git-sdk-64-build-installers/usr/bin/sh
> rust: disabled
> feature: fsmonitor--daemon
>
> --
> Kind Regards,
>
> Dylan Carlyle
> REFTEK Systems, Inc.
> Systems Administrator
>
>
^ permalink raw reply
* Re: [PATCH v3] index-pack: retain child bases in delta cache
From: Derrick Stolee @ 2026-06-03 12:24 UTC (permalink / raw)
To: Arijit Banerjee via GitGitGadget, git
Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Jeff King,
Arijit Banerjee, Arijit Banerjee
In-Reply-To: <pull.2131.v3.git.1780445118653.gitgitgadget@gmail.com>
On 6/2/26 8:05 PM, Arijit Banerjee via GitGitGadget wrote:
> Changes since v2:
>
> * Addressed Jeff King's review question by releasing cached base data
> after all direct children have been dispatched, while keeping the
> existing subtree bookkeeping intact.
> * Re-ran t/t5302-pack-index.sh, p5302-pack-index.sh, and end-to-end
> full clone spot checks with the precise-release version.
...
> +static int base_data_has_remaining_direct_children(struct base_data *c)
> +{
> + return c->ref_first <= c->ref_last ||
> + c->ofs_first <= c->ofs_last;
> +}
> +
I'm glad you were able to find some bookkeeping that already exists to
help with this decision.
> static void prune_base_data(struct base_data *retain)
> {
> struct list_head *pos;
> @@ -1201,8 +1207,12 @@ static void *threaded_second_pass(void *data)
> }
>
> work_lock();
> - if (parent)
> + if (parent) {
> parent->retain_data--;
> + if (!parent->retain_data &&
> + !base_data_has_remaining_direct_children(parent))
> + free_base_data(parent);
> + }
This appears like the correct place to do this.
> if (child && child->data) {
> /*
> @@ -1212,7 +1222,6 @@ static void *threaded_second_pass(void *data)
> list_add(&child->list, &work_head);
> base_cache_used += child->size;
> prune_base_data(NULL);
> - free_base_data(child);
And still we don't want this universal free.
Thanks for re-running your performance numbers after this change. I didn't
see any significant difference in the relative changes.
I don't think we have a way of measuring "memory pressure" during the
performance test suite. Did you see any evidence that this change has the
intended effect of reducing process memory proactively instead of relying
on the cache evictions?
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Weijie Yuan @ 2026-06-03 12:23 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Patrick Steinhardt, Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <aiAK9eLvew+mgWt+@szeder.dev>
On Wed, Jun 03, 2026 at 01:07:33PM +0200, SZEDER Gábor wrote:
> No, in Git shallow threading means that all patches are sent as a
> respose to the current cover letter, period. It has nothing to do
> with whether the current cover letter is sent as a reply to the cover
> letter of the first or the previous version.
Thanks, agree
> Deep threading means that every mail is a reply to the previous one.
> Again, it has nothing to do with the relation of the current cover
> letter and the previous cover letters.
>
> Therefore, we do not recommend deep threading.
So the same reason with Patrick?
Thanks
^ permalink raw reply
* Re: [PATCH v2 1/8] t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
From: Karthik Nayak @ 2026-06-03 12:22 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-1-2fa5b385c13e@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> In subsequent commits we'll rework how we set up the repository. This is
> a somewhat intricate and thus fragile sequence; there's many things that
> can go subtly wrong, and there are lots of interesting interactions that
> one can discover.
>
> One such discovered edge case was the interaction between git-init(1)
> and the "GIT_OBJECT_DIRECTORY" environment variable. When set, the
> behaviour is that the object directory should be created at the path
> that the variable points to. This behaviour is documented as such in
> its man page:
>
> If the object storage directory is specified via the
> GIT_OBJECT_DIRECTORY environment variable then the sha1 directories
> are created underneath; otherwise, the default $GIT_DIR/objects
> directory is used.
>
> Curiously enough though we don't seem to have any tests that exercise
> this directly, and thus a subsequent commit inadvertently would have
> broken this expectation.
>
> Plug this test gap.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> t/t0001-init.sh | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index e4d32bb4d2..e89feca544 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -980,4 +980,14 @@ test_expect_success 're-init reads matching includeIf.onbranch' '
> test_cmp expect err
> '
>
> +test_expect_success 'init honors GIT_OBJECT_DIRECTORY' '
> + test_when_finished "rm -rf init-objdir custom-odb" &&
> + mkdir custom-odb &&
> + env GIT_OBJECT_DIRECTORY="$(pwd)/custom-odb" \
> + git init init-objdir &&
> + test_path_is_missing init-objdir/.git/objects/pack &&
> + test_path_is_dir custom-odb/pack &&
> + test_path_is_dir custom-odb/info
> +'
> +
I was surprised to find that such a small number of tests ever use
GIT_OBJECT_DIRECTORY. This looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH 2/2] builtin/add: use die_for_required_opt() helper
From: Siddharth Shrimali @ 2026-06-03 11:10 UTC (permalink / raw)
To: git; +Cc: gitster, christian.couder, toon, jn.avila, r.siddharth.shrimali
In-Reply-To: <20260603111044.39116-1-r.siddharth.shrimali@gmail.com>
Clean up manual option dependency checks by replacing explicit conditional
blocks with the newly introduced die_for_required_opt() helper function.
Specifically, simplify the prerequisite check logic for both
'--ignore-missing' (which requires '--dry-run') and '--pathspec-file-nul'
(which requires '--pathspec-from-file').
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
builtin/add.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..a5c91c6dcf 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -441,8 +441,7 @@ int cmd_add(int argc,
if (addremove && take_worktree_changes)
die(_("options '%s' and '%s' cannot be used together"), "-A", "-u");
- if (!show_only && ignore_missing)
- die(_("the option '%s' requires '%s'"), "--ignore-missing", "--dry-run");
+ die_for_required_opt(ignore_missing, "--ignore-missing", show_only, "--dry-run");
if (chmod_arg && ((chmod_arg[0] != '-' && chmod_arg[0] != '+') ||
chmod_arg[1] != 'x' || chmod_arg[2]))
@@ -462,6 +461,8 @@ int cmd_add(int argc,
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, argv);
+ die_for_required_opt(pathspec_file_nul, "--pathspec-file-nul",
+ !!pathspec_from_file, "--pathspec-from-file");
if (pathspec_from_file) {
if (pathspec.nr)
die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
@@ -470,8 +471,6 @@ int cmd_add(int argc,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, pathspec_from_file, pathspec_file_nul);
- } else if (pathspec_file_nul) {
- die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
}
if (require_pathspec && pathspec.nr == 0) {
--
2.54.0
^ permalink raw reply related
* [PATCH 1/2] parse-options: introduce die_for_required_opt()
From: Siddharth Shrimali @ 2026-06-03 11:10 UTC (permalink / raw)
To: git; +Cc: gitster, christian.couder, toon, jn.avila, r.siddharth.shrimali
In-Reply-To: <20260603111044.39116-1-r.siddharth.shrimali@gmail.com>
Introduce a new helper function die_for_required_opt() to check if a
given option is present without its required prerequisite option.
This provides a centralized API for handling simple option dependencies
(i.e., X requires Y), matching the style of the existing mutual-exclusion
helpers like die_for_incompatible_opt{2,3,4}().
Suggested-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
parse-options.c | 7 +++++++
parse-options.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/parse-options.c b/parse-options.c
index a676da86f5..e100f9a0c1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1558,3 +1558,10 @@ void die_for_incompatible_opt4(int opt1, const char *opt1_name,
break;
}
}
+
+void die_for_required_opt(int opt1, const char *opt1_name,
+ int opt2, const char *opt2_name)
+{
+ if (opt1 && !opt2)
+ die(_("the option '%s' requires '%s'"), opt1_name, opt2_name);
+}
diff --git a/parse-options.h b/parse-options.h
index 0d1f738f8d..99dc53325d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -460,6 +460,9 @@ static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
0, "");
}
+void die_for_required_opt(int opt1, const char *opt1_name,
+ int opt2, const char *opt2_name);
+
/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
--
2.54.0
^ permalink raw reply related
* [PATCH 0/2] parse-options: introduce die_for_required_opt() helper
From: Siddharth Shrimali @ 2026-06-03 11:10 UTC (permalink / raw)
To: git; +Cc: gitster, christian.couder, toon, jn.avila, r.siddharth.shrimali
Many built-in commands in Git manually check for option prerequisites
(i.e., option X relies on option Y being present) using explicit
conditional blocks and duplicated error message strings.
This short series comes out of a discussion with Christian about
localization and code duplication. To address these issues, it
introduces a centralized API helper that handles simple option
prerequisites safely.
- Patch 1 introduces the `die_for_required_opt()` helper function
inside parse-options.
- Patch 2 cleans up `builtin/add.c` as a proof-of-concept by migrating
its manual prerequisite checks for '--ignore-missing' and
'--pathspec-file-nul' over to the new helper.
If this initial approach looks good, we can later extend the helper
to handle more complex multi-option dependencies.
Siddharth Shrimali (2):
parse-options: introduce die_for_required_opt()
builtin/add: use die_for_required_opt() helper
builtin/add.c | 7 +++----
parse-options.c | 7 +++++++
parse-options.h | 3 +++
3 files changed, 13 insertions(+), 4 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: SZEDER Gábor @ 2026-06-03 11:07 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Weijie Yuan, Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <ah_PyDwO1Sffr5yq@pks.im>
On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> On Wed, Jun 03, 2026 at 10:12:22AM +0800, Weijie Yuan wrote:
> > On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> > > Huh? Doesn't MyFirstContribution speak *against* shallow threading?
> > >
> > > [...] make sure to replace it with the correct Message-ID for your
> > > **previous cover letter** - that is, if you're sending v2, use the Message-ID
> > > from v1; if you're sending v3, use the Message-ID from v2.
> >
> > I don't get it. Doesn't shallow threading means every following patches
> > are replying to the cover letter? Replying to the previous one is
> > --chain-reply-to, if I'm not mistaken.
>
> Shallow threading basically means that all patches are sent as a
> response to the current cover letter, and the current cover letter is
> always attached to the cover letter of the _first_ version.
No, in Git shallow threading means that all patches are sent as a
respose to the current cover letter, period. It has nothing to do
with whether the current cover letter is sent as a reply to the cover
letter of the first or the previous version.
> So this quote is definitely at odds with the configuration I have
> proposed. It's actually quite surprising to me that we recommend deep
> threading -- I personally find it extremely hard to navigate as the
> nesting eventually gets way too deep.
Deep threading means that every mail is a reply to the previous one.
Again, it has nothing to do with the relation of the current cover
letter and the previous cover letters.
Therefore, we do not recommend deep threading.
> You know -- I'll include a patch that changes the wording there to also
> use shallow nesting, mostly to kick off a discussion and arrive at a
> decision there.
^ permalink raw reply
* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Weijie Yuan @ 2026-06-03 10:29 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Tuomas Ahola, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-1-a8aea0aa2c23@pks.im>
I'm afraid there will be some chaos.
As mentioned earlier, GitGitGadget now supports deep nesting of
iterations, if b4 changes while GitGitGadget doesn't, it would be
inconsistent in the archive. So, negotiation is necessary here.
As I know, b4 can generate a cover letter containing "Changes with vn",
e.g. in [PATCH v5 00/10], there would be Changes with v4, v3, v2, v1. In
this case, it is semantically correct that the cover letter of v5 is
replying-to the cover letter of v1.
But in traditional way, it seems that the norm is put a range-diff in
the cover letter. In this case, chain-reply-to makes more sence to me:
e.g. The cover letter contains the range-diff against v2, so cover
letter v3 is pointing to cover letter v2. (I don't know whether
git format-patch accepts several --range-diff or not, but if so, I
guess it might be painful to typing several refs, or copy and paste
from previous cover letter) Therefore, if git format-patch could
generate cover letter containing all the changes with v4/v3/v2/v1 as b4
does, it would be consistent, and semantically correct to pointing to
the first cover letter.
Do we need to consider backward compatibility here? ;-)
Thanks!
^ permalink raw reply
* [PATCH v2] rebase: skip branch symref aliases
From: Son Luong Ngoc via GitGitGadget @ 2026-06-03 10:27 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Phillip Wood, Son Luong Ngoc,
Son Luong Ngoc
In-Reply-To: <pull.2126.git.1779946921.gitgitgadget@gmail.com>
From: Son Luong Ngoc <sluongng@gmail.com>
git rebase --update-refs can fail after the normal rebase path has
updated the current branch when another local branch is a symref to it.
This can happen during a default-branch rename where refs/heads/main
points at refs/heads/master while users migrate.
The sequencer queues update-ref commands from local branch decorations.
Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
decorations that are not local branches, such as HEAD and tags. A branch
symref is different: it is still a local branch decoration, but if it
resolves to another branch then that target branch is itself present in
the decoration list and will be updated as a concrete branch.
Skip branch decorations whose symrefs resolve to refs/heads/*, because
those targets are already represented by concrete branch decorations.
This prevents aliases from scheduling a second update for the same
branch. Keep symrefs to non-branch targets on the existing path.
Preserve the existing checked-out branch handling before applying these
skips. Such refs still need a todo-list comment instead of an update-ref
command, even when the checked-out ref is the branch being rebased or a
branch symref alias. Use a copy of the resolved HEAD ref so later ref
resolution does not overwrite it.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
rebase: handle --update-refs branch symrefs
Changes since v1:
* Squashed the regression test and fix into a single patch.
* Reworked the implementation per Phillip's review: skip local branch
symrefs that are not checked out when their targets resolve to
refs/heads/*, while preserving the existing behavior for symrefs to
non-branch targets.
* Preserved checked-out branch comments before applying the new symref
skip, so worktrees still get the existing todo-list warning instead
of an update-ref command.
* Folded the symref coverage into the existing "--update-refs updates
refs correctly" test, covering both an alias of HEAD and an alias of
another branch.
* Kept the relationship to 106b6885c7 in the commit message: non-local
decorations are already filtered, and this patch handles the
remaining local branch decorations that are symref aliases.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2126
Range-diff vs v1:
1: a550923440 < -: ---------- t3404: add failing branch symref test
2: 0ab0a71744 ! 1: 68f698225c rebase: skip branch symref aliases
@@ Metadata
## Commit message ##
rebase: skip branch symref aliases
- rebase --update-refs records local branch decorations before replaying
- commits. If a decoration is a symbolic branch such as refs/heads/main
- pointing at refs/heads/master, updating it later dereferences back to
- master and can fail because the normal rebase path already moved that
- branch.
+ git rebase --update-refs can fail after the normal rebase path has
+ updated the current branch when another local branch is a symref to it.
+ This can happen during a default-branch rename where refs/heads/main
+ points at refs/heads/master while users migrate.
- Resolve local branch symref decorations to their referents before
- queuing update-ref commands, and skip duplicates. This keeps branch
- aliases from scheduling a second update for the same underlying branch
- while still using the existing old-OID check for the single queued
- update.
+ The sequencer queues update-ref commands from local branch decorations.
+ Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
+ decorations that are not local branches, such as HEAD and tags. A branch
+ symref is different: it is still a local branch decoration, but if it
+ resolves to another branch then that target branch is itself present in
+ the decoration list and will be updated as a concrete branch.
+
+ Skip branch decorations whose symrefs resolve to refs/heads/*, because
+ those targets are already represented by concrete branch decorations.
+ This prevents aliases from scheduling a second update for the same
+ branch. Keep symrefs to non-branch targets on the existing path.
+
+ Preserve the existing checked-out branch handling before applying these
+ skips. Such refs still need a todo-list comment instead of an update-ref
+ command, even when the checked-out ref is the branch being rebased or a
+ branch symref alias. Use a copy of the resolved HEAD ref so later ref
+ resolution does not overwrite it.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
const struct name_decoration *decoration = get_name_decoration(&commit->object);
- const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
- "HEAD",
-+ struct ref_store *refs = get_main_ref_store(the_repository);
-+ const char *head_ref = refs_resolve_ref_unsafe(refs, "HEAD",
- RESOLVE_REF_READING,
+- RESOLVE_REF_READING,
- NULL,
- NULL);
-+ NULL, NULL);
-+ char *resolved_head_ref = refs_resolve_refdup(refs, "HEAD",
-+ RESOLVE_REF_READING,
-+ NULL, NULL);
-+ struct strbuf update_ref = STRBUF_INIT;
++ struct ref_store *refs = get_main_ref_store(the_repository);
++ char *head_ref = refs_resolve_refdup(refs, "HEAD",
++ RESOLVE_REF_READING,
++ NULL, NULL);
while (decoration) {
struct todo_item *item;
const char *path;
-+ const char *ref = decoration->name;
+ const char *resolved_ref;
-+ int is_symref = 0;
+ int flags = 0;
size_t base_offset = ctx->buf->len;
/*
-@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
- * updated by the default rebase behavior.
- * Exclude it from the list of refs to update,
- * as well as any non-branch decorations.
-+ *
-+ * Resolve branch symrefs after checking for the current HEAD so
-+ * that aliases do not schedule duplicate updates for their
-+ * referents.
-+ *
+- * If the branch is the current HEAD, then it will be
+- * updated by the default rebase behavior.
+- * Exclude it from the list of refs to update,
+- * as well as any non-branch decorations.
* Non-branch decorations may be present if the pretty format
* includes "%d", which would have loaded all refs
* into the global decoration table.
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
+ continue;
+ }
+
-+ if (head_ref && !strcmp(head_ref, ref)) {
++ path = branch_checked_out(decoration->name);
++
++ /*
++ * If the branch is the current HEAD, then it will be
++ * updated by the default rebase behavior. Exclude it from
++ * the list of refs to update, unless it is checked out and
++ * needs a comment in the todo list.
++ */
++ if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
+ decoration = decoration->next;
+ continue;
+ }
+
-+ strbuf_reset(&update_ref);
-+ resolved_ref = refs_resolve_ref_unsafe(refs, ref,
-+ RESOLVE_REF_READING |
-+ RESOLVE_REF_NO_RECURSE,
++ resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
++ RESOLVE_REF_READING,
+ NULL, &flags);
-+ if ((flags & REF_ISSYMREF) && resolved_ref) {
-+ if (!starts_with(resolved_ref, "refs/heads/")) {
-+ decoration = decoration->next;
-+ continue;
-+ }
-+
-+ strbuf_addstr(&update_ref, resolved_ref);
-+ ref = update_ref.buf;
-+ is_symref = 1;
-+ }
-+
-+ if ((is_symref && resolved_head_ref &&
-+ !strcmp(resolved_head_ref, ref)) ||
-+ string_list_has_string(&ctx->refs_to_oids, ref)) {
++ if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
++ starts_with(resolved_ref, "refs/heads/")) {
decoration = decoration->next;
continue;
}
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
/* If the branch is checked out, then leave a comment instead. */
- if ((path = branch_checked_out(decoration->name))) {
-+ if ((path = branch_checked_out(ref))) {
++ if (path) {
item->command = TODO_COMMENT;
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
-- decoration->name, path);
-+ ref, path);
- } else {
- struct string_list_item *sti;
- item->command = TODO_UPDATE_REF;
-- strbuf_addf(ctx->buf, "%s\n", decoration->name);
-+ strbuf_addf(ctx->buf, "%s\n", ref);
-
- sti = string_list_insert(&ctx->refs_to_oids,
-- decoration->name);
-- sti->util = init_update_ref_record(decoration->name);
-+ ref);
-+ sti->util = init_update_ref_record(ref);
- }
-
- item->offset_in_buf = base_offset;
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
decoration = decoration->next;
}
-+ strbuf_release(&update_ref);
-+ free(resolved_head_ref);
++ free(head_ref);
return 0;
}
## t/t3404-rebase-interactive.sh ##
@@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs ignores non-branch decorations' '
- test_cmp expect actual
'
--test_expect_failure '--update-refs skips branch symrefs to current branch' '
-+test_expect_success '--update-refs skips branch symrefs to current branch' '
- test_when_finished "
- test_might_fail git rebase --abort &&
- git checkout primary &&
+ test_expect_success '--update-refs updates refs correctly' '
++ test_when_finished "
++ test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
++ test_might_fail git symbolic-ref -d refs/heads/second-alias
++ " &&
+ git checkout -B update-refs no-conflict-branch &&
+ git branch -f base HEAD~4 &&
+ git branch -f first HEAD~3 &&
+ git branch -f second HEAD~3 &&
+ git branch -f third HEAD~1 &&
++ git symbolic-ref refs/heads/no-conflict-branch-alias \
++ refs/heads/no-conflict-branch &&
++ git symbolic-ref refs/heads/second-alias refs/heads/second &&
+ test_commit extra2 fileX &&
+ git commit --amend --fixup=L &&
+
+@@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs updates refs correctly' '
+
+ test_cmp_rev HEAD~3 refs/heads/first &&
+ test_cmp_rev HEAD~3 refs/heads/second &&
++ test_cmp_rev HEAD~3 refs/heads/second-alias &&
+ test_cmp_rev HEAD~1 refs/heads/third &&
+ test_cmp_rev HEAD refs/heads/no-conflict-branch &&
++ test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
++ test_write_lines refs/heads/no-conflict-branch >expect &&
++ git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
++ test_cmp expect actual &&
++ test_write_lines refs/heads/second >expect &&
++ git symbolic-ref refs/heads/second-alias >actual &&
++ test_cmp expect actual &&
+
+ q_to_tab >expect <<-\EOF &&
+ Successfully rebased and updated refs/heads/update-refs.
sequencer.c | 43 +++++++++++++++++++++++++----------
t/t3404-rebase-interactive.sh | 15 ++++++++++++
2 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..6ab8b47108 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,
struct todo_add_branch_context *ctx)
{
const struct name_decoration *decoration = get_name_decoration(&commit->object);
- const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
- "HEAD",
- RESOLVE_REF_READING,
- NULL,
- NULL);
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ char *head_ref = refs_resolve_refdup(refs, "HEAD",
+ RESOLVE_REF_READING,
+ NULL, NULL);
while (decoration) {
struct todo_item *item;
const char *path;
+ const char *resolved_ref;
+ int flags = 0;
size_t base_offset = ctx->buf->len;
/*
- * If the branch is the current HEAD, then it will be
- * updated by the default rebase behavior.
- * Exclude it from the list of refs to update,
- * as well as any non-branch decorations.
* Non-branch decorations may be present if the pretty format
* includes "%d", which would have loaded all refs
* into the global decoration table.
*/
- if ((head_ref && !strcmp(head_ref, decoration->name)) ||
- (decoration->type != DECORATION_REF_LOCAL)) {
+ if (decoration->type != DECORATION_REF_LOCAL) {
+ decoration = decoration->next;
+ continue;
+ }
+
+ path = branch_checked_out(decoration->name);
+
+ /*
+ * If the branch is the current HEAD, then it will be
+ * updated by the default rebase behavior. Exclude it from
+ * the list of refs to update, unless it is checked out and
+ * needs a comment in the todo list.
+ */
+ if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
+ decoration = decoration->next;
+ continue;
+ }
+
+ resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
+ RESOLVE_REF_READING,
+ NULL, &flags);
+ if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
+ starts_with(resolved_ref, "refs/heads/")) {
decoration = decoration->next;
continue;
}
@@ -6478,7 +6496,7 @@ static int add_decorations_to_list(const struct commit *commit,
memset(item, 0, sizeof(*item));
/* If the branch is checked out, then leave a comment instead. */
- if ((path = branch_checked_out(decoration->name))) {
+ if (path) {
item->command = TODO_COMMENT;
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
@@ -6501,6 +6519,7 @@ static int add_decorations_to_list(const struct commit *commit,
decoration = decoration->next;
}
+ free(head_ref);
return 0;
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c27..bc0b6a31f7 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1979,11 +1979,18 @@ test_expect_success '--update-refs ignores non-branch decorations' '
'
test_expect_success '--update-refs updates refs correctly' '
+ test_when_finished "
+ test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
+ test_might_fail git symbolic-ref -d refs/heads/second-alias
+ " &&
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
git branch -f first HEAD~3 &&
git branch -f second HEAD~3 &&
git branch -f third HEAD~1 &&
+ git symbolic-ref refs/heads/no-conflict-branch-alias \
+ refs/heads/no-conflict-branch &&
+ git symbolic-ref refs/heads/second-alias refs/heads/second &&
test_commit extra2 fileX &&
git commit --amend --fixup=L &&
@@ -1991,8 +1998,16 @@ test_expect_success '--update-refs updates refs correctly' '
test_cmp_rev HEAD~3 refs/heads/first &&
test_cmp_rev HEAD~3 refs/heads/second &&
+ test_cmp_rev HEAD~3 refs/heads/second-alias &&
test_cmp_rev HEAD~1 refs/heads/third &&
test_cmp_rev HEAD refs/heads/no-conflict-branch &&
+ test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
+ test_write_lines refs/heads/no-conflict-branch >expect &&
+ git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
+ test_cmp expect actual &&
+ test_write_lines refs/heads/second >expect &&
+ git symbolic-ref refs/heads/second-alias >actual &&
+ test_cmp expect actual &&
q_to_tab >expect <<-\EOF &&
Successfully rebased and updated refs/heads/update-refs.
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 2/2] builtin/history: implement "drop" subcommand
From: Patrick Steinhardt @ 2026-06-03 10:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbjdt25e3.fsf@gitster.g>
On Tue, Jun 02, 2026 at 08:43:48AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > +static int update_worktree(struct repository *repo,
> > + const struct commit *old_head,
> > + const struct commit *new_head,
> > + bool dry_run)
> > +{
> > +...
> > +
> > +out:
> > + clear_unpack_trees_porcelain(&opts);
> > + rollback_lock_file(&lock);
> > + release_index(&index);
> > + free(desc_buf[0]);
> > + free(desc_buf[1]);
> > + return ret;
> > +}
>
> The function looks very familiar---anybody who wants to perform
> "checkout <other-commit>" needs to do exactly the above. It is a
> bit surprising and disappointing that this topic needs to *invent*
> its own helper function and carry it as a file-scope static.
It certainly is. We basically have this whole dance in ~8 different
locations by now, and given the verbosity that is required for the whole
setup it's a good hint that the interface is not exactly great.
One of the functions that we might be able to reuse is `reset_head()`...
goes down the rabbit hole... ugh, this is turning out to be somewhat
painful. I'll send a v2 that does the whole exercise, but I'm not a 100%
convinced it's the right thing to do. There's various assumptions that
we have to break:
- It assumes that the index is always clean.
- We don't have a dry-run mode.
- We need to stop it from updating any refs.
- We need to introduce another field to let the caller decide which
commit we're moving from.
So I'm 7 commits deep now adapting the function to our needs. But maybe
the end result is ultimately worth it...? We'll see.
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] builtin/history: implement "drop" subcommand
From: Patrick Steinhardt @ 2026-06-03 10:06 UTC (permalink / raw)
To: Pablo Sabater; +Cc: git
In-Reply-To: <CAN5EUNQbSN7+SDWcrh3jTD7SXrnD=e-fQ9Qj9778R7cy2q4b1g@mail.gmail.com>
On Tue, Jun 02, 2026 at 09:31:17AM +0200, Pablo Sabater wrote:
> El mar, 2 jun 2026 a las 8:16, Patrick Steinhardt (<ps@pks.im>) escribió:
[snip]
> > + original = lookup_commit_reference_by_name(argv[0]);
> > + if (!original) {
> > + ret = error(_("commit cannot be found: %s"), argv[0]);
> > + goto out;
> > + }
> > +
> > + if (!original->parents) {
> > + ret = error(_("cannot drop root commit %s: "
> > + "it has no parent to replay onto"),
> > + argv[0]);
> > + goto out;
> > + } else if (original->parents->next) {
> > + ret = error(_("cannot drop merge commit"));
>
> Why the if block adds which commit context, but not on the else if block?
True indeed, will adapt.
> > diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
> > new file mode 100755
> > index 0000000000..b320ff09b3
> > --- /dev/null
> > +++ b/t/t3454-history-drop.sh
> > @@ -0,0 +1,513 @@
> > +#!/bin/sh
> > +
> > +test_description='tests for git-history drop subcommand'
> > +
> > +. ./test-lib.sh
> > +. "$TEST_DIRECTORY/lib-log-graph.sh"
> > +
> > +expect_graph () {
> > + cat >expect &&
> > + lib_test_cmp_graph --graph --format=%s "$@"
> > +}
>
> This function appears exactly the same at t6016 and t4215 but named as
> check_graph. I was gonna do a cleanup for a commit series I'm working
> on to bring that function to `lib-log-graph.sh` because all these test
> files share that they import graph functions from `lib-log-graph.c`,
> maybe you could do it?
I'd rather keep this series focussed, but I wouldn't mind a follow up
that deduplicates these call sites.
> Also:
>
> lib_test_cmp_graph () {
> git log --graph "$@" >output &&
> sed 's/ *$//' >output.sanitized <output &&
> test_cmp expect output.sanitized
> }
>
> Already uses `--graph` you can drop it from expect_graph()
True indeed, dropped the "--graph" argument.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Tuomas Ahola @ 2026-06-03 10:01 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-1-a8aea0aa2c23@pks.im>
Patrick Steinhardt <ps@pks.im> wrote:
> The "MyFirstContribution" document recommends the use of deep threading:
> every cover letter of subsequent iterations shall be linked to the cover
> letter of the preceding version. The result of this is that eventually,
> threads with many versions are getting nested so deep that it becomes
> hard to follow.
>
> Adapt the recommendation to instead propose shallow threading: instead
> of linking the cover letter to the previous cover letter, the user is
> supposed to always link it to the first cover letter. This still makes
> it easy to follow the iterations, but has the benefit of nesting to a
> much shallower level.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Documentation/MyFirstContribution.adoc | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index b9fdefce02..069020196c 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -1227,8 +1227,8 @@ Message-ID: <foo.12345.author@example.com>
>
> Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
> below as well; make sure to replace it with the correct Message-ID for your
> -**previous cover letter** - that is, if you're sending v2, use the Message-ID
> -from v1; if you're sending v3, use the Message-ID from v2.
> +**first cover letter** - that is, for any subsequent version that you send,
> +always use the Message-ID from v1.
>
> While you're looking at the email, you should also note who is CC'd, as it's
> common practice in the mailing list to keep all CCs on a thread. You can add
>
> --
> 2.54.0.1064.gd145956f57.dirty
If we adapt this change to the guidance, let's fix also other places of the
document that talk about replying to the previous cover letter.
-----8<-----
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 069020196c..bf64a211bd 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -790,7 +790,7 @@ We can note a few things:
v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of
three patches in the second iteration. Each iteration is sent with a new cover
letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the
- previous iteration (more on that below).
+ first iteration (more on that below).
NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without
_i_/_n_ numbering (in the above thread overview, no single-patch topic appears,
@@ -1214,7 +1214,7 @@ between your last version and now, if it's something significant. You do not
need the exact same body in your second cover letter; focus on explaining to
reviewers the changes you've made that may not be as visible.
-You will also need to go and find the Message-ID of your previous cover letter.
+You will also need to go and find the Message-ID of your original cover letter.
You can either note it when you send the first series, from the output of `git
send-email`, or you can look it up on the
https://lore.kernel.org/git[mailing list]. Find your cover letter in the
^ 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