* [PATCH v3 1/3] doc: config: terminate runaway lists
From: Tuomas Ahola @ 2026-06-11 16:19 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Junio C Hamano, Jeff King,
Jean-Noël Avila, Tuomas Ahola
In-Reply-To: <20260611161946.12166-1-taahol@utu.fi>
There are many places in git-config(1) where paragraphs that should
logically come after a list are instead appended to the last item of
the list. This is a well-documented quirk of AsciiDoc, and can be
mitigated by enclosing the list in an open block:
--
* first item
* last item
--
+
New paragraph after the list.
Fix the issue accordingly.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
Notes (doc-diff):
diff --git a/29bd7ed5127255713c1ac2f43b7c6f257d7b4594/home/taahol/share/man/man1/git-config.1 b/c9131b23fd1c8611fde5664fcfd4e6d5283648ad/home/taahol/share/man/man1/git-config.1
index 326782e637..d915897ca5 100644
--- a/29bd7ed5127255713c1ac2f43b7c6f257d7b4594/home/taahol/share/man/man1/git-config.1
+++ b/c9131b23fd1c8611fde5664fcfd4e6d5283648ad/home/taahol/share/man/man1/git-config.1
@@ -234,10 +234,10 @@ OPTIONS
sanity-check is performed to ensure that the given value is
canonicalize-able as an ANSI color, but it is written as-is.
- If the command is in list mode, then the --type <type> argument
- will apply to each listed config value. If the value does not
- successfully parse in that format, then it will be omitted from
- the list.
+ If the command is in list mode, then the --type <type> argument
+ will apply to each listed config value. If the value does not
+ successfully parse in that format, then it will be omitted from the
+ list.
--bool, --int, --bool-or-int, --path, --expiry-date
Historical options for selecting a type specifier. Prefer instead
@@ -841,9 +841,9 @@ CONFIGURATION FILE
Boolean false literals are no, off, false, 0 and the empty
string.
- When converting a value to its canonical form using the
- --type=bool type specifier, git config will ensure that the
- output is "true" or "false" (spelled in lowercase).
+ When converting a value to its canonical form using the --type=bool
+ type specifier, git config will ensure that the output is "true" or
+ "false" (spelled in lowercase).
integer
The value for many variables that specify various sizes can be
@@ -5869,28 +5869,26 @@ CONFIGURATION FILE
exactly match the value advertised by the server for the
"token" field.
- If any of these conditions is not met for any field name listed
- in promisor.checkFields, the advertised remote "foo" is
- rejected.
+ If any of these conditions is not met for any field name listed in
+ promisor.checkFields, the advertised remote "foo" is rejected.
- For the "partialCloneFilter" field, this allows the client to
- ensure that the server’s filter matches what it expects
- locally, preventing inconsistencies in filtering behavior. For
- the "token" field, this can be used to verify that
- authentication credentials match expected values.
+ For the "partialCloneFilter" field, this allows the client to
+ ensure that the server’s filter matches what it expects locally,
+ preventing inconsistencies in filtering behavior. For the "token"
+ field, this can be used to verify that authentication credentials
+ match expected values.
- Field values are compared case-sensitively.
+ Field values are compared case-sensitively.
- The "name" and "url" fields are always checked according to the
- promisor.acceptFromServer policy, independently of this
- setting.
+ The "name" and "url" fields are always checked according to the
+ promisor.acceptFromServer policy, independently of this setting.
- The field names and values should be passed by the server
- through the "promisor-remote" capability by using the
- promisor.sendFields config variable. The fields are checked
- only if the promisor.acceptFromServer config variable is not
- set to "None". If set to "None", this config variable has no
- effect. See gitprotocol-v2(5).
+ The field names and values should be passed by the server through
+ the "promisor-remote" capability by using the promisor.sendFields
+ config variable. The fields are checked only if the
+ promisor.acceptFromServer config variable is not set to "None". If
+ set to "None", this config variable has no effect. See gitprotocol-
+ v2(5).
promisor.storeFields
A comma or space separated list of additional remote related field
@@ -6630,15 +6628,15 @@ CONFIGURATION FILE
the top-level --git-dir command-line option, or the GIT_DIR
environment variable (see git(1)).
- If you do not use bare repositories in your workflow, then it
- may be beneficial to set safe.bareRepository to explicit in
- your global config. This will protect you from attacks that
- involve cloning a repository that contains a bare repository
- and running a Git command within that directory.
+ If you do not use bare repositories in your workflow, then it may
+ be beneficial to set safe.bareRepository to explicit in your global
+ config. This will protect you from attacks that involve cloning a
+ repository that contains a bare repository and running a Git
+ command within that directory.
- This config setting is only respected in protected
- configuration (see the section called “SCOPES”). This prevents
- untrusted repositories from tampering with this value.
+ This config setting is only respected in protected configuration
+ (see the section called “SCOPES”). This prevents untrusted
+ repositories from tampering with this value.
safe.directory
These config entries specify Git-tracked directories that are
Documentation/config.adoc | 4 +++-
Documentation/config/promisor.adoc | 2 ++
Documentation/config/safe.adoc | 2 ++
Documentation/git-config.adoc | 2 ++
4 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.adoc b/Documentation/config.adoc
index dcea3c0c15..fc48c1c461 100644
--- a/Documentation/config.adoc
+++ b/Documentation/config.adoc
@@ -276,13 +276,15 @@ boolean::
When a variable is said to take a boolean value, many
synonyms are accepted for 'true' and 'false'; these are all
case-insensitive.
-
++
+--
true;; Boolean true literals are `yes`, `on`, `true`,
and `1`. Also, a variable defined without `= <value>`
is taken as true.
false;; Boolean false literals are `no`, `off`, `false`,
`0` and the empty string.
+--
+
When converting a value to its canonical form using the `--type=bool` type
specifier, 'git config' will ensure that the output is "true" or
diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index b0fa43b839..39af63dcb8 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -63,11 +63,13 @@ If one of these field names (e.g., "token") is being checked for an
advertised promisor remote (e.g., "foo"), three conditions must be met
for the check of this specific field to pass:
+
+--
1. The corresponding local configuration (e.g., `remote.foo.token`)
must be set.
2. The server must advertise the "token" field for remote "foo".
3. The value of the locally configured `remote.foo.token` must exactly
match the value advertised by the server for the "token" field.
+--
+
If any of these conditions is not met for any field name listed in
`promisor.checkFields`, the advertised remote "foo" is rejected.
diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
index 2d45c98b12..5ae4476b24 100644
--- a/Documentation/config/safe.adoc
+++ b/Documentation/config/safe.adoc
@@ -2,10 +2,12 @@ safe.bareRepository::
Specifies which bare repositories Git will work with. The currently
supported values are:
+
+--
* `all`: Git works with all bare repositories. This is the default.
* `explicit`: Git only works with bare repositories specified via
the top-level `--git-dir` command-line option, or the `GIT_DIR`
environment variable (see linkgit:git[1]).
+--
+
If you do not use bare repositories in your workflow, then it may be
beneficial to set `safe.bareRepository` to `explicit` in your global
diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index 00545b2054..8439ce97df 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -221,6 +221,7 @@ Use `--no-value` to unset _<pattern>_.
+
Valid `<type>`'s include:
+
+--
- 'bool': canonicalize values `true`, `yes`, `on`, and positive
numbers as "true", and values `false`, `no`, `off` and `0` as
"false".
@@ -239,6 +240,7 @@ Valid `<type>`'s include:
escape sequence. When setting a value, a sanity-check is performed to ensure
that the given value is canonicalize-able as an ANSI color, but it is written
as-is.
+--
+
If the command is in `list` mode, then the `--type <type>` argument will apply
to each listed config value. If the value does not successfully parse in that
--
2.30.2
^ permalink raw reply related
* Re: What's cooking in git.git (Jun 2026, #04)
From: Mirko Faina @ 2026-06-11 16:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Mirko Faina
In-Reply-To: <xmqqmrx1oy9y.fsf@gitster.g>
On Thu, Jun 11, 2026 at 09:08:25AM -0700, Junio C Hamano wrote:
> * mf/revision-max-count-oldest (2026-05-18) 1 commit
> (merged to 'next' on 2026-06-09 at 076600fa21)
> + revision.c: implement --max-count-oldest
>
> "git rev-list" (and "git log" family of commands) learned a new "--max-count-oldest"
> that picks oldest N commits in the range instead of the usual newest.
>
> Will merge to 'master'.
> source: <xmqq4ijm3p2x.fsf@gitster.g>
Might want to wait and merge it in with [1].
[1] https://lore.kernel.org/git/a804828a046d8f12ef0d03eaf014807b079bb707.1781102091.git.mroik@delayed.space/
^ permalink raw reply
* Re: [PATCH 1/6] SubmittingPatches: encourage trailer use for substantial help
From: Junio C Hamano @ 2026-06-11 16:44 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <encourage_substantial.8f4@msgid.xyz>
kristofferhaugsbakk@fastmail.com writes:
> Let’s replace “If you like” with outright encouragment in this section
"encouragement"?
> At the same, it is important to temper this recommendation to a sign-
> ificant enough contribution; in my experience beginners can be eager
"At the same time"?
It is a bit unusual to see a long word split at the end of a line
to line-wrap in our documentation and commit log messages.
> ---
> Documentation/SubmittingPatches | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
The patch text itself looks great. Thanks.
^ permalink raw reply
* Re: [PATCH 3/6] SubmittingPatches: discourage common Linux trailers
From: Junio C Hamano @ 2026-06-11 16:46 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <discourage_Linux.8f6@msgid.xyz>
kristofferhaugsbakk@fastmail.com writes:
> This project does regularly mention what commits a patch/commit fixes,
> but that is done inline in the commit message proper (c.f. the trailer
> block of the message).
"cf."?
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/ [2]
;-)
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
> Documentation/SubmittingPatches | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 51c308a89a8..5dc32128883 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -479,6 +479,10 @@ to be accepted since these are the most common ones. But another kind of
> trailer might be relevant, for example to link to an issue tracker
> belonging to a downstream project that is affected by a bug in Git.
>
> +Other projects might regularly refer to other kinds of data, like
> +`Fixes:` and `Link:` in the Linux Kernel project, but these ones in
> +particular are not used in this project.
> +
> Only capitalize the very first letter of the trailer, i.e. favor
> "Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
^ permalink raw reply
* Re: [PATCH 4/6] SubmittingPatches: document Based-on-patch-by trailer
From: Junio C Hamano @ 2026-06-11 16:52 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <Based-on-patch-by.8f7@msgid.xyz>
kristofferhaugsbakk@fastmail.com writes:
> +. `Based-on-patch-by:` can be used when someone else authored parts of
> + the patch that you are submitting. This might be relevant if someone
> + sent a patch to the mailing list without a commit message or a
> + `Signed-off-by:` and you have picked it up.
Hmph, this seems to encourage pick up material that come outside of
the usual DCO process, which should not be the intention of this
document.
Unless the changes are trivial enough to not be copyrightable, it
may be better to say "... if someone submitted a preliminary patch or
a detailed code snippet with their sign-off", plus encourage asking
the original author to sign-off if it initially came without, or
something like that?
> . `Helped-by:` is used to credit someone who suggested ideas for
> changes without providing the precise changes in patch form.
> . `Mentored-by:` is used to credit someone with helping develop a
^ permalink raw reply
* [PATCH v3 0/3] doc: config: fix AsciiDoc glitches
From: Tuomas Ahola @ 2026-06-11 16:19 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Junio C Hamano, Jeff King,
Jean-Noël Avila, Tuomas Ahola
In-Reply-To: <20260610185148.23920-1-taahol@utu.fi>
Fix various markup shortcomings in git-config(1).
Based on 29bd7ed512 (The second batch, 2026-05-12).
Tuomas Ahola (3):
doc: config: terminate runaway lists
doc: config/sideband: fix description list delimiter
doc: git-config: escape erroneous highlight markup
Documentation/config.adoc | 4 +++-
Documentation/config/promisor.adoc | 2 ++
Documentation/config/safe.adoc | 2 ++
Documentation/config/sideband.adoc | 2 +-
Documentation/git-config.adoc | 8 +++++---
5 files changed, 13 insertions(+), 5 deletions(-)
Intervall-diff mot v2:
-: ---------- > 1: c9131b23fd doc: config: terminate runaway lists
-: ---------- > 2: ca65211ea4 doc: config/sideband: fix description list delimiter
1: 0341a4bde9 ! 3: e2d0cc8218 doc: git-config: escape erroneous highlight markup
@@ Commit message
Paired octothorpes are used in AsciiDoc to mark highlighted text,
<mark> being the equivalent HTML tag. To use the symbol as a literal
- character, it can be escaped with a backslash.
+ character, it can be escaped with backticks.
Do so in git-config.adoc.
@@ Documentation/git-config.adoc: OPTIONS
+
If _<message>_ begins with one or more whitespaces followed
-by "#", it is used as-is. If it begins with "#", a space is
-+by "\#", it is used as-is. If it begins with "\#", a space is
- prepended before it is used. Otherwise, a string " # " (a
+-prepended before it is used. Otherwise, a string " # " (a
++by `#`, it is used as-is. If it begins with `#`, a space is
++prepended before it is used. Otherwise, a string `" # "` (a
space followed by a hash followed by a space) is prepended
-to it. And the resulting string is placed immediately after
+to it. The resulting string is placed immediately after
base-commit: 29bd7ed5127255713c1ac2f43b7c6f257d7b4594
--
2.30.2
^ permalink raw reply
* [PATCH v3 2/3] doc: config/sideband: fix description list delimiter
From: Tuomas Ahola @ 2026-06-11 16:19 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Junio C Hamano, Jeff King,
Jean-Noël Avila, Tuomas Ahola
In-Reply-To: <20260611161946.12166-1-taahol@utu.fi>
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
Notes (doc-diff):
diff --git a/c9131b23fd1c8611fde5664fcfd4e6d5283648ad/home/taahol/share/man/man1/git-config.1 b/ca65211ea4c351071c5e76dabe4700ad074b75d3/home/taahol/share/man/man1/git-config.1
index d915897ca5..e0e2bf3c36 100644
--- a/c9131b23fd1c8611fde5664fcfd4e6d5283648ad/home/taahol/share/man/man1/git-config.1
+++ b/ca65211ea4c351071c5e76dabe4700ad074b75d3/home/taahol/share/man/man1/git-config.1
@@ -6827,8 +6827,10 @@ CONFIGURATION FILE
color
Allow ANSI color sequences, line feeds and horizontal tabs, but
mask all other control characters. This is the default.
- cursor:: Allow control sequences that move the cursor. This is
- disabled by default.
+
+ cursor
+ Allow control sequences that move the cursor. This is disabled
+ by default.
erase
Allow control sequences that erase charactrs. This is disabled
Documentation/config/sideband.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index 96fade7f5f..06de0d5c07 100644
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -9,7 +9,7 @@ sideband.allowControlCharacters::
`color`::
Allow ANSI color sequences, line feeds and horizontal tabs,
but mask all other control characters. This is the default.
- `cursor:`:
+ `cursor`::
Allow control sequences that move the cursor. This is
disabled by default.
`erase`::
--
2.30.2
^ permalink raw reply related
* Re: [PATCH v3] index-pack: retain child bases in delta cache
From: Junio C Hamano @ 2026-06-11 17:29 UTC (permalink / raw)
To: Jeff King
Cc: Arijit Banerjee, Arijit Banerjee via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Arijit Banerjee
In-Reply-To: <20260611065748.GF2191159@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jun 10, 2026 at 07:51:19AM -0700, Junio C Hamano wrote:
>
>> Arijit Banerjee <arijit91@gmail.com> writes:
>>
>> > Apologies, my earlier replies were sent through GitHub's notification
>> > emails and appeared only as PR comments, so they did not reach the mailing
>> > list.
>> >
>> > On Thu, Jun 4, 2026, Jeff King wrote:
>> >> So I am happy with either v2 or v3.
>> >
>> > I also did not see a meaningful performance difference between v2 and v3.
>> > I am happy with either direction and defer to the maintainers on whether
>> > v3's more precise release is worth the added complexity.
>>
>> I have no strong preference either way.
>
> Nor me. I'd probably go with v2 simply because it is shorter and less
> code. If there is an optimization whose effect we cannot measure, it is
> probably not worth even the few lines to have it. It could always be
> resurrected if somebody finds a case where it matters.
>
> -Peff
Sounds like a good idea. I just resurrected v2 from my reflog ;-)
Let's mark the topic for 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 3/3] doc: git-config: escape erroneous highlight markup
From: Junio C Hamano @ 2026-06-11 17:33 UTC (permalink / raw)
To: Jeff King; +Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <20260611083139.GA2237523@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Using backticks does work, though it always opens a typographical
> question. When reading the source, you see `#`, so you get a punctuation
> delimiter but no typographical one. In the rendered output, you'll see
> it in a typewriter font (assuming we fix the config issue), but we'd
> lose the visible punctuation. I could live with that.
> ...
> you might hope by asciidoc. Doing `" # "` does work, and is probably OK
> enough here.
Yucky, yucky, asciidoc. I think these literal notation `#` would be
a good way forward, given the constraints.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] ls-files: filter pathspec before lstat
From: Junio C Hamano @ 2026-06-11 17:38 UTC (permalink / raw)
To: Jeff King; +Cc: Tamir Duberstein, git, René Scharfe, Patrick Steinhardt
In-Reply-To: <20260611084132.GK2191159@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Yeah, absolutely it's arbitrary. The simplest answer is that others
>> are welcome to bump this, provided they make the case for it.
>
> OK. I can live with, I suppose, but I am tempted to say that it should
> just kick in always (i.e., removing the pathspec.nr check).
Yeah, that is certainly simpler, and this ...
> Though I did show a case where the performance regresses, it was pretty
> made-up and not something I'd expect in the real world. And you'd see
> that same crappy performance with "git ls-files -- $(git ls-files)",
> without the "-m".
... makes it clear that "trigger only when there is one element in
the pathspec" is optimizing for a wrong case.
I think we want the log message document that this kind of thinking
went into the final choice of the heuristics, like, "trigger only
when there is one because ...", or "even though it would actually be
an anti-optimization when the pathspec has enourmous number of
elements, we always use this optimization because ...", but as long
as that is done, either solution is fine.
Thanks.
^ permalink raw reply
* Re: git-diff in a worktree is an order of magnitude slower?
From: Junio C Hamano @ 2026-06-11 17:43 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Git
In-Reply-To: <20260611085526.GL2191159@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> possible, but do not use it without USE_NSEC, 2009-03-04), which details
> some reasons you might not want USE_NSEC. Feels like it ought to be a
> run-time config, though, and maybe even something that gets auto-probed
> by git-init.
I thought for a bit but didn't think of a clean way to auto-probe if
a filesystem loses nanosecond-precision part of .st_Xtime when
"metadata is flushed and later read back in" with reasonable
overhead. I do not think we want to trigger system-wide sync and/or
dropping of buffer cache ;-)
> Definitely not an area I have looked at much, though, nor thought hard
> about. So there might be gotchas. :)
>
> -Peff
^ permalink raw reply
* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Junio C Hamano @ 2026-06-11 17:48 UTC (permalink / raw)
To: Derrick Stolee
Cc: Kristofer Karlsson via GitGitGadget, git, Kristofer Karlsson
In-Reply-To: <ffaf26b1-c55e-43c7-84b6-f810a54f7717@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
> Finally, a commentary: You seem to have a habit of responding to
> review feedback only through new patch versions, but I'd rather see
> some thoughts in the discussion thread as direct replies to the review,
> especially if you think you will change direction like this. Saying
> something like "Maybe I should update the method to have two walk modes"
> in a reply would have given me an opportunity to respond and perhaps
> avoided a new version that went in this direction.
Thanks for saying this.
I haven't (yet) found it in my exchange with Kristofer, but I did
find similar irritations during review sessions with other
contributors.
I wonder if we should talk about it in the SubmittingPatches and/or
MyFirstContribution document?
^ permalink raw reply
* Re: [PATCH v5 06/10] reset: introduce ability to skip updating HEAD
From: Junio C Hamano @ 2026-06-11 18:00 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Pablo Sabater, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-6-34d35725559c@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Note that in a previous iteration we instead introduced a flag that made
> callers opt out of updating any references. This was somewhat awkward
> though because we already have the `UPDATE_ORIG_HEAD` flag, so the
> result was somewhat inconsistent.
>
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/rebase.c | 14 ++++++++++----
> reset.c | 9 +++++++--
> reset.h | 9 ++++++---
> sequencer.c | 4 +++-
> 4 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/reset.c b/reset.c
> ...
> @@ -129,7 +133,7 @@ int reset_working_tree(struct repository *r,
> oid = &head_oid;
>
> if (refs_only) {
> - if (!dry_run)
> + if (update_head)
> return update_refs(r, opts, oid, head);
> return 0;
> }
So when refs_only and update-head are in effect, we will call
update_refs(), even if dry_run is given. update_refs() does not
seem to pay attention to (opts->flags & RESET_WORKING_TREE_DRY_RUN)
at all, so wouldn't this mean that we would update even in a dry-run
session?
^ permalink raw reply
* Re: [PATCH v2] update-ref: add --rename option
From: Junio C Hamano @ 2026-06-11 18:47 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <aiqytJD-rcEirhgE@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> One thing that I'm missing from the commit message: what's the
> motivation for this new mode?
Maintenance of merge-fix database, a kludgy way to manage evil
merges that are needed to deal with inter-topic semantic crashes.
If you are really interested, see the appendix.
>> diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
>> index 37a5019a8b..0c27efaa52 100644
>> --- a/Documentation/git-update-ref.adoc
>> +++ b/Documentation/git-update-ref.adoc
>> @@ -39,6 +40,14 @@ the result of following the symbolic pointers.
>> With `-d`, it deletes the named <ref> after verifying that it
>> still contains <old-oid>.
>>
>> +With `--rename`, it renames <old-refname> together with its reflog to
>> +<new-refname>. The command fails if <old-refname> does not exist, or
>> +if <new-refname> already exists. Because `git update-ref` does not
>> +update active worktree `HEAD` symbolic references or `.git/config`
>> +tracking settings when you rename a local branch in the `refs/heads/`
>> +hierarchy, think twice before using this command to rename a local
>> +branch (use `git branch -m` instead).
>
> I'd rephrase this slightly to first document behaviour and then draw the
> conclusion that it shouldn't be used in many cases separately. For
> example:
>
> This command does not update any symbolic references pointing to
> the renamed reference, and neither does it update `.git/config`
> tracking settings. It is thus not recommended to use it for renaming
> local branches. Use `git branch -m` instead.
Thanks, that is much better.
>> + if (!refs_ref_exists(get_main_ref_store(the_repository), oldref))
>> + die("no ref named '%s'", oldref);
>> +
>> + if (refs_ref_exists(get_main_ref_store(the_repository), newref))
>> + die("ref '%s' already exists", newref);
>> +
>> + if (refs_rename_ref(get_main_ref_store(the_repository),
>> + oldref, newref, msg))
>> + die("rename failed");
>> + return 0;
>> + }
>
> Hm. I think we're not using "--deref" / "--no-deref" at all, but we
> document this flag as accepted in the synopsis.
Good point. refs_rename_ref() never derefs, right? We should drop
these two from the synopsis section.
[Appendix]
Often there are two topics, A and B, in flight that merging A into B
(or vice versa) requires changes more than the mechanical merge
needs. If this is a one-shot merge of A into B (or B into A), then
we can just record the evil merge and be done with it, but the same
issue arises if you are merging A into 'seen' first and then later
(possibly after merging other topics on top) B into 'seen'. The
merge of 'B' needs the same evil merge to resolve semantic
conflicts.
As those familiar with how 'seen' works in my tree, reapplying such
evil merges MUST BE automated, or the project will not work at all,
as 'seen' is rebuilt at least twice during the day, or even more
often.
So, what I do is, when I merge 'B' into 'seen' after merging 'A' and
possibly some other topics, I let the rerere database to record the
resolution of textual conflicts and make a commit. The tree
recorded in this commit will not work, due to semantic conflicts. I
create another commit on top of this merge to resolve the semantic
conflict to make the tree work.
Let's take ps/history-drop (A) and ps/setup-drop-global-state (B) as
an easy-to-understand example.
$ git checkout --detach ps/history-drop
$ git merge ps/setup-drop-global-state
This textually merges cleanly, but the result would not compile.
The history-drop added a new call to "is_bare_repository()", while
setup-drop-global-state added an extra parameter to the function.
So a merge-fix prepared on top of this "textually clean but does
not work" merge is created and looks something like this:
diff --git a/builtin/history.c b/builtin/history.c
index 65845e7359..eece221e63 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1150,7 +1150,7 @@ static int cmd_history_drop(int argc,
* inconsistent repository state. So we first perform a dry-run merge
* here before updating refs.
*/
- if (!is_bare_repository()) {
+ if (!is_bare_repository(repo)) {
ret = find_head_tree_change(repo, &result, &old_head,
&new_head, &head_moves);
if (ret < 0)
And this commit (i.e. a commit on top of the mechanical/textual
merge result that adjusts the non-working merge result into workable
form) is pointed at by refs/merge-fix/ps/setup-drop-global-state.
Rebuilding 'seen' is driven by a script that takes a moral
equivalent of 'git log --first-parent --oneline --reverse
master..seen' and replays each merge on top of what is checked out
(to bootstrap, you would "git checkout -B seen master" and start
there). For each topic branch found in the input, the script
(1) skips if the topic has been merged and move on to the next
topic.
(2) runs "git merge" of the topic, taking resolution by the rerere
database. If this step leaves mechanical/textual conflicts,
the script stops and I'll hand resolve to update my rerere
database, and rerun the script (which will succeed the next
time).
(3) runs "git cherry-pick --no-commit merge-fix/$topic" if such a
ref exists, and if successfull, runs "git commit --amend".
That is how merging ps/setup-drop-global-state into 'seen' that has
already merged ps/history-drop would automatically get the right
evil merge to resolve semantic conflicts.
The renaming of update-ref becomes needed when the order of merging
topics into 'seen' changes. Ideally, these cherry-pickable commits
that are stored under refs/merge-fix hierarchies SHOULD be indexable
by a pair of topic (i.e. "when topic A and topic B first meets, apply
this evil merge"), but this computation is cumbersome to write, so
the above scheme has baked-in assumption that we know which topic
comes later. Once we start merging ps/setup-drop-global-state first
and then ps/history-drop next, we would need
$ git update-ref --rename \
refs/merge-fix/ps/setup-drop-global-state \
refs/merge-fix/ps/history-drop
^ permalink raw reply related
* Re: What's cooking in git.git (Jun 2026, #04)
From: Junio C Hamano @ 2026-06-11 18:55 UTC (permalink / raw)
To: Mirko Faina; +Cc: git
In-Reply-To: <airkGWlc69uVsVa8@exploit>
Mirko Faina <mroik@delayed.space> writes:
> On Thu, Jun 11, 2026 at 09:08:25AM -0700, Junio C Hamano wrote:
>> * mf/revision-max-count-oldest (2026-05-18) 1 commit
>> (merged to 'next' on 2026-06-09 at 076600fa21)
>> + revision.c: implement --max-count-oldest
>>
>> "git rev-list" (and "git log" family of commands) learned a new "--max-count-oldest"
>> that picks oldest N commits in the range instead of the usual newest.
>>
>> Will merge to 'master'.
>> source: <xmqq4ijm3p2x.fsf@gitster.g>
>
> Might want to wait and merge it in with [1].
>
> [1] https://lore.kernel.org/git/a804828a046d8f12ef0d03eaf014807b079bb707.1781102091.git.mroik@delayed.space/
Thanks for reminding me. Will do.
^ permalink raw reply
* Re: [PATCH v2 2/2] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-11 20:10 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, Jeff King, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <CAOLa=ZSezQOj56-TezVaAcisUyczxhJmu4VghyFBHcBB_mKJ2A@mail.gmail.com>
On Thu, Jun 11, 2026 at 1:16 AM Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Tamir Duberstein <tamird@gmail.com> writes:
>
> > On Wed, Jun 10, 2026 at 4:47 AM Karthik Nayak <karthik.188@gmail.com> wrote:
> >>
> >> Tamir Duberstein <tamird@gmail.com> writes:
> >>
> >> > git branch and git for-each-ref call repo_is_descendant_of() for
> >> > each candidate selected by --contains or --no-contains. Each call
> >> > starts a new graph walk, so refs with shared history repeatedly
> >> > traverse the same commits.
> >> >
> >> > ffc4b8012d (tag: speed up --contains calculation, 2011-06-11)
> >> > introduced a depth-first walk for git tag that caches positive and
> >> > negative answers across candidates. ee2bd06b0f (ref-filter: implement
> >> > '--contains' option, 2015-07-07) preserved both implementations when
> >> > ref-filter learned --contains.
> >> >
> >> > The memoized walk is not always faster. Without generation numbers,
> >> > a negative check can walk to the root even when the breadth-first
> >> > merge-base walk finds a nearby divergence. With generation numbers,
> >> > the depth-first walk can stop below the oldest target while still
> >> > reusing answers across candidates.
> >> >
> >> > Keep the existing memoized selection for git tag. Select it for other
> >> > ref-filter callers when generation numbers are enabled, and retain
> >> > the breadth-first walk otherwise.
> >> >
> >> > When generation numbers are unavailable, repo_is_descendant_of() can
> >> > return -1 if ancestry cannot be read. The ref-filter Boolean interface
> >> > treated that error as a match. Check it and exit instead. The memoized
> >> > path already dies on the same parse failure, so both selected paths now
> >> > fail rather than return a result.
> >> >
> >> > Add p1500 cases for up to 8,192 packed refs along one first-parent
> >> > history and for sibling refs near the tip with generation numbers
> >> > forced off.
> >> >
> >> > On a checkout with 62,174 remote-tracking refs and generation numbers
> >> > enabled, I ran:
> >> >
> >> > hyperfine --warmup 0 --runs 3 \
> >> > --command-name parent \
> >> > '"$parent" branch -r --contains c78ae85f3ce7e >/dev/null' \
> >> > --command-name this-commit \
> >> > '"$this" branch -r --contains c78ae85f3ce7e >/dev/null'
> >> >
> >> > The results were:
> >> >
> >> > parent this commit
> >> > elapsed 104.365 s 467.7 ms
> >> > user 93.702 s 220.2 ms
> >> > system 0.723 s 182.7 ms
> >> >
> >> > The wall-time standard deviations were 11.356 seconds and 133.8
> >> > milliseconds, respectively. Separate runs without redirection produced
> >> > the same output with SHA-256
> >> > 2466f6e2b72aa16b1a2126eddb81c8a1b2764ee251204ac034c191a925aa896f.
> >> >
> >> > Both revisions were built with the default -O2 flags using Apple
> >> > clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro (Mac16,6)
> >> > with a 16-core Apple M4 Max (12 performance and four efficiency
> >> > cores) and 128 GB RAM.
> >> >
> >> > Link: https://lore.kernel.org/git/1445163904-24611-1-git-send-email-Karthik.188@gmail.com/
> >> > Link: https://lore.kernel.org/r/20230324191009.GA536967@coredump.intra.peff.net
> >> > Link: https://lore.kernel.org/git/20260527070510.3510836-1-krka@spotify.com/
> >> > Link: https://lore.kernel.org/r/20260608223430.GA340696@coredump.intra.peff.net
> >> > Suggested-by: Jeff King <peff@peff.net>
> >> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> >> > ---
> >> > commit-reach.c | 13 +++++++++--
> >> > commit-reach.h | 7 ++++++
> >> > t/perf/p1500-graph-walks.sh | 49 +++++++++++++++++++++++++++++++++++++++++-
> >> > t/t6301-for-each-ref-errors.sh | 22 +++++++++++++++++++
> >> > 4 files changed, 88 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/commit-reach.c b/commit-reach.c
> >> > index 65b618959b..83a48004ef 100644
> >> > --- a/commit-reach.c
> >> > +++ b/commit-reach.c
> >> > @@ -821,9 +821,18 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
> >> > int commit_contains(struct ref_filter *filter, struct commit *commit,
> >> > struct commit_list *list, struct contains_cache *cache)
> >> > {
> >> > - if (filter->with_commit_tag_algo)
> >> > + int result;
> >> > +
> >> > + if (!list)
> >> > + return 1;
> >> > + if (filter->with_commit_tag_algo ||
> >> > + generation_numbers_enabled(the_repository))
> >>
> >> What's stopping us from dropping `filter->with_commit_tag_algo`
> >> completely and then doing?
> >>
> >> if (generation_numbers_enabled(the_repository))
> >> return contains_algo(commit, list, cache) == CONTAINS_YES;
> >> return repo_is_descendant_of(the_repository, commit, list);
> >
> > Jeff raised this distinction during the v1 review:
> >
> > https://lore.kernel.org/r/20260608223430.GA340696@coredump.intra.peff.net/
> >
> > `with_commit_tag_algo` preserves the existing behavior of `git tag` when
> > generation numbers are unavailable. `git tag --contains` has used the
> > memoized walk since ffc4b8012d (tag: speed up --contains calculation,
> > 2011-06-11). Dropping the flag would send it back through repeated
> > `repo_is_descendant_of()` walks in repositories without usable generation
> > numbers.
> >
>
> I did read that, my question is on top of that. Do we also want to use
> the non-memoized walk for 'git tag' when there are no generation numbers
> available or does that not work? If not, we should mention that too in
> the commit message.
We should keep the memoized walk for git tag. In git.git, with commit
graphs disabled, hyperfine measured:
git -c core.commitGraph=false tag --contains HEAD~200
git -c core.commitGraph=false for-each-ref \
--contains HEAD~200 refs/tags/
at 478.9 ms and 4.861 s, respectively. The second command takes the
non-memoized path, so memoization is about 10 times faster for this
workload. I added the rationale to the commit message in v3.
^ permalink raw reply
* Re: followRemoteHEAD management question
From: Bence Ferdinandy @ 2026-06-11 20:36 UTC (permalink / raw)
To: Jeff King, Matt Hunter; +Cc: git
In-Reply-To: <20260611060123.GA2187173@coredump.intra.peff.net>
On Thu Jun 11, 2026 at 08:01, Jeff King <peff@peff.net> wrote:
>
> My initial thought is that it might affect clone as well as fetch. But I
> guess this feature does not kick in for clone, as it has its own logic
> for handling the remote-tracking HEAD. Though arguably it should be
> possible to configure it not to create one in the first place.
If memory serves well clone has set the remote/HEAD well before this and
I think it indeed uses a different mechanism/logic.
>
>> As for another design decision: I'm leaning toward omitting support for
>> the "warn-if-not-$branch" value in fetch.followRemoteHEAD.
>>
>> My take on that option as-documented is that it serves more as an
>> acknowledgment from the user that "yes, I understand that origin has
>> pointed HEAD at foo, please only warn me if it changes" as opposed to the
>> user expressing that the branch "foo" is in some way special to them.
Yes, that was the reasoning. So I also agree on not adding it to global.
Bit late to the party, but happy to review/test patches if they come.
Best,
Bence
^ permalink raw reply
* Re: [PATCH v2 3/3] doc: git-config: escape erroneous highlight markup
From: Jean-Noël AVILA @ 2026-06-11 20:43 UTC (permalink / raw)
To: Tuomas Ahola, Jeff King; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260611062525.GB2189088@coredump.intra.peff.net>
On Thursday, 11 June 2026 08:25:25 CEST Jeff King wrote:
> [and naturally I forgot to cc Jean-Noël; resending, sorry for the noise]
>
> On Thu, Jun 11, 2026 at 02:24:23AM -0400, Jeff King wrote:
> > On Thu, Jun 11, 2026 at 02:11:57AM -0400, Jeff King wrote:
> > > Though curiously the case of `#` in git-fast-import seems not to get
> > > marked as <code> in the html output (even though the nearby `LF` does).
> > > I wonder if there is some special treatment of `#` or something.
> >
> > Ah, weird, it has to do with our config file.
> >
> > If I do this (not in the git repository):
> > echo 'This is a literal `#` symbol.' >foo.adoc
> > asciidoc foo.adoc
> > grep -i symbol foo.html
> >
> > then I get <code> markers, like:
> > <div class="paragraph"><p>This is a literal <code>#</code> symbol.</p></div>
> >
> > But if I build with:
> > asciidoc -f path/to/git/Documentation/asciidoc.conf foo.adoc
> >
> > then the grep shows:
> > <div class="paragraph"><p>This is a literal # symbol.</p></div>
> >
> > Looks like it is due to our [literal-inlinemacro] definition, which
> > comes from 974cdca345 (doc: introduce a synopsis typesetting,
> > 2024-09-24). I think this might have been an unintended side effect.
> > +cc the author of that commit.
> >
> > For the purposes of your series, I think we can ignore any issues with
> > [literal-inlinemacro] for the moment, and decide on "\" versus ``
> > depending on which we prefer.
> >
> > -Peff
Oh, this is the black magic regexp that is not considering # for keyword
character. Should be solved by something like (and I really hate these .in
files):
-- >8 --
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
Date: Thu, 11 Jun 2026 19:44:43 +0200
Subject: [PATCH] asciidoc: fix handling of # in synopsis text
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
There are occurrences of # in the synopsis text of git-config(1) and
git-clone(1) that are not handled as keyword by the current asciidoc
and asciidoctor processors.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/asciidoc.conf.in | 12 ++++++------
Documentation/asciidoctor-extensions.rb.in | 6 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/Documentation/asciidoc.conf.in b/Documentation/asciidoc.conf.in
index 31b883a72c..b50fad588e 100644
--- a/Documentation/asciidoc.conf.in
+++ b/Documentation/asciidoc.conf.in
@@ -43,7 +43,7 @@ ifdef::doctype-book[]
endif::doctype-book[]
[literal-inlinemacro]
-{eval:re.sub(r'(<[-a-zA-Z0-9.]+>)', r'<emphasis>\1</emphasis>', re.sub(r'([\[\s|()>]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@\\\*\/_^\$%]+\.?)+|,)',r'\1<literal>\2</literal>', re.sub(r'(\.\.\.?)([^\]$.])', r'<literal>\1</literal>\2', macros.passthroughs[int(attrs['passtext'][1:-1])] if attrs['passtext'][1:-1].isnumeric() else attrs['passtext'][1:-1])))}
+{eval:re.sub(r'(<[-a-zA-Z0-9.]+>)', r'<emphasis>\1</emphasis>', re.sub(r'([\[\s|()>]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@#\\\*\/_^\$%]+\.?)+|,)',r'\1<literal>\2</literal>', re.sub(r'(\.\.\.?)([^\]$.])', r'<literal>\1</literal>\2', macros.passthroughs[int(attrs['passtext'][1:-1])] if attrs['passtext'][1:-1].isnumeric() else attrs['passtext'][1:-1])))}
endif::backend-docbook[]
@@ -75,24 +75,24 @@ git-relative-html-prefix=
<a href="{git-relative-html-prefix}{target}.html">{target}{0?({0})}</a>
[literal-inlinemacro]
-{eval:re.sub(r'(<[-a-zA-Z0-9.]+>)', r'<em>\1</em>', re.sub(r'([\[\s|()>]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@,\\\*\/_^\$]+\.?)+)',r'\1<code>\2</code>', re.sub(r'(\.\.\.?)([^\]$.])', r'<code>\1</code>\2', macros.passthroughs[int(attrs['passtext'][1:-1])] if attrs['passtext'][1:-1].isnumeric() else attrs['passtext'][1:-1])))}
+{eval:re.sub(r'(<[-a-zA-Z0-9.]+>)', r'<em>\1</em>', re.sub(r'([\[\s|()>]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@#,\\\*\/_^\$]+\.?)+)',r'\1<code>\2</code>', re.sub(r'(\.\.\.?)([^\]$.])', r'<code>\1</code>\2', macros.passthroughs[int(attrs['passtext'][1:-1])] if attrs['passtext'][1:-1].isnumeric() else attrs['passtext'][1:-1])))}
endif::backend-xhtml11[]
ifdef::backend-docbook[]
ifdef::doctype-manpage[]
[blockdef-open]
-synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<phrase>\\0</phrase>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<literal>\\2</literal>!g;s!<[-a-zA-Z0-9.]\\+>!<emphasis>\\0</emphasis>!g'"
+synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<phrase>\\0</phrase>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@#,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<literal>\\2</literal>!g;s!<[-a-zA-Z0-9.]\\+>!<emphasis>\\0</emphasis>!g'"
[paradef-default]
-synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<phrase>\\0</phrase>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<literal>\\2</literal>!g;s!<[-a-zA-Z0-9.]\\+>!<emphasis>\\0</emphasis>!g'"
+synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<phrase>\\0</phrase>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@#,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<literal>\\2</literal>!g;s!<[-a-zA-Z0-9.]\\+>!<emphasis>\\0</emphasis>!g'"
endif::doctype-manpage[]
endif::backend-docbook[]
ifdef::backend-xhtml11[]
[blockdef-open]
-synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<span>\\0</span>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<code>\\2</code>!g;s!<[-a-zA-Z0-9.]\\+>!<em>\\0</em>!g'"
+synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<span>\\0</span>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@#,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<code>\\2</code>!g;s!<[-a-zA-Z0-9.]\\+>!<em>\\0</em>!g'"
[paradef-default]
-synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<span>\\0</span>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<code>\\2</code>!g;s!<[-a-zA-Z0-9.]\\+>!<em>\\0</em>!g'"
+synopsis-style=template="verseparagraph",filter="sed 's!…\\(\\]\\|$\\)!<span>\\0</span>!g;s!\\([\\[ |()]\\|^\\|\\]\\|>\\)\\([-=a-zA-Z0-9:+@#,\\/_^\\$.\\\\\\*]\\+\\|…\\)!\\1<code>\\2</code>!g;s!<[-a-zA-Z0-9.]\\+>!<em>\\0</em>!g'"
endif::backend-xhtml11[]
diff --git a/Documentation/asciidoctor-extensions.rb.in b/Documentation/asciidoctor-extensions.rb.in
index fe64a62d96..b5f06827ca 100644
--- a/Documentation/asciidoctor-extensions.rb.in
+++ b/Documentation/asciidoctor-extensions.rb.in
@@ -50,7 +50,7 @@ module Git
def process parent, reader, attrs
outlines = reader.lines.map do |l|
l.gsub(/(\.\.\.?)([^\]$\. ])/, '{empty}`\1`{empty}\2')
- .gsub(%r{([\[\] |()>]|^)([-a-zA-Z0-9:+=~@,/_^\$\\\*]+)}, '\1{empty}`\2`{empty}')
+ .gsub(%r{([\[\] |()>]|^)([-a-zA-Z0-9:+=~@#,/_^\$\\\*]+)}, '\1{empty}`\2`{empty}')
.gsub(/(<[-a-zA-Z0-9.]+>)/, '__\\1__')
.gsub(']', ']{empty}')
end
@@ -73,7 +73,7 @@ module Git
elsif type == :monospaced
node.text.gsub(/(\.\.\.?)([^\]$\.])/, '<literal>\1</literal>\2')
.gsub(/^\.\.\.?$/, '<literal>\0</literal>')
- .gsub(%r{([\[\s|()>.]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@/_^\$\\\*%]+\.{0,2})+|,)}, '\1<literal>\2</literal>')
+ .gsub(%r{([\[\s|()>.]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@#/_^\$\\\*%]+\.{0,2})+|,)}, '\1<literal>\2</literal>')
.gsub(/(<[-a-zA-Z0-9.]+>)/, '<emphasis>\1</emphasis>')
else
open, close, supports_phrase = QUOTE_TAGS[type]
@@ -102,7 +102,7 @@ module Git
if node.type == :monospaced
node.text.gsub(/(\.\.\.?)([^\]$.])/, '<code>\1</code>\2')
.gsub(/^\.\.\.?$/, '<code>\0</code>')
- .gsub(%r{([\[\s|()>.]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@,/_^\$\\\*%]+\.{0,2})+)}, '\1<code>\2</code>')
+ .gsub(%r{([\[\s|()>.]|^|\]|>)(\.?([-a-zA-Z0-9:+=~@#,/_^\$\\\*%]+\.{0,2})+)}, '\1<code>\2</code>')
.gsub(/(<[-a-zA-Z0-9.]+>)/, '<em>\1</em>')
else
--
^ permalink raw reply related
* t5563-simple-http-auth failures with v2.55.0-rc0
From: Todd Zullinger @ 2026-06-11 21:04 UTC (permalink / raw)
To: git; +Cc: Matthew John Cheetham
Hi,
I tested the freshly-tagged 2.55.0-rc0 and noticed some new
failures on the in-progress Fedora 45 (AKA Rawhide) for
t5563.18 (http.emptyAuth=auto attempts Negotiate before
credential_fill) which was added in 9b1630b972 (t5563: add
tests for http.emptyAuth with Negotiate, 2026-04-16).
I notice that Fedora 44 (where the tests all pass) has
curl-8.18.0 while Fedora 45 has curl-8.21.0-rc2. The
version of httpd is the same between them, FWIW. I didn't
compare other package differences; it could be something
else entirely.
Here is the output from a failing test run:
--8<--
++ test_when_finished per_test_cleanup
++ test 0 = 0
++ test_cleanup=$'{ per_test_cleanup\n\t\t} || eval_ret=$?; :'
++ set_credential_reply get
+++ test -n ''
++ local suffix=
++ cat
++ cat
++ cat
++ test_config_global credential.helper test-helper
++ test_when_finished 'test_unconfig --global '\''credential.helper'\'''
++ test 0 = 0
++ test_cleanup=$'{ test_unconfig --global \'credential.helper\'\n\t\t} || eval_ret=$?; { per_test_cleanup\n\t\t} || eval_ret=$?; :'
++ git config --global credential.helper test-helper
++ GIT_TRACE_CURL='/builddir/build/BUILD/git-2.55.0_rc0-build/git-2.55.0.rc0/t/trash directory.t5563-simple-http-auth/trace-auto'
++ git -c http.emptyAuth=auto ls-remote http://127.0.0.1:5563/custom_auth/repo.git
ddd63c907a6168e9992caee4ef0e0fa1139e4eb3 HEAD
ddd63c907a6168e9992caee4ef0e0fa1139e4eb3 refs/heads/master
ddd63c907a6168e9992caee4ef0e0fa1139e4eb3 refs/tags/foo
++ grep 'HTTP/[0-9.]* 401' '/builddir/build/BUILD/git-2.55.0_rc0-build/git-2.55.0.rc0/t/trash directory.t5563-simple-http-auth/trace-auto'
++ test_line_count = 3 actual_401s
++ test 3 '!=' 3
+++ wc -l
++ test 2 = 3
++ echo 'test_line_count: line count for actual_401s != 3'
test_line_count: line count for actual_401s != 3
++ cat actual_401s
<= Recv header: HTTP/1.1 401 Authorization Required
<= Recv header: HTTP/1.1 401 Authorization Required
++ return 1
error: last command exited with $?=1
not ok 18 - http.emptyAuth=auto attempts Negotiate before credential_fill
--8<--
And a diff of the trace-auto from Fedora 44 and 45 via
./t5563-simple-http-auth.sh -dix --run='-18' (with the
sending port normalized to 44444 to reduce the noise):
--- /dev/fd/63 2026-06-11 16:51:05.852135692 -0400
+++ /dev/fd/62 2026-06-11 16:51:05.853135711 -0400
@@ -23,6 +23,7 @@
<= Recv header:
<= Recv data, 0000000000 bytes (0x00000000)
== Info: shutting down connection #0
+== Info: Could not find host 127.0.0.1 in the .netrc file; using defaults
== Info: NTLM-proxy picked AND auth done set, clear picked
== Info: Hostname 127.0.0.1 was found in DNS cache
== Info: Trying 127.0.0.1:5563...
@@ -47,37 +48,8 @@
== Info: no chunk, no close, no size. Assume close to signal end
<= Recv header, 0000000001 bytes (0x00000001)
<= Recv header:
-== Info: shutting down connection #1
-== Info: Issue another request to this URL: 'http://127.0.0.1:5563/custom_auth/repo.git/info/refs?service=git-upload-pack'
-== Info: NTLM-proxy picked AND auth done set, clear picked
-== Info: Hostname 127.0.0.1 was found in DNS cache
-== Info: Trying 127.0.0.1:5563...
-== Info: Established connection to 127.0.0.1 (127.0.0.1 port 5563) from 127.0.0.1 port 44444
-== Info: using HTTP/1.x
-== Info: gss_init_sec_context() failed: No credentials were supplied, or the credentials were unavailable or inaccessible. SPNEGO cannot find mechanisms to negotiate.
-== Info: Server auth using Negotiate with user ''
-=> Send header, 0000000214 bytes (0x000000d6)
-=> Send header: GET /custom_auth/repo.git/info/refs?service=git-upload-pack HTTP/1.1
-=> Send header: Host: 127.0.0.1:5563
-=> Send header: User-Agent: git/2.55.0.rc0
-=> Send header: Accept: */*
-=> Send header: Accept-Encoding: deflate, gzip, br
-=> Send header: Pragma: no-cache
-=> Send header: Git-Protocol: version=2
-=> Send header:
-== Info: Request completely sent off
-<= Recv header, 0000000036 bytes (0x00000024)
-<= Recv header: HTTP/1.1 401 Authorization Required
-== Info: gss_init_sec_context() failed: No credentials were supplied, or the credentials were unavailable or inaccessible. SPNEGO cannot find mechanisms to negotiate.
-<= Recv header, 0000000028 bytes (0x0000001c)
-<= Recv header: WWW-Authenticate: Negotiate
-<= Recv header, 0000000044 bytes (0x0000002c)
-<= Recv header: WWW-Authenticate: Basic realm="example.com"
-== Info: no chunk, no close, no size. Assume close to signal end
-<= Recv header, 0000000001 bytes (0x00000001)
-<= Recv header:
<= Recv data, 0000000000 bytes (0x00000000)
-== Info: shutting down connection #2
+== Info: shutting down connection #1
== Info: NTLM-proxy picked AND auth done set, clear picked
== Info: Hostname 127.0.0.1 was found in DNS cache
== Info: Trying 127.0.0.1:5563...
@@ -113,7 +85,7 @@
<= Recv data: orn.0020fetch=shallow wait-for-done.0012server-option.0017ob
<= Recv data: ject-format=sha1.0000
<= Recv data, 0000000000 bytes (0x00000000)
-== Info: shutting down connection #3
+== Info: shutting down connection #2
== Info: NTLM-proxy picked AND auth done set, clear picked
== Info: Hostname 127.0.0.1 was found in DNS cache
== Info: Trying 127.0.0.1:5563...
@@ -154,4 +126,4 @@
<= Recv data: 9e4eb3 refs/heads/master.003bddd63c907a6168e9992caee4ef0e0fa
<= Recv data: 1139e4eb3 refs/tags/foo.0000
<= Recv data, 0000000000 bytes (0x00000000)
-== Info: shutting down connection #4
+== Info: shutting down connection #3
The absence of one of the requests stands out. Anyone
familiar with this area have suggestions for how to further
debug it? It should reproduce easily in a Fedora 45
container, if anyone wants to poke at it more directly.
Thanks,
--
Todd
^ permalink raw reply
* Re: git-diff in a worktree is an order of magnitude slower?
From: brian m. carlson @ 2026-06-11 21:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, D. Ben Knoble, Git
In-Reply-To: <xmqqbjdhnfaf.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]
On 2026-06-11 at 17:43:52, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> > possible, but do not use it without USE_NSEC, 2009-03-04), which details
> > some reasons you might not want USE_NSEC. Feels like it ought to be a
> > run-time config, though, and maybe even something that gets auto-probed
> > by git-init.
>
> I thought for a bit but didn't think of a clean way to auto-probe if
> a filesystem loses nanosecond-precision part of .st_Xtime when
> "metadata is flushed and later read back in" with reasonable
> overhead. I do not think we want to trigger system-wide sync and/or
> dropping of buffer cache ;-)
We could have `git update-index` take options like it does for
`--untracked-cache` and `--no-untracked-cache` to control these for
people who want them. For instance, I know what operating system and
file system I'm using (Linux with btrfs), so if I know that option is
safe, I can enable it at runtime and reap the benefits.
We could even have `--test-use-nsec` to perform a `uname` and `statfs`
call to determine whether this is a known safe configuration if probing
is not possible.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply
* [PATCH v3] update-ref: add --rename option
From: Junio C Hamano @ 2026-06-11 21:37 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <xmqqv7brz9ba.fsf@gitster.g>
Add a "--rename" option to "git update-ref" with the syntax:
$ git update-ref --rename <old-refname> <new-refname>
It renames <old-refname> together with its reflog to <new-refname>;
even when used on a local branch ref, the current value and the
reflog of the ref are the only things that are renamed. Document it
and redirect casual users to "git branch -m" if that is what they
wanted to do.
Because the "--stdin" mode wants to operate on its refs in a
reference transaction, and the API function refs_rename_ref() does
not work well as part of a transaction, it is currently not possible
to add a corresponding "rename" verb to the "--stdin" mode before
the underlying API learns to rename refs atomically inside a
transaction. It hence is left for a future refactoring.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* As a single patch topic, the range-diff relative to v2 is at the
end of the message.
- Simplified the proposed commit log message a bit.
- Dropped mention of --[no-]deref from the synopsis section.
- Reworded documentation with help from Patrick.
Documentation/git-update-ref.adoc | 9 +++++++++
builtin/update-ref.c | 32 +++++++++++++++++++++++++++++--
t/t1400-update-ref.sh | 24 +++++++++++++++++++++++
3 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
index 37a5019a8b..3b4df23a86 100644
--- a/Documentation/git-update-ref.adoc
+++ b/Documentation/git-update-ref.adoc
@@ -9,6 +9,7 @@ SYNOPSIS
--------
[synopsis]
git update-ref [-m <reason>] [--no-deref] -d <ref> [<old-oid>]
+git update-ref [-m <reason>] --rename <old-refname> <new-refname>
git update-ref [-m <reason>] [--no-deref] [--create-reflog] <ref> <new-oid> [<old-oid>]
git update-ref [-m <reason>] [--no-deref] --stdin [-z] [--batch-updates]
@@ -39,6 +40,14 @@ the result of following the symbolic pointers.
With `-d`, it deletes the named <ref> after verifying that it
still contains <old-oid>.
+With `--rename`, it renames <old-refname> together with its reflog to
+<new-refname>. The command fails if <old-refname> does not exist, or
+if <new-refname> already exists. The command does not update any
+symbolic references pointing to the renamed reference, and neither
+does it update `.git/config` tracking settings. It is thus not
+recommended to use it for renaming local branches. Use `git branch -m`
+instead.
+
With `--stdin`, update-ref reads instructions from standard input and
performs all modifications together. Specify commands of the form:
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 2d68c40ecb..65ee8af08c 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -15,6 +15,7 @@
static const char * const git_update_ref_usage[] = {
N_("git update-ref [<options>] -d <refname> [<old-oid>]"),
N_("git update-ref [<options>] <refname> <new-oid> [<old-oid>]"),
+ N_("git update-ref [<options>] --rename <old-refname> <new-refname>"),
N_("git update-ref [<options>] --stdin [-z] [--batch-updates]"),
NULL
};
@@ -756,13 +757,14 @@ int cmd_update_ref(int argc,
{
const char *refname, *oldval;
struct object_id oid, oldoid;
- int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;
+ int delete = 0, rename = 0, no_deref = 0, read_stdin = 0, end_null = 0;
int create_reflog = 0;
unsigned int flags = 0;
struct option options[] = {
OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")),
OPT_BOOL('d', NULL, &delete, N_("delete the reference")),
+ OPT_BOOL( 0 , "rename", &rename, N_("rename the reference")),
OPT_BOOL( 0 , "no-deref", &no_deref,
N_("update <refname> not the one it points to")),
OPT_BOOL('z', NULL, &end_null, N_("stdin has NUL-terminated arguments")),
@@ -787,7 +789,7 @@ int cmd_update_ref(int argc,
}
if (read_stdin) {
- if (delete || argc > 0)
+ if (delete || rename || argc > 0)
usage_with_options(git_update_ref_usage, options);
if (end_null)
line_termination = '\0';
@@ -800,6 +802,32 @@ int cmd_update_ref(int argc,
if (end_null)
usage_with_options(git_update_ref_usage, options);
+ if (rename) {
+ const char *oldref, *newref;
+
+ if (delete || argc != 2)
+ usage_with_options(git_update_ref_usage, options);
+
+ oldref = argv[0];
+ newref = argv[1];
+
+ if (check_refname_format(oldref, 0))
+ die("invalid ref format: %s", oldref);
+ if (check_refname_format(newref, 0))
+ die("invalid ref format: %s", newref);
+
+ if (!refs_ref_exists(get_main_ref_store(the_repository), oldref))
+ die("no ref named '%s'", oldref);
+
+ if (refs_ref_exists(get_main_ref_store(the_repository), newref))
+ die("ref '%s' already exists", newref);
+
+ if (refs_rename_ref(get_main_ref_store(the_repository),
+ oldref, newref, msg))
+ die("rename failed");
+ return 0;
+ }
+
if (delete) {
if (argc < 1 || argc > 2)
usage_with_options(git_update_ref_usage, options);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b2858a9061..4330cad282 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -2455,4 +2455,28 @@ test_expect_success 'dangling symref overwritten without old oid' '
test_must_fail git rev-parse --verify refs/heads/does-not-exist
'
+test_expect_success '--rename fails if old-refname does not exist' '
+ test_must_fail git update-ref --rename refs/tags/no-such-ref refs/tags/new-ref 2>err &&
+ test_grep "no ref named .refs/tags/no-such-ref." err
+'
+
+test_expect_success '--rename fails if new-refname does exist' '
+ git update-ref refs/tags/existing HEAD &&
+ git update-ref refs/tags/old-ref HEAD &&
+ test_must_fail git update-ref --rename refs/tags/old-ref refs/tags/existing 2>err &&
+ test_grep "ref .refs/tags/existing. already exists" err
+'
+
+test_expect_success '--rename moves old-refname and its reflog to new-refname' '
+ test_config core.logallrefupdates always &&
+ git update-ref -m "old tag" refs/tags/old-tag HEAD &&
+ git update-ref -m "to new" --rename refs/tags/old-tag refs/tags/new-tag 2>err &&
+ test_must_be_empty err &&
+ git show-ref --exists refs/tags/new-tag &&
+ test_must_fail git show-ref --exists refs/tags/old-tag &&
+ git log -g refs/tags/new-tag >output &&
+ test_grep "old tag" output &&
+ test_grep "to new" output
+'
+
test_done
Range-diff against v2:
1: 00cd13fda7 ! 1: a54c2d4d68 update-ref: add --rename option
@@ Commit message
$ git update-ref --rename <old-refname> <new-refname>
- It renames <old-refname> together with its reflog to <new-refname>
- (even when used on a local branch ref, the current value and the
- reflog of the ref are the only things that are renamed). As the
- command is a low-level plumbing command, attempts to rename branches
- are not warned, but we document it to draw attention of unsuspecting
- users and protect them from burning themselves.
+ It renames <old-refname> together with its reflog to <new-refname>;
+ even when used on a local branch ref, the current value and the
+ reflog of the ref are the only things that are renamed. Document it
+ and redirect casual users to "git branch -m" if that is what they
+ wanted to do.
Because the "--stdin" mode wants to operate on its refs in a
reference transaction, and the API function refs_rename_ref() does
@@ Documentation/git-update-ref.adoc: SYNOPSIS
--------
[synopsis]
git update-ref [-m <reason>] [--no-deref] -d <ref> [<old-oid>]
-+git update-ref [-m <reason>] [--no-deref] --rename <old-refname> <new-refname>
++git update-ref [-m <reason>] --rename <old-refname> <new-refname>
git update-ref [-m <reason>] [--no-deref] [--create-reflog] <ref> <new-oid> [<old-oid>]
git update-ref [-m <reason>] [--no-deref] --stdin [-z] [--batch-updates]
@@ Documentation/git-update-ref.adoc: the result of following the symbolic pointers
+With `--rename`, it renames <old-refname> together with its reflog to
+<new-refname>. The command fails if <old-refname> does not exist, or
-+if <new-refname> already exists. Because `git update-ref` does not
-+update active worktree `HEAD` symbolic references or `.git/config`
-+tracking settings when you rename a local branch in the `refs/heads/`
-+hierarchy, think twice before using this command to rename a local
-+branch (use `git branch -m` instead).
++if <new-refname> already exists. The command does not update any
++symbolic references pointing to the renamed reference, and neither
++does it update `.git/config` tracking settings. It is thus not
++recommended to use it for renaming local branches. Use `git branch -m`
++instead.
+
With `--stdin`, update-ref reads instructions from standard input and
performs all modifications together. Specify commands of the form:
--
2.55.0-rc0-119-ga57a595f62
^ permalink raw reply related
* Re: [PATCH v2] log: improve --follow following renames for non-linear history
From: Junio C Hamano @ 2026-06-11 22:32 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, git
In-Reply-To: <aipTOsH8LKTSwglj@collabora.com>
Miklos Vajna <vmiklos@collabora.com> writes:
> situation when determining what path to follow for a specific commit
> with multiple previously visited children.
> ---
Missing sign-off; omitting sign-off to say that this is primarily
for requesting comments and not ready for application (often we see
RFC on the Subject line when this is done) is fine, though.
>> Can a "map" cut it?
>>
>> If a history forked at commit A, with two children commit B and
>> commit C, and you started traversing the history from a much later
>> descendant M that merges these two lines of history (i.e., M^1
>> contains B, M^2 contains C, and A==B^1==C^1), while traversing down
>> from M to B you may find that you need to follow path1 and similarly
>> somewhere between M down to C the path you are following may be
>> path2. And the traversal meets at A. The slab records path1 for B
>> and path2 for C. Wouldn't you need to be able to store both path1
>> and path2 for commit A? What path do you need to pay attention to
>> when traversing past A to its ancestors?
>
> Indeed, I focused on merge commits and their parents and I did not
> consider that slab[A] may be set to path1 when visiting one parent and
> then slab[A] may be set to path2 when visiting an other parent -- even
> if "A" itself is just a plain commit with no renames and is not a merge.
"A" in my example is a fork point. One of A's children may arrive
at A following path1 while another child may come to A following
path2. IOW, in the history below:
B---X---o---M---o---Z
/ /
A---C---Y---o
* A has the original path at "path0"; so do B and C.
* X renames "path0" to "path1"
* Y renames "path0" to "path2"
* M merges path1 coming from upper and path2 from lower history
and records the result at path "path".
* Z has "path".
You run "git log --follow Z -- path".
My answer to my (rhetorical) question (Can a "map" cut it?) actually
was "we probably can", since our "rename following" code does not
handle cases where two paths in a parent is merged into a single
path in a child, or a single path in a parent is split to form
multiple paths in a child.
So the "what path are we following?" slab would need to keep track
of a single path. From Z down to M, we follow "path". "path1" is
followed from M to X and "path2" is followed from M to Y. And from
X to A, and Y to A, we follow "path0". IOW, I did not think we need
two paths recorded for one commit.
Are any of your test cases added by this patch behave differently
with this version (vs the "single path assigned to each commit"
version you had earlier)? If so, then obviously there is some hole
in my above discussion.
One case that _could_ break down is if a rename on one track (say,
at Y) is so huge that it is not recognised as a rename. Then from Y
down to A we would probably try to track "path2" (because we fail to
notice that "path2" came from "path0") and declare that "path2"
appeared at Y from nowhere. But even then, we shouldn't propagate
"path2" down to A, so A would get only "path0" which was what we
follow going from X down to A, I think. Still no need for following
multiple paths at a fork point.
> + /* Any recorded paths for this commit? If so, restore it */
> + if (opt->diffopt.flags.follow_renames) {
> + paths = get_follow_pathspec_at(opt, commit);
> + if (!paths->nr) {
> + const char *path = pathspec_single_path(&opt->diffopt.pathspec);
> + if (path)
> + string_list_insert(paths, path);
We do not need to worry about deduplicating, as string_list_insert()
will automatically takes care of that for us, which is nice.
> + }
> + set_pathspec_to_paths(&opt->diffopt.pathspec, paths);
> + if (paths->nr > 1) {
> + /* diff_check_follow_pathspec() doesn't handle multiple paths */
> + saved_follow_renames = opt->diffopt.flags.follow_renames;
> + opt->diffopt.flags.follow_renames = 0;
> + }
> + }
Eek. That's a subtle workaround to break the built-in safety to
ensure there is only one pathspec element while following.
^ permalink raw reply
* Re: [RFC PATCH] MyFirstContribution: mention trimming quoted text in replies
From: Junio C Hamano @ 2026-06-11 23:48 UTC (permalink / raw)
To: Weijie Yuan; +Cc: git
In-Reply-To: <080402ff0ac8127b654dccea59a1bf643df62a5c.1781186476.git.wy@wyuan.org>
Weijie Yuan <wy@wyuan.org> writes:
> ReviewingGuidelines already advises reviewers to trim irrelevant quoted
> context when replying. Give the same advice to new contributors in
> MyFirstContribution, so our documentation is consistent about mailing
> list reply etiquette.
>
> Signed-off-by: Weijie Yuan <wy@wyuan.org>
Makes sense.
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index 607876f3d8..0e2a9313ce 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -1453,6 +1453,11 @@ effect which had not occurred to you. It is always okay to ask for clarification
> if you aren't sure why a change was suggested, or what the reviewer is asking
> you to do.
>
> +When replying to review comments, quote only the parts of the message that are
> +relevant to your response. It is usually helpful to trim away unrelated context,
> +such as large portions of the patch that are not being discussed, while keeping
> +enough quoted text for readers to understand what you are responding to.
> +
> Make sure your email client has a plaintext email mode and it is turned on; the
> Git list rejects HTML email. Please also follow the mailing list etiquette
> outlined in the
The insertion point is well chosen, immediately following the
discussion on how to handle review comments and before the technical
details of email client configuration. The text itself is clear and
gives sound advice.
Will queue and wait for others to weigh in. Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/2] commit-reach: handle cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 2:40 UTC (permalink / raw)
To: Jeff King
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <20260611072942.GG2191159@coredump.intra.peff.net>
On Thu, Jun 11, 2026 at 12:29 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jun 08, 2026 at 07:36:34PM -0700, Tamir Duberstein wrote:
>
> > @@ -744,7 +745,7 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
> > }
> >
> > static enum contains_result contains_tag_algo(struct commit *candidate,
> > - const struct commit_list *want,
> > + struct commit_list *want,
> > struct contains_cache *cache)
>
> OK, we must lose the const here because repo_is_descendant_of() does not
> have it. We could add const to that function, though that cascades down
> to a few other helpers (see below). I'm not sure if that is making the
> world a better place, or if it is just const pedantry.
I left the signature change local rather than propagating const
through the other reachability helpers.
>
> diff --git a/commit-reach.c b/commit-reach.c
> index 5df471a313..8cede01f01 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -563,7 +563,7 @@ int repo_get_merge_bases(struct repository *r,
> */
> int repo_is_descendant_of(struct repository *r,
> struct commit *commit,
> - struct commit_list *with_commit)
> + const struct commit_list *with_commit)
> {
> if (!with_commit)
> return 1;
> @@ -955,11 +955,12 @@ int can_all_from_reach_with_flag(struct object_array *from,
> return result;
> }
>
> -int can_all_from_reach(struct commit_list *from, struct commit_list *to,
> +int can_all_from_reach(const struct commit_list *from,
> + const struct commit_list *to,
> int cutoff_by_min_date)
> {
> struct object_array from_objs = OBJECT_ARRAY_INIT;
> - struct commit_list *from_iter = from, *to_iter = to;
> + const struct commit_list *from_iter = from, *to_iter = to;
> int result;
> timestamp_t min_commit_date = cutoff_by_min_date ? from->item->date : 0;
> timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
> diff --git a/commit-reach.h b/commit-reach.h
> index 3f3a563d8a..76e82f827e 100644
> --- a/commit-reach.h
> +++ b/commit-reach.h
> @@ -37,7 +37,7 @@ int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result)
>
> int repo_is_descendant_of(struct repository *r,
> struct commit *commit,
> - struct commit_list *with_commit);
> + const struct commit_list *with_commit);
> int repo_in_merge_bases(struct repository *r,
> struct commit *commit,
> struct commit *reference);
> @@ -93,7 +93,8 @@ int can_all_from_reach_with_flag(struct object_array *from,
> unsigned int assign_flag,
> timestamp_t min_commit_date,
> timestamp_t min_generation);
> -int can_all_from_reach(struct commit_list *from, struct commit_list *to,
> +int can_all_from_reach(const struct commit_list *from,
> + const struct commit_list *to,
> int commit_date_cutoff);
>
>
> > +cycle:
> > + free(contains_stack.contains_stack);
> > + clear_contains_cache(cache);
> > + init_contains_cache(cache);
> > +
> > + result = repo_is_descendant_of(the_repository, candidate, want);
> > + if (result < 0)
> > + exit(128);
>
> We are feeding the whole initial "want" list, so we should get a correct
> answer regardless of how far we got into the cycle, which would run into
> problems (e.g., if the cycle existed only on some branch of the
> history). But going back to the initial list will always be correct.
> Good.
>
> Two small points, though.
>
> One, the call to init_contains_cache() is redundant here; the clear
> function is documented as making things ready for use (it's a little
> hard to grep for, due to macros, but the docs are in commit-slab.h).
> It's probably not hurting anything.
>
> Two, the call to exit(128) is unusual for our code base (I'd guess it
> was cribbed off of the top-level exits in builtin/pull.c). We'd usually
> die() instead. Even if repo_is_descendant_of() produced its own error
> message, it may be useful to mention that we were falling back to it due
> to a cycle.
I removed the redundant initialization and replaced exit(128) with
die(), adding context that the failure occurred after detecting a
cycle.
>
> But even better is if we can return the error up the stack. We do not
> return errors from contains_tag_algo() currently, but it has only one
> caller. And that caller may also directly return the result of
> repo_is_descendant_of(). So could we just pass that along?
>
> Perhaps not. Looking at the callers of commit_contains(), they treat the
> result as a pure boolean. So probably calling die() is reasonable, and
> we already do so via parse_commit_or_die() elsewhere in the algorithm.
> That does leave a potential lurking bug for the non-tag-algo code path.
I traced the callers. Returning an error from commit_contains() would
only move the fatal check into apply_ref_filter(): its NULL return
already means “filtered out”, filter_and_format_refs() returns void, and
the branch caller ignores filter_refs()'s return value. Propagating the
error to the command would require changing that whole chain, and none
of the commands can recover from an unreadable commit.
The series therefore makes both cases fail explicitly. Patch 1 calls
die() if the cycle fallback cannot read the ancestry. Patch 3 calls
die() when the ordinary non-memoized walk returns -1.
>
> > + *contains_cache_at(cache, candidate) =
> > + result ? CONTAINS_YES : CONTAINS_NO;
> > + return result ? CONTAINS_YES : CONTAINS_NO;
>
> So we actually cache our discovered value. Cute, and it might save us
> from hitting the cycle again, though not always. E.g., two candidates A
> and B share a parent P, and the cycle starts at P but does not include A
> or B. We discover the cycle and cache the value for A, but discover it
> again for B.
>
> We do lose all of the existing non-cycle cached values when we call
> clear_contains_cache(). But we have to at least clear out all of the
> IN_PROGRESS commits. It is hard to care too much about optimizing the
> outcome for this case which we expect to happen approximately never.
> So I think doing the simplest correct thing is OK.
>
> > +test_expect_success 'tag --contains handles cyclic replacement histories' '
> > + first=$(git rev-parse HEAD~2) &&
> > + second=$(git rev-parse HEAD~) &&
> > + third=$(git rev-parse HEAD) &&
> > + test_when_finished "
> > + git replace -d $first
> > + git replace -d $third
> > + git tag -d cycle-a cycle-b
> > + " &&
>
> We usually &&-chain the commands inside test_when_finished. If they
> fail, the test harness will note this and complain (if the test was not
> otherwise failing). It's usually not a big deal either way, though
> sometimes it can catch silly mistakes (e.g., if you wrote $second
> instead of $third and the "replace -d" is quietly doing nothing at all).
Fixed in v3.
>
> I'm a little surprised that the chainlint checker doesn't catch this,
> but I guess it doesn't know to recurse into the snippet handed to
> test_when_finished. It probably is not really worth the trouble to teach
> it to do so.
>
> Otherwise the test looks good to me.
>
> -Peff
^ permalink raw reply
* Re: [PATCH v2 2/2] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-12 2:40 UTC (permalink / raw)
To: Jeff King
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <20260611082244.GH2191159@coredump.intra.peff.net>
On Thu, Jun 11, 2026 at 1:22 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jun 08, 2026 at 07:36:35PM -0700, Tamir Duberstein wrote:
>
> > The wall-time standard deviations were 11.356 seconds and 133.8
> > milliseconds, respectively. Separate runs without redirection produced
> > the same output with SHA-256
> > 2466f6e2b72aa16b1a2126eddb81c8a1b2764ee251204ac034c191a925aa896f.
>
> Heh. Without the original repo, this sha256 hash is meaningless to us,
> isn't it? Ditto for the sha1 the earlier command.
Yeah, AI slop. Removed.
>
> > int commit_contains(struct ref_filter *filter, struct commit *commit,
> > struct commit_list *list, struct contains_cache *cache)
> > {
> > - if (filter->with_commit_tag_algo)
> > + int result;
> > +
> > + if (!list)
> > + return 1;
> > + if (filter->with_commit_tag_algo ||
> > + generation_numbers_enabled(the_repository))
> > return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
> > - return repo_is_descendant_of(the_repository, commit, list);
> > +
> > + result = repo_is_descendant_of(the_repository, commit, list);
> > + if (result < 0)
> > + exit(128);
> > + return result;
>
> There's a little more going on here than I expected from the commit
> message. Is it important for us to short-circuit the empty list and just
> return 1? Or did the existing helper functions already handle that?
>
> Looking at contains_tag_algo(), I think it would actually return
> CONTAINS_NO here (though I didn't test it). So this is actually a change
> in behavior for "git tag" if that's correct. I doubt it is triggerable
> in practice, though, as we would simply never call commit_contains() in
> the first place with an empty list. But if we are going to add in this
> logic, I think it makes sense to do so as a separate commit (describing
> what it is doing and why it's not (yet) a triggerable bug).
>
> Checking the result of repo_is_descendant_of() makes sense, as discussed
> earlier. But probably that should come as its own patch, since it's an
> independent bug-fix. I'm also tempted to say it should call die()
> instead of a direct exit, though it does look like the error exit paths
> from repo_is_descendant_of() would all have produced their own messages.
I dropped the empty-list change. The error check is now a separate
patch and uses die().
>
>
> And one side note. While looking at the implementation of
> repo_is_descendant_of(), I did notice something curious: it also
> switches algorithms based on the presence of generation numbers! So it
> should also be cutting off the traversal early when possible. But I
> guess its main problem is that we call it independently for each
> candidate, so it may traverse the same (useful) stretch of history
> multiple times.
>
> So probably an alternative approach to this patch would be feeding all
> of the candidates at once, the way we do with reach_filter() via
> filter_refs(). I'm not sure if we have the right functions available for
> that (naively, --contains and --merged are inversions of each other, so
> swapping the arguments to tips_reachable_from_bases() might work, but I
> didn't think very hard on it).
I tried the suggested argument swap, but tips_reachable_from_bases()
only reports whether a tip is reachable from any base. It cannot report
which candidate refs contain a target, which is what --contains needs.
I did not find an existing batched reachability helper that returns
those per-candidate answers.
>
> I wonder if that might perform better or worse. I'm content to leave it
> for another day, though, as switching to the memoizing depth-first algo
> here is a pretty easy change.
>
> > - commit=$(git commit-tree $(git rev-parse HEAD^{tree})) &&
> > + git rev-list --first-parent --max-count=8192 HEAD >contains-commits &&
> > + test_file_not_empty contains-commits &&
> > + git update-ref refs/contains-perf-base "$(tail -n 1 contains-commits)" &&
> > + awk "{
> > + printf \"update refs/contains-perf/%04d %s\\n\", NR, \$1
> > + }" contains-commits |
> > + git update-ref --stdin &&
> > + git pack-refs --include "refs/contains-perf/*" &&
>
> My head almost exploded reading the embedded quoting in that awk
> invocation. But I can't think offhand of a better way to do it. You
> can't use test_seq because it needs both the number and the original
> string. You can do it with sed, but it probably ends up even more
> unreadable.
>
> But OK, we are making a bunch of refs based on first-parent history.
>
> > + tree=$(git rev-parse HEAD^{tree}) &&
> > + base=$(git rev-parse HEAD) &&
> > + target=$(echo target | git commit-tree "$tree" -p "$base") &&
> > + git update-ref refs/contains-diverged/target "$target" &&
> > + for i in $(test_seq 1 4)
> > + do
> > + commit=$(echo candidate-$i |
> > + git commit-tree "$tree" -p "$base") &&
> > + git update-ref refs/contains-diverged/candidate-$i "$commit" ||
> > + return 1
> > + done &&
>
> And then a few candidate refs that are not reachable from other refs, or
> from each other. OK.
>
> I think you could just write:
>
> git commit-tree HEAD^{tree} -p HEAD
>
> instead of doing separate rev-parses, but it's probably not a big deal
> either way.
>
> > +test_expect_success 'verify contains results' '
> > + git for-each-ref --contains=refs/contains-perf-base \
> > + refs/contains-perf/ >actual &&
> > + test_line_count = $(wc -l <contains-commits) actual &&
> > +
> > + echo refs/contains-diverged/target >expect &&
> > + GIT_TEST_COMMIT_GRAPH=0 \
> > + git -c core.commitGraph=false for-each-ref \
> > + --format="%(refname)" \
> > + --contains=refs/contains-diverged/target \
> > + refs/contains-diverged/ >actual &&
> > + test_cmp expect actual
> > +'
>
> This is a funny test to have in the middle of a perf script (which
> hardly anybody ever runs). If we are concerned about the correctness,
> should this be in a non-perf test script? Though I'd imagine something
> like it is already covered there.
I deleted that block rather than moving it. It only rechecked ordinary
--contains semantics already covered by t3201, t6302, and t7004; with
GIT_TEST_COMMIT_GRAPH=1, those tests exercise the newly selected
memoized path for branch and for-each-ref.
The series adds functional tests for the behavior that is actually new:
t7004 covers cyclic replacement histories, and t6301 covers unreadable
ancestry. The p1500 additions now measure performance only.
>
> There's a lot of subtlety in what we're verifying, too. In the first
> half, we are checking that all of the commits in contains-perf contain
> the base. And that base is the final element of the contains-commits
> list. Which made me wonder what happens in a branch history, since that
> list is linearized. But because we used --first-parent to generate it,
> it _is_ linear, and the results work out. So OK, I don't think it's
> wrong, but I am struggling to understand the meaning of the test.
>
> The second half is just checking that...the other refs which are not
> contained in "target" are not mentioned? OK, but why do it only with
> commit graphs off. Why not both off and on? Again, I'm not sure I
> understand what we're trying to focus on here.
>
> > +test_perf 'contains: git for-each-ref --contains' '
> > + git for-each-ref --contains=refs/contains-perf-base \
> > + refs/contains-perf/ >/dev/null
> > +'
>
> Yay, actual perf tests. Here we have a ton of matches, and they all walk
> over the same chunk of history. Should get much faster, though it's
> mostly a synthetic test.
>
> For --merged, we already have separate tests with each of for-each-ref,
> branch, and tag. Should we have the same here for --contains? And should
> we be using the input repo data, rather than our synthetic test? It is
> nice to show off the performance with the synthetic test, but ultimately
> the point of the perf suite is feeding it real workloads and looking for
> regressions.
I added p1500 cases for all three frontends using refs from the input
repository, while retaining the synthetic shared-history case.
>
> > +test_perf 'contains without generations: divergent refs' '
> > + GIT_TEST_COMMIT_GRAPH=0 \
> > + git -c core.commitGraph=false for-each-ref \
> > + --contains=refs/contains-diverged/target \
> > + refs/contains-diverged/ >/dev/null
> > +'
>
> OK, and this one should find that most of them are not contained, but
> the depth-first algorithm could walk all the way down to the roots. But
> we don't run it at all, since we disable commit graphs!
>
> So what are we trying to measure here? If it left commit graphs enabled,
> I think we could demonstrate that using the depth-first algorithm with
> generation numbers does not make anything _worse_. I.e., that
> for-each-ref and branch did not regress from the change.
The divergent-ref test did not exercise the changed path, so I removed
it.
>
> > +test_expect_success 'missing ancestors are reported by contains filters' '
> > + test_when_finished "git update-ref -d refs/heads/missing-parent" &&
> > + {
> > + echo "tree $(git rev-parse HEAD^{tree})" &&
> > + echo "parent $MISSING" &&
> > + git cat-file commit HEAD |
> > + sed -n -e "/^author /p" -e "/^committer /p" &&
> > + echo &&
> > + echo "missing parent"
> > + } >commit &&
> > + broken=$(git hash-object -t commit -w commit) &&
> > + git update-ref refs/heads/missing-parent "$broken" &&
> > + for option in --contains --no-contains
> > + do
> > + test_must_fail git for-each-ref "$option=HEAD" \
> > + refs/heads/missing-parent >out 2>err &&
> > + test_must_be_empty out &&
> > + test_grep "parse commit $MISSING" err ||
> > + return 1
> > + done
> > +'
>
> This is a great thing to test, but probably should be pulled out into
> a separate patch along with the fix to check the return code.
Done in v3.
>
> The commit construction looks OK, and is nicer than corrupting the
> repository by deleting a real object. Given that we are pulling the
> idents from an existing commit, it might be simpler to just use the
> whole commit as a template, like:
>
> git cat-file commit HEAD |
> sed "s/^parent /parent $MISSING/"
>
> but it may be a matter of taste.
>
> -Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox