* Re: [PATCH] http: handle absolute-path alternates from server root
From: Patrick Steinhardt @ 2026-05-15 7:41 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, slonkazoid
In-Reply-To: <20260513185825.GB147423@coredump.intra.peff.net>
On Wed, May 13, 2026 at 02:58:25PM -0400, Jeff King wrote:
> On Wed, May 13, 2026 at 10:10:54AM +0900, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > ... Probably in a way that makes it totally invalid, but
> > > if you were very unlucky you could turn something like:
> > >
> > > http://victim.com.evil.domain:8000
> > >
> > > into:
> > >
> > > http://victim.com
> > >
> > > Which looks like the start of a redirect attack, except that
> > > the attacker could just have written "http://victim.com" in
> > > the first place! Either way we feed it to
> > > is_alternate_allowed(), which is where we check redirect and
> > > protocol rules.
> >
> > Yuck. I know I am the guilty party who introduced the dumb HTTP
> > walker but I wish we could kill it off after all these years. I did
> > not even recall that we supported the alternate object store in the
> > "protocol" until I saw this patch X-<.
>
> Me too. It's been the source of many obscure bugs, and I think a couple
> of vulnerabilities (even though clients never intend to use dumb clones
> in the first place).
>
> We talked about dropping it a few years ago, but Eric countered that
> dumb clones are easier on the server in some cases (like gigantic
> public-inbox repos that are packed to keep most of the old history in
> one big pack that is never updated). The verbatim pack-reuse feature
> tries to get smart clones closer to that, but it's hard to beat serving
> a static file from the server's perspective. I haven't measured anything
> in that area in a while, though.
In theory we can get much closer with packfile URIs, too, can't we? If
the packfiles are directly accessible anyway the server could just
announce these directly and have the client fetch them. That should
significantly reduce the load on the server even further.
Of course, the big downside is that "fetch.uriProtocols" is empty by
default, so Git will not use them. Makes me wonder whether this is
something we want to eventually change, but I guess the current default
behaviour is somewhat insecure as it would allow the server to redirect
clients to arbitrary locations. It would be great if we had a mechanism
that only allowed packfile URIs that use the same host, which would make
this a lot more reasonable to enable by default.
Patrick
^ permalink raw reply
* Re: [PATCH] revision: use priority queue in limit_list()
From: Kristofer Karlsson @ 2026-05-15 7:47 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Derrick Stolee
Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <20260515041641.GA81292@coredump.intra.peff.net>
Thanks for the reviews!
**Junio C Hamano**:
Good question about A..B. Since 1b4d8827 (revision: use
generation for A..B --topo-order queries, 2019-05-21), a plain `A..B`
with commit-graph avoids limit_list() entirely via init_topo_walk().
The commands I listed are those that still force `revs->limited = 1`
even with a commit-graph.
As you suggested, I ran benchmarks without commit-graph. On the same
2.3M-commit repo with `core.commitGraph=false`:
git rev-list --left-right --count HEAD~100...HEAD (3,751 sym-diff)
baseline (no commit-graph): 67.0s
patched (no commit-graph): 43.1s (1.6x speedup)
baseline (with commit-graph): 21.2s
patched (with commit-graph): 8.5s (2.5x speedup)
The gain is smaller without commit-graph because more time goes to
parsing commits from pack, but it's still a meaningful improvement.
**Derrick Stolee**:
Unfortunately git.git's mostly-linear history doesn't
trigger the quadratic behavior (the queue stays narrow). Even with
5,584 commits in the symmetric diff, `--left-right --count` finishes
in ~0.4s on git.git for both baseline and patched. A 50-pair
interleaved run shows no statistically significant difference:
git rev-list --left-right --count v2.47.1...v2.54.0 (git.git, 5,584 commits)
50 interleaved paired runs:
baseline: mean 393ms, stdev 13ms, median 392ms
patched: mean 396ms, stdev 14ms, median 393ms
paired t-test: +2.9ms, t=1.16, p>0.05 (not significant)
There may be a tiny constant-factor overhead (~1%) from the heap's
bookkeeping on narrow queues (sift-up/sift-down vs simple pointer
splice), but it's well within noise and dwarfed by the 2.5-3x win
on wide queues.
The improvement is specific to merge-heavy DAGs where the active
frontier (queue width) grows large.
I also measured `--ancestry-path`, which hits the same limit_list()
bottleneck. 74% of CPU was in commit_list_insert_by_date():
git log --oneline --ancestry-path HEAD~100..HEAD (monorepo, 100 results)
baseline: 16.5s
patched: 3.8s (4.3x speedup)
You're right that `git log --graph` without commit-graph also goes
through limit_list(). I can add that to the description.
Regarding the O(N·w) analysis in the cover letter vs commit message:
I'll move the key points into the commit message in v2.
The existing t/perf tests don't cover this path. p0001 doesn't
use --left-right and p6010 is merge-base specific. I could add a
perf test, though it would need a merge-heavy test repo to show the
difference. Would a synthetic one (like p6010 does) be useful?
**Jeff King**
Confirmed: unsorted_input is only set alongside no_walk, and
limit_list() is called after the no_walk early return.
So the incoming list is always date-sorted when limit_list() runs.
That said, even if unsorted input did reach this code, the prio_queue
maintains its sorted invariant on every prio_queue_put(), so the
output order would still be correct; the heap sorts by commit date
regardless of insertion order.
Your patch to convert revs.commits to a prio_queue sounds like a
natural next step; this change would indeed slot right in (the
initial drain-and-fill loop would just disappear).
On Fri, 15 May 2026 at 06:16, Jeff King <peff@peff.net> wrote:
>
> On Thu, May 14, 2026 at 04:51:31PM +0000, Kristofer Karlsson via GitGitGadget wrote:
>
> > @@ -1451,6 +1447,7 @@ static int limit_list(struct rev_info *revs)
> > struct commit_list *newlist = NULL;
> > struct commit_list **p = &newlist;
> > struct commit *interesting_cache = NULL;
> > + struct prio_queue queue = { .compare = compare_commits_by_commit_date };
> >
> > if (revs->ancestry_path_implicit_bottoms) {
> > collect_bottom_commits(original_list,
> > @@ -1461,6 +1458,11 @@ static int limit_list(struct rev_info *revs)
> >
> > while (original_list) {
> > struct commit *commit = pop_commit(&original_list);
> > + prio_queue_put(&queue, commit);
> > + }
> > +
> > + while (queue.nr) {
> > + struct commit *commit = prio_queue_get(&queue);
>
> Here we push the whole starting list into the prio-queue, which will let
> us pull the commits out in date order. But is the incoming list always
> in date order?
>
> If revs->unsorted_input, then we don't sort the initial list. So we'd
> now see the commits in a different order, and put them onto newlist in
> that different order.
>
> I _think_ it may not matter because we don't call limit_list() when
> revs->no_walk is set, and we only have revs->unsorted_input when no_walk
> is also set. If that wasn't true, it would get weird when limit_list()
> calls process_parents(), which uses commit_list_insert_by_date().
>
>
> I was on the lookout for this issue particularly because I have another
> patch which converts revs.commits to a prio_queue totally. And I
> remember running into issues (and the solution is that sometimes the
> prio_queue has a NULL comparator and acts like a LIFO queue). But if my
> analysis is right above, we can ignore that for now. And if we
> eventually move to revs.commits as a prio_queue, then it will just slot
> in nicely here (we can drop the queue generation step and just use it
> directly).
>
> The rest of the patch looks as I'd expect from what my other patch does.
>
> -Peff
^ permalink raw reply
* Email issues
From: Harald Nordgren @ 2026-05-15 7:56 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren
In-Reply-To: <xmqqecjdea13.fsf@gitster.g>
> Why do I get the above, which apparently is a response to my review
> for
>
> [PATCH] config: suggest the correct form when key contains "="
>
> under this thread? Am I dealing with some sort of mechanical slop?
I think the problem here is my email sending process is not good. I edit
all the emails in Sublime text, where I keep the same file for all
different threads.
I have the subject line as the first line of the file and like you notice I
forget to change it sometimes.
I keep each of the topics bookmarked like this,
https://lore.kernel.org/git/xmqqecjdea13.fsf@gitster.g/, and then utilize
that like to send the email
```
git send-email \
--in-reply-to=xmqqecjdea13.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=haraldnordgren@gmail.com \
/path/to/YOUR_REPLY
```
I tried playing with neomutt and and email client replacement, but that
adds the complexity of downloading a new mbox file for each reply, it
didn't seem easier, but maybe it is.
How do you handle emails?
Harald
^ permalink raw reply
* Re: [PATCH v4 0/3] Avoid hardcoded "good"/"bad" bisect terms
From: Jonas Rebmann @ 2026-05-15 8:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Chris Down, Jeff King, Phillip Wood
In-Reply-To: <xmqqv7cpepec.fsf@gitster.g>
On 14/05/2026 21.56, Junio C Hamano wrote:
> Please make sure that your cover letter of the (i+1)th iteration
> [v(i+1) 0/N] is a reply to the cover letter of the i-th iteration
> [v(i) 0/M]. With that, anybody who has the i-th iteration can
> [...]
Thanks, I never knew! I've now set
git config b4.send-same-thread yes
so b4 will send future rerolls In-Reply-To the cover of the
previous version.
Regards,
Jonas
^ permalink raw reply
* Re: [PATCH v7 1/3] git-gui: restructure repository startup
From: Johannes Sixt @ 2026-05-15 8:26 UTC (permalink / raw)
To: Shroom Moo, git; +Cc: Mark Levedahl, Aina Boot
In-Reply-To: <tencent_2D0A6C14E8B34348B6F236BC8E7B66AB5105@qq.com>
Am 09.05.26 um 15:37 schrieb Shroom Moo:
> When git-gui is started inside a .git directory of a non-bare
> repository, it should treat the parent directory as the worktree,
> as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
> without a worktree an error, 2019-11-19). However, a bare repository
> or a separated gitdir without a worktree must be rejected early.
> > Protect the previously unguarded calls to `git rev-parse
> --show-object-format` and `--show-toplevel`. Restructure the startup
> sequence to:
>
> - Check for a bare repository right after loading the config. If the
> repository is bare and the current subcommand does not allow bare
> repos (e.g. normal commit mode), show "Cannot use bare repository"
> and exit.
>
> - When `rev-parse --show-toplevel` fails and the repository is
> non-bare, the gitdir path ends with ".git", and we are inside that
> gitdir, use the parent directory as the worktree. This preserves
> the ability to start git-gui from within a regular repository’s
> .git directory, which was intentionally supported since 87cd09f43e56
> (git-gui: work from the .git dir, 2010-01-23).
>
> - Otherwise, show a descriptive error and exit.
>
> - Wrap `rev-parse --show-object-format` in a catch to avoid a crash
> when the repository configuration is broken (e.g. core.worktree
> pointing to an invalid path).
>
> Also removes the old `_prefix`‑based fallback that computed a relative
> path to the worktree top from a subdirectory, and the unconditional
> `[file dirname $_gitdir]` guess. Both are unnecessary now that
> `rev‑parse --show‑toplevel` directly provides the absolute top‑level
> path and we can `cd` to it. The guess is further unsafe in
> multi‑worktree setups, where a gitdir may have more than one worktree.
> The only remaining fallback is the explicit “.git directory” rule for
> non‑bare repositories, which mirrors the historical behaviour.
> Additionally, only export GIT_WORK_TREE when it is not empty, to avoid
> confusing commands in bare-repository subcommands.
>
> This fixes the fatal Tcl error when the working tree is missing, while
> keeping the .git startup feature and avoiding any automatic directory
> switching that could be dangerous in multi‑worktree setups.
I think that the end result is useful. However, frankly, the patch
attempts to do too many things at once and should still be split further:
- The removal of the cdup fallback could be a preliminary patch.
- The protection of --show-object-format could be a follow-up patch.
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> Helped-by: Mark Levedahl <mlevedahl@gmail.com>
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
> git-gui/git-gui.sh | 76 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 49 insertions(+), 27 deletions(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..9eb93a76b5 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1129,7 +1129,8 @@ if {[catch {
> }]
> && [catch {
> # beware that from the .git dir this sets _gitdir to .
> - # and _prefix to the empty string
> + # and _prefix to the empty string; this is handled by
> + # the startup safety checks below
> set _gitdir [git rev-parse --git-dir]
> set _prefix [git rev-parse --show-prefix]
> } err]} {
> @@ -1142,8 +1143,20 @@ if {[catch {
> set picked 1
> }
>
> +if {![file isdirectory $_gitdir]} {
> + catch {wm withdraw .}
> + error_popup [strcat
> + [mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
> + [mc "Please ensure GIT_DIR points to a valid Git repository"]]
> + exit 1
> +}
> +
This was moved from below. I would appreciated if there were no changes
in the moved code so that `git diff --color-moved` can show that no
changes were intended. I am not sure that the additional sentence that
mentions GIT_DIR is warranted. If you feel it is needed, please add it
in a separate patch with a justification.
> # Use object format as hash algorithm (either "sha1" or "sha256")
> -set hashalgorithm [git rev-parse --show-object-format]
> +if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
> + exit 1
> +}
> if {$hashalgorithm eq "sha1"} {
> set hashlength 40
> } elseif {$hashalgorithm eq "sha256"} {
> @@ -1160,46 +1173,52 @@ if {$_gitdir eq "."} {
> set _gitdir [pwd]
> }
>
> -if {![file isdirectory $_gitdir]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
> - exit 1
> -}
> # _gitdir exists, so try loading the config
> load_config 0
> apply_config
>
> -set _gitworktree [git rev-parse --show-toplevel]
> -
> -if {$_prefix ne {}} {
> - if {$_gitworktree eq {}} {
> - regsub -all {[^/]+/} $_prefix ../ cdup
> - } else {
> - set cdup $_gitworktree
> - }
> - if {[catch {cd $cdup} err]} {
> +# Handle bare repository and determine working tree
> +if {[is_bare]} {
> + # Bare repository: only allowed for certain subcommands
> + if {![is_enabled bare]} {
> catch {wm withdraw .}
> - error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
> + error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
> exit 1
> }
> - set _gitworktree [pwd]
> - unset cdup
> -} elseif {![is_enabled bare]} {
> - if {[is_bare]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
> - exit 1
> + # Allowed bare repo does not have a worktree
> + set _gitworktree {}
> +} else {
> + # Non-bare repository: we must find a worktree
> + if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
> + # The only acceptable failure is when we are inside
> + # the .git directory of a regular repository.
> + set inside_gitdir 0
> + catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
> + if {$inside_gitdir eq {true} && [file tail $_gitdir] eq {.git}} {
> + # Use the parent directory as worktree (historic behavior)
> + set _gitworktree [file normalize [file dirname $_gitdir]]
> + } else {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
> + exit 1
> + }
> }
> +
> if {$_gitworktree eq {}} {
> - set _gitworktree [file dirname $_gitdir]
> + catch {wm withdraw .}
> + error_popup [mc "Cannot determine working tree (unexpected empty result)"]
> + exit 1
> }
An empty $_gitworktree should be practically impossible at this point.
Personally, I would let the following "cd" handle the case (it fails if
the argument is empty).
> +
> if {[catch {cd $_gitworktree} err]} {
> catch {wm withdraw .}
> - error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
> + error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
> exit 1
> }
> set _gitworktree [pwd]
> }
> +
> +# Derive a human-readable repository name
> set _reponame [file split [file normalize $_gitdir]]
> if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end-1]
> @@ -1207,8 +1226,11 @@ if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end]
> }
>
> +# Export the final paths
> set env(GIT_DIR) $_gitdir
> -set env(GIT_WORK_TREE) $_gitworktree
> +if {$_gitworktree ne {}} {
> + set env(GIT_WORK_TREE) $_gitworktree
> +}
>
> ######################################################################
> ##
-- Hannes
^ permalink raw reply
* Re: [PATCH v7 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early
From: Johannes Sixt @ 2026-05-15 8:28 UTC (permalink / raw)
To: Shroom Moo, git; +Cc: Mark Levedahl, Aina Boot
In-Reply-To: <tencent_C4AD92361C8D7B76EB4C8A6F14EA33496805@qq.com>
Am 09.05.26 um 15:37 schrieb Shroom Moo:
> Users expect these two invocations to be equivalent:
>
> GIT_WORK_TREE=/some/path GIT_DIR=/some/path/.git git gui
> git -C /some/path gui
>
> Currently, the environment variable variant often brings up the
> repository picker or ignores the requested worktree because
> GIT_WORK_TREE is processed too late.
I cannot reproduce the case that brings the repository picker. All other
failure cases that I can produce are reasonable and do not indicate that
GIT_WORK_TREE is processed too late.
> Moreover, after determining
> the working tree, git-gui unconditionally exports GIT_WORK_TREE.
> When no worktree is found (e.g., in a bare repository with a
> read-only subcommand like blame), an empty value is exported, which
> confuses commands like `git branch --show-current`.
True. I think the culprit is that we export GIT_WORK_TREE in the first
place.
I suggest the following patch to replace this and the previous patch.
---- 8< ----
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] git-gui: operate git commands without GIT_WORK_TREE
The manual page of the git command states about the --git-dir option:
Specifying the location of the ".git" directory using this option
(or GIT_DIR environment variable) turns off the repository
discovery [...], and tells Git that you are at the top level of
the working tree.
Use this to our advantage:
- Set GIT_DIR in the environment to the value that was discovered, so
that the invoked git commands operate on the same repository
database that Git GUI uses even after it changes the working
directory.
- After changing the working directory to the top level of the working
tree, ensure that GIT_WORK_TREE is not set, because, as per
documentation, all git invocations from then on will assume that the
current working directory is also the top level working tree.
- Remove the now obsolete GIT_WORK_TREE dance when subordinate Gitk or
Git GUI are invoked for a submodule.
Do keep the state of GIT_WORK_TREE if we are in a bare repository,
because Git GUI is not interested in the worktree at all, as no commit
mode is possible in a bare repository.
This avoids cases where an empty GIT_WORK_TREE was exported into the
environment, most notably by a call of `git gui blame HEAD file` in a
bare repository.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
git-gui.sh | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 76560ec825cf..146a29a809a8 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1216,6 +1216,7 @@ if {[is_bare]} {
exit 1
}
set _gitworktree [pwd]
+ catch { unset env(GIT_WORK_TREE) }
}
# Derive a human-readable repository name
@@ -1228,9 +1229,6 @@ if {[lindex $_reponame end] eq {.git}} {
# Export the final paths
set env(GIT_DIR) $_gitdir
-if {$_gitworktree ne {}} {
- set env(GIT_WORK_TREE) $_gitworktree
-}
######################################################################
##
@@ -2029,7 +2027,7 @@ proc incr_font_size {font {amt 1}} {
proc do_gitk {revs {is_submodule false}} {
global current_diff_path file_states current_diff_side ui_index
- global _gitdir _gitworktree
+ global _gitdir
# -- Always start gitk through whatever we were loaded with. This
# lets us bypass using shell process on Windows systems.
@@ -2043,11 +2041,7 @@ proc do_gitk {revs {is_submodule false}} {
set pwd [pwd]
- if {!$is_submodule} {
- if {![is_bare]} {
- cd $_gitworktree
- }
- } else {
+ if {$is_submodule} {
cd $current_diff_path
if {$revs eq {--}} {
set s $file_states($current_diff_path)
@@ -2067,18 +2061,16 @@ proc do_gitk {revs {is_submodule false}} {
}
set revs $old_sha1...$new_sha1
}
- # GIT_DIR and GIT_WORK_TREE for the submodule are not the ones
- # we've been using for the main repository, so unset them.
+ # GIT_DIR for the submodule is not the one we've been using for
+ # the main repository, so unset it. (GIT_WORK_TREE is already unset.)
# TODO we could make life easier (start up faster?) for gitk
# by setting these to the appropriate values to allow gitk
# to skip the heuristics to find their proper value
unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
}
safe_exec_bg [concat $cmd $revs "--" "--"]
set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
cd $pwd
if {[info exists main_status]} {
@@ -2102,12 +2094,11 @@ proc do_git_gui {} {
error_popup [mc "Couldn't find git gui in PATH"]
} else {
global env
- global _gitdir _gitworktree
+ global _gitdir
- # see note in do_gitk about unsetting these vars when
+ # see note in do_gitk about unsetting this variable when
# running tools in a submodule
unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
set pwd [pwd]
cd $current_diff_path
@@ -2115,7 +2106,6 @@ proc do_git_gui {} {
safe_exec_bg [concat $exe gui]
set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
cd $pwd
set status_operation [$::main_status \
--
2.54.0.215.g4fe990ec16
^ permalink raw reply related
* [PATCH v3] generate-configlist: collapse depfile for older Ninja
From: Toon Claes @ 2026-05-15 8:42 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Patrick Steinhardt, Toon Claes
In-Reply-To: <20260422-toon-fix-almalinux8-v2-1-45d8471ed0e9@iotcl.com>
The tools/generate-configlist.sh script generates two files:
* config-list.h
* config-list.h.d
The former is included by the source code and the latter defines on
which files the former depends.
The contents of `config-list.h.d` consists of two sections:
config-list.h: Documentation/config.adoc
config-list.h: Documentation/git-config.adoc
config-list.h: Documentation/config/add.adoc
config-list.h: Documentation/config/advice.adoc
config-list.h: Documentation/config/alias.adoc
config-list.h: Documentation/config/am.adoc
config-list.h: Documentation/config/apply.adoc
...
This first section actually defines on which individual files
`config-list.h` depends and thus needs to be rebuild if one of those
changes.
And the second section contains content like:
Documentation/config.adoc:
Documentation/git-config.adoc:
Documentation/config/add.adoc:
Documentation/config/advice.adoc:
Documentation/config/alias.adoc:
Documentation/config/am.adoc:
Documentation/config/apply.adoc:
...
These rules exist to ensure Make won't fail with the following error if
one of the .adoc files is renamed or removed:
make: *** No rule to make target 'Documentation/config.adoc', needed by 'config-list.h'.
With the no-op targets defined in `config-list.h.d`, Make knows there's
no work to be done to generate these files, so it doesn't error out if
it doesn't exist.
For the Makefile build system this works great. And since
ebeea3c471 (build: regenerate config-list.h when Documentation changes,
2026-02-24) this script is also called from the Meson build system.
Nevertheless, on AlmaLinux 8 the following build failure is seen:
ninja: error: dependency cycle: config-list.h -> config-list.h
This version of this distro uses Ninja 1.8.2 and it seems to have some
issues with the format of the `config-list.h.d` file.
Ninja versions before 1.10.0 do not reset the depfile parser state on
newlines. This causes issues when the depfile has one dependency per
line, like we have in `config-list.h.d`:
config-list.h: Documentation/config.adoc
config-list.h: Documentation/config/add.adoc
The parser only recognizes the first "config-list.h:" as a target. On
subsequent lines it is still in dependency-parsing mode, so the repeated
output name is recorded as an input. This causes the error mentioned
above.
The bug in Ninja is fixed in 1.10, with commit
ninja-build/ninja@1daa7470ab7e (depfile_parser: remove restriction on
multiple outputs, 2019-11-20).
To be compatible with older versions of Ninja, collapse the dependencies
for `config-list.h` into a single line like:
config-list.h: Documentation/config.adoc Documentation/config/add.adoc ...
This works around the bug in older versions of Ninja, and is fully
compatible Make and with more recent versions of Ninja. And while the
no-op targets are not needed for Ninja, they also don't do any harm.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
At GitLab we build images for various distros, including AlmaLinux 8.
On this distro we got this error while compiling Git.
ninja: error: dependency cycle: config-list.h -> config-list.h
It seems this is caused by a bug in older versions of Ninja. There are
more details in the commit message, but here are a few simple steps to
reproduce:
docker run --rm -it -v $(pwd):/git -w /git almalinux:8 bash
dnf -yq install epel-release
dnf -yq install shadow-utils sudo make pkg-config gcc findutils \
diffutils perl python3 gawk gettext zlib-devel expat-devel \
openssl-devel curl-devel pcre2-devel cargo
pip3 install --prefix=/usr meson ninja==1.8.2
meson setup build --warnlevel 2 --werror
ninja -C build config-list.h
ninja -C build config-list.h # fails with dependency cycle
---
Changes in v3:
- Stop using \n in sed(1) replacement strings because it is not
portable.
- Link to v2: https://patch.msgid.link/20260422-toon-fix-almalinux8-v2-1-45d8471ed0e9@iotcl.com
Changes in v2:
- Simplify the changes *a lot* by doing the collapsing unconditionally.
- Link to v1: https://patch.msgid.link/20260421-toon-fix-almalinux8-v1-1-aec1d54addde@iotcl.com
---
tools/generate-configlist.sh | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
index e28054f9e0..d1d2ba4bb7 100755
--- a/tools/generate-configlist.sh
+++ b/tools/generate-configlist.sh
@@ -42,9 +42,12 @@ if test -n "$DEPFILE"
then
QUOTED_OUTPUT="$(printf '%s\n' "$OUTPUT" | sed 's,[&/\],\\&,g')"
{
+ printf '%s' "$QUOTED_OUTPUT: "
printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
"$SOURCE_DIR"/Documentation/config/*.adoc |
- sed -e 's/[# ]/\\&/g' -e "s/^/$QUOTED_OUTPUT: /"
+ sed -e 's/[# ]/\\&/g' |
+ tr '\n' ' '
+ printf '\n'
printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
"$SOURCE_DIR"/Documentation/config/*.adoc |
sed -e 's/[# ]/\\&/g'
---
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
change-id: 20260421-toon-fix-almalinux8-102de9138294
^ permalink raw reply related
* Re: [PATCH v2] generate-configlist: collapse depfile for older Ninja
From: Toon Claes @ 2026-05-15 8:44 UTC (permalink / raw)
To: phillip.wood, git; +Cc: D. Ben Knoble, Patrick Steinhardt
In-Reply-To: <0557838b-214d-4e8f-9cbd-bc342563e9ba@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> I don't think this use of '\n' portable. The sed man page [1] says that
> '\n' matches a newline in the pattern space, but does not mention it
> being supported in the replacement string. We do have an existing use in
> t4150-am.sh:"am newline in subject" which does
>
> sed -e "s/second/second \\\n foo/" patch1 >patchnl &&
>
> However if I add "cat patchnl" it shows the subject line is
>
> Subject: [PATCH] second \n foo
>
> so sed has inserted "\n" rather than a newline. Indeed looking at the
> commit message for that test it is testing a fix that c escapes are
> printed verbatim introduced by 4b7cc26a74 (git-am: use printf instead of
> echo on user-supplied strings, 2007-05-25).
>
> I've not tested it but I think
>
> sed 's/ $/\
> /'
>
> will insert a newline. Alternatively we could do
>
> printf '%s' "$QUOTED_OUTPUT: "
> printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
> "$SOURCE_DIR"/Documentation/config/*.adoc |
> sed -e 's/[# ]/\\&/g' |
> tr '\n' ' '
> printf '\n'
Whoops, I somehow archived your reply without addressing it. Thanks for
finding this. I've sent out v3 with this suggestion.
--
Toon
^ permalink raw reply
* Re: [PATCH v7 2/3] git-gui: disable gitk visualization when no worktree available
From: Johannes Sixt @ 2026-05-15 8:28 UTC (permalink / raw)
To: Shroom Moo, git; +Cc: Mark Levedahl, Aina Boot
In-Reply-To: <tencent_A6BA86DF71476C6948398C167C0E0919550A@qq.com>
Am 09.05.26 um 15:37 schrieb Shroom Moo:
> When git-gui is started in a bare repository with the 'bare' option
> enabled (e.g., for blame/browser), there is no working tree. The
> "Visualize Current Branch's History" and "Visualize All Branch
> History" menu items remain enabled, but clicking them triggers a Tcl
> error because do_gitk tries to change directory to an empty
> _gitworktree.
I cannot reproduce this claim. The failure is not a Tcl error, but an
error in some `git` invocation that cannot handle an empty
GIT_WORK_TREE. And that happens only beginning with the *second*
invocation of one of the "Visualize" calls, because then an empty
GIT_WORK_TREE is exported into the environment.
>
> Fix this by disabling the two visualization menu items when the
> repository is bare and the 'bare' option is active. Also update
> current_branch_write to keep the state consistent when the branch
> changes, and add a defensive check in do_gitk to avoid the error
> should the menu state somehow become out of sync.
This change is not correct. Gitk can operate without a working tree. The
menu entries should not be disabled, ever. The bug is somewhere else.
See also my suggested replacement patch in my reply to 3/3.
-- Hannes
^ permalink raw reply
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Phillip Wood @ 2026-05-15 9:33 UTC (permalink / raw)
To: Pablo Sabater, phillip.wood
Cc: git, gitster, christian.couder, karthik.188, jltobler,
ayu.chandekar, siddharthasthana31, chandrapratap3519
In-Reply-To: <CAN5EUNQCsKD0CJqDi43i2JVBQQChAZVt_THQ1wGpdeydNHHCFw@mail.gmail.com>
On 14/05/2026 18:45, Pablo Sabater wrote:
> El jue, 14 may 2026 a las 17:15, Phillip Wood
> (<phillip.wood123@gmail.com>) escribió:
>> On 02/04/2026 22:17, Pablo Sabater wrote:
>>> When having a history with multiple root commits and drawing the history
>>> near the roots, the graphing engine renders the commit one below the other,
>>> seeming that they are related, which makes the graph confusing.
>>>
>>> This issue was reported by Junio at:
>>> https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
>>>
>>> e.g.:
>>>
>>> * root-B
>>> * child-A2
>>> * child-A1
>>> * root-A
>>>
>>> [...]
>> >
>>> * root-B
>>> * child-A2
>>> /
>>> * child-A1
>>> * root-A
>>
>> I'm rather late to the party here, but personally I find the indentation
>> a bit confusing, it would be clearer to me if we had a blank line after
>> a root commit
>
> Hi,
>
>>
>> * root-B
>>
>> * child-A2
>> * child-A1
>> * root-A
>>
>> It takes the same amount of vertical space but keeps the children of
>> root-A together.
>
> I have mixed feelings about which approach to choose.
> The idea of a blank line was thought at
> https://lore.kernel.org/git/xmqq8s8vvw9m.fsf@gitster.c.googlers.com/
> but Junio argued against it for having an extra row because the
> indentation he proposed didn't collapse, however I find indentation +
> no collapse the most confusing one.
> I'd say that I'm fine with both approaches, blank line or indentation
> + collapse.
I'm afraid I don't understand this - what does it mean for the
indentation to collapse, or not collapse. Looking at the examples Junio
gave they look quite nice to me, though I'd find it clearer if
| | * 12345678 2021-01-14 merge xxxxx@xxxx into the history
| | |\
| | | \
| | * \ 23456789 2021-01-12 merge citest into the main history
| | |\ * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | | * 3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
Added defau
| | | * ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
f7daf088)
was rendered as
| | * 12345678 2021-01-14 merge xxxxx@xxxx into the history
| | |\
| | | * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | * 23456789 2021-01-12 merge citest into the main history
| | |\
| | | * 3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
Added defau
| | | * ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
f7daf088)
>>> without the patch:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> * D root
>>>
>>> with the patch, the indentation cascades:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> _ /
>>> /
>>> /
>>> * D root
>
> * A root
>
> * B root
>
> * C root
>
> * D1 child
>
> * D root
>
> Here I think a blank line looks worse, too much space for just 5
> commits and becomes one extra line which if this were like up to 7 or
> more parentless commits one after the other would be more noticeable.
But there shouldn't be a blank line between D and D1 so the two
alternatives take up the same amount of vertical space, the main
difference being whether D1 appears next to D
* A root * A root
* B root
* B root * C root
* D1 child
* C root _/
/
* D1 child /
* D root * D root
Of course if the indentation was smarter it would take up less room and
look better than having blank lines
* A root
* B root
* C root
* D1 child
* D root
> But there are cases that blank line might be better:
>
> * 10_A2
> * 10_A1
> * 10_A
> * 10_M
> /|\
> | | * 10_D
> | * 10_C
> * 10_B
>
> Feels like a shower of commits instead of an indented merge.
Yes, that is a bit confusing. I think the thing I find confusing with
this approach is that we're treating the commit rendered below the root
commit specially, rather than treating the root commit itself specially.
To me it is the root commit that's the odd one out because it does not
have any parents, but we treat the commit that's rendered below as the
odd one by indenting it relative to its parents.
> Pro to the blank line, the parentless check is the same and it's just
> printing a '\n' at the right spot, while indent i'm mimicking like if
> there was a commit there.
> Anyways, I think in the majority of the cases the indentation +
> collapsing looks better.
> Sorry for the brief reply, I'm busy today.
No need to apologize, it seemed quite comprehensive to me
Thanks
Phillip
> Regards,
>
> --
> Pablo
>
>>
>> Thanks
>>
>> Phillip
>>
>>> This is done by adding a is_placeholder flag to the columns, the root commit
>>> is actually there but marked as a placeholder
>>>
>>> e.g.:
>>>
>>> * root-B
>>> (B) * child-A2
>>> /
>>> * child-A1
>>> * root-A
>>>
>>> (B) would be root-B column with the placeholder flag active.
>>>
>>> Then teaching the rendering function to print a padding ' ' when meeting a
>>> placeholder column outputs the second example.
>>>
>>> There could also be the case where there are multiple roots
>>>
>>> without the patch:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> * D root
>>>
>>> with the patch, the indentation cascades:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> _ /
>>> /
>>> /
>>> * D root
>>>
>>> the _ / might look weird but that's how the collapsing rendering does it
>>> for big gaps, this case being from the 4th column to the 0th column.
>>> Another patch could change the collapsing rendering for placeholders ?
>>> I haven't done it to keep it minimal, but a follow up could make it
>>> to be straight '/'. This would make it bigger but easier for the eye to follow.
>>> IMO is not worth it, but opinions are welcome.
>>>
>>> The patch also adds tests for different cases like a root preceding multiple
>>> parents merges and the examples above.
>>>
>>> There could be some edge cases still so any testing is very welcome.
>>>
>>> Pablo Sabater (1):
>>> graph: add indentation for commits preceded by a root
>>>
>>> graph.c | 68 ++++++++++++++++--
>>> t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
>>> 2 files changed, 198 insertions(+), 6 deletions(-)
>>>
>>>
>>> base-commit: 256554692df0685b45e60778b08802b720880c50
>>
^ permalink raw reply
* Re: [PATCH v3] generate-configlist: collapse depfile for older Ninja
From: Phillip Wood @ 2026-05-15 9:35 UTC (permalink / raw)
To: Toon Claes, git; +Cc: D. Ben Knoble, Patrick Steinhardt
In-Reply-To: <20260515-toon-fix-almalinux8-v3-1-b545a0647f0f@iotcl.com>
Hi Toon
Thanks for re-rolling, this version looks good to me
Phillip
On 15/05/2026 09:42, Toon Claes wrote:
> The tools/generate-configlist.sh script generates two files:
> * config-list.h
> * config-list.h.d
>
> The former is included by the source code and the latter defines on
> which files the former depends.
>
> The contents of `config-list.h.d` consists of two sections:
>
> config-list.h: Documentation/config.adoc
> config-list.h: Documentation/git-config.adoc
> config-list.h: Documentation/config/add.adoc
> config-list.h: Documentation/config/advice.adoc
> config-list.h: Documentation/config/alias.adoc
> config-list.h: Documentation/config/am.adoc
> config-list.h: Documentation/config/apply.adoc
> ...
>
> This first section actually defines on which individual files
> `config-list.h` depends and thus needs to be rebuild if one of those
> changes.
>
> And the second section contains content like:
>
> Documentation/config.adoc:
> Documentation/git-config.adoc:
> Documentation/config/add.adoc:
> Documentation/config/advice.adoc:
> Documentation/config/alias.adoc:
> Documentation/config/am.adoc:
> Documentation/config/apply.adoc:
> ...
>
> These rules exist to ensure Make won't fail with the following error if
> one of the .adoc files is renamed or removed:
>
> make: *** No rule to make target 'Documentation/config.adoc', needed by 'config-list.h'.
>
> With the no-op targets defined in `config-list.h.d`, Make knows there's
> no work to be done to generate these files, so it doesn't error out if
> it doesn't exist.
>
> For the Makefile build system this works great. And since
> ebeea3c471 (build: regenerate config-list.h when Documentation changes,
> 2026-02-24) this script is also called from the Meson build system.
> Nevertheless, on AlmaLinux 8 the following build failure is seen:
>
> ninja: error: dependency cycle: config-list.h -> config-list.h
>
> This version of this distro uses Ninja 1.8.2 and it seems to have some
> issues with the format of the `config-list.h.d` file.
>
> Ninja versions before 1.10.0 do not reset the depfile parser state on
> newlines. This causes issues when the depfile has one dependency per
> line, like we have in `config-list.h.d`:
>
> config-list.h: Documentation/config.adoc
> config-list.h: Documentation/config/add.adoc
>
> The parser only recognizes the first "config-list.h:" as a target. On
> subsequent lines it is still in dependency-parsing mode, so the repeated
> output name is recorded as an input. This causes the error mentioned
> above.
>
> The bug in Ninja is fixed in 1.10, with commit
> ninja-build/ninja@1daa7470ab7e (depfile_parser: remove restriction on
> multiple outputs, 2019-11-20).
>
> To be compatible with older versions of Ninja, collapse the dependencies
> for `config-list.h` into a single line like:
>
> config-list.h: Documentation/config.adoc Documentation/config/add.adoc ...
>
> This works around the bug in older versions of Ninja, and is fully
> compatible Make and with more recent versions of Ninja. And while the
> no-op targets are not needed for Ninja, they also don't do any harm.
>
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> At GitLab we build images for various distros, including AlmaLinux 8.
> On this distro we got this error while compiling Git.
>
> ninja: error: dependency cycle: config-list.h -> config-list.h
>
> It seems this is caused by a bug in older versions of Ninja. There are
> more details in the commit message, but here are a few simple steps to
> reproduce:
>
> docker run --rm -it -v $(pwd):/git -w /git almalinux:8 bash
> dnf -yq install epel-release
> dnf -yq install shadow-utils sudo make pkg-config gcc findutils \
> diffutils perl python3 gawk gettext zlib-devel expat-devel \
> openssl-devel curl-devel pcre2-devel cargo
> pip3 install --prefix=/usr meson ninja==1.8.2
> meson setup build --warnlevel 2 --werror
> ninja -C build config-list.h
> ninja -C build config-list.h # fails with dependency cycle
> ---
> Changes in v3:
> - Stop using \n in sed(1) replacement strings because it is not
> portable.
> - Link to v2: https://patch.msgid.link/20260422-toon-fix-almalinux8-v2-1-45d8471ed0e9@iotcl.com
>
> Changes in v2:
> - Simplify the changes *a lot* by doing the collapsing unconditionally.
> - Link to v1: https://patch.msgid.link/20260421-toon-fix-almalinux8-v1-1-aec1d54addde@iotcl.com
> ---
> tools/generate-configlist.sh | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
> index e28054f9e0..d1d2ba4bb7 100755
> --- a/tools/generate-configlist.sh
> +++ b/tools/generate-configlist.sh
> @@ -42,9 +42,12 @@ if test -n "$DEPFILE"
> then
> QUOTED_OUTPUT="$(printf '%s\n' "$OUTPUT" | sed 's,[&/\],\\&,g')"
> {
> + printf '%s' "$QUOTED_OUTPUT: "
> printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
> "$SOURCE_DIR"/Documentation/config/*.adoc |
> - sed -e 's/[# ]/\\&/g' -e "s/^/$QUOTED_OUTPUT: /"
> + sed -e 's/[# ]/\\&/g' |
> + tr '\n' ' '
> + printf '\n'
> printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
> "$SOURCE_DIR"/Documentation/config/*.adoc |
> sed -e 's/[# ]/\\&/g'
>
> ---
> base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
> change-id: 20260421-toon-fix-almalinux8-102de9138294
>
>
^ permalink raw reply
* Re: [PATCH] fetch: add fetch.pruneLocalBranches config
From: Harald Nordgren @ 2026-05-15 9:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, gitgitgadget
In-Reply-To: <xmqqecjdea13.fsf@gitster.g>
> Why do I get the above, which apparently is a response to my review
> for
>
> [PATCH] config: suggest the correct form when key contains "="
>
> under this thread? Am I dealing with some sort of mechanical slop?
(Testing plain text email sending via Gmail for a less error-prone
workflow, does it still add the CC's correctly?)
Harald
^ permalink raw reply
* Re: Bug: lowercase "head" resolves to wrong commit in linked worktrees on case-insensitive filesystems
From: Torsten Bögershausen @ 2026-05-15 9:47 UTC (permalink / raw)
To: Alexander Sandström, git
In-Reply-To: <95BE8E60-1684-4E0A-9E46-E61E81D06CE1@alexandersandstrom.se>
On 2026-05-13 10:18, Alexander Sandström wrote:
> Hello everyone,
>
> I ran into a bug that took me a while to figure out.
Thanks for the report.
>
> I'm sadly not a good enough C programmer to submit a proper patch,
> but perhaps this bug report will at least be indexed by search engines
> and help others that might have this issue to understand the cause.
>
> My guess is that it will happen much more frequently now that
> worktrees are more popular.
>
> **Report**
>
> On case-insensitive filesystems (macOS APFS/HFS+), `git rev-parse head`
> (lowercase) in a linked worktree resolves to the main worktree's HEAD
> rather than the current worktree's HEAD. This causes commands like
> `git reset --soft head~1` to silently operate on the wrong commit.
>
> **Setup**
>
> ```sh
> $ git init main && cd main
> $ git commit --allow-empty -m "base"
> $ git commit --allow-empty -m "main-only"
> $ git worktree add ../linked HEAD~1
> $ cd ../linked
> $ git commit --allow-empty -m "linked-only"
> ```
>
> **Expected** `head` and `HEAD` resolve to the same commit in the
> linked worktree (or `head` is rejected as an unknown revision).
This is probably not what you expect.
head should not be used at all, since it is not a valid reference.
When using case-insensitive file systems, head and HEAD may
be the same, but that is not a feature.
In theory, we may be able to refuse head on a case insensitive file system.
And Head, hEad, HEad, you got it.
In practice nobody has done that yet.
And a quick search for
git refs case insensitive
shows a lot of reports about the limitations that a
case insensitive file system gives you.
And yes, you can format a partition on MacOs case-insensitive.
Or live with the limitations that arise.
Or send a patch. For the documentation.
HTH
>
> **Actual**
>
> ```
> $ cd ../linked
> $ git rev-parse HEAD
> <commit: "linked-only">
> $ git rev-parse head
> <commit: "main-only">
> ```
>
> `HEAD` (uppercase) correctly resolves via the per-worktree ref at
> `.git/worktrees/linked/HEAD`. But lowercase `head` falls through to
> general ref resolution, which opens a file named `head` on disk. On a
> case-insensitive filesystem, this matches `.git/HEAD`, the main
> worktree's HEAD, instead of the linked worktree's HEAD.
>
> Without worktrees the bug is latent: `.git/HEAD` is the only HEAD file,
> so the wrong codepath happens to produce the correct result. The bug
> becomes observable only with linked worktrees, where the main and linked
> worktree HEADs diverge.
>
> **Impact** `git reset --soft head~1` in a linked worktree silently
> resets to the wrong commit, staging unexpected changes. This is
> particularly confusing because there is no error or warning. The
> command appears to succeed.
>
> I realize one argument might simply be "lower-case head isn't a thing",
> so feel free to disregard if that is the projects stance.
>
> **Possible fix** During ref resolution, when the input string matches
> `HEAD` case-insensitively but is not exactly `HEAD`, git could either:
> - reject it with an error (matching Linux behavior, where lowercase
> `head` fails with "unknown revision"), or
> - normalize it to `HEAD` and route through the per-worktree codepath.
>
> **Environment**
> - git 2.53.0
> - macOS 15.6 (APFS, case-insensitive)
>
>
> Regards,
> Alexander
>
>
^ permalink raw reply
* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
From: Aina Boot @ 2026-05-15 11:00 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Johannes Sixt, Shroom Moo, git
In-Reply-To: <20260514143322.865587-5-mlevedahl@gmail.com>
On 5/14/26 2:33 PM, Mark Levedahl wrote:
> set picked 0
> +proc pick_repo {} {
> + unset_gitdir_vars
> + load_config 1
> + apply_config
> + choose_repository::pick
> + set _gitdir [git rev-parse --absolute-git-dir]
> + set _prefix {}
> + set picked 1
> +}
> +
>
Here inside the proc it create vars locally, "global..." is missing.
Aina
^ permalink raw reply
* Re: What's cooking in git.git (May 2026, #03)
From: Karthik Nayak @ 2026-05-15 11:49 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqik8tm16n.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
> Here are the topics that have been cooking in my tree. Commits
> prefixed with '+' are in 'next' (being in 'next' is a sign that a
> topic is stable enough to be used and is a candidate to be in a
> future release). Commits prefixed with '-' are only in 'seen', and
> aren't considered "accepted" at all and may be annotated with a URL
> to a message that raises issues but they are by no means exhaustive.
> A topic without enough support may be discarded after a long period
> of no activity (of course they can be resubmitted when new interests
> arise).
>
> The first batch of topics marked for graduation for quite a while
> since 2.54-rc2 have all been merged to 'master'.
>
> Copies of the source code to Git live in many repositories, and the
> following is a list of the ones I push into or their mirrors. Some
> repositories have only a subset of branches.
>
> With maint, master, next, seen, todo:
>
> git://git.kernel.org/pub/scm/git/git.git/
> git://repo.or.cz/alt-git.git/
> https://kernel.googlesource.com/pub/scm/git/git/
> https://github.com/git/git/
> https://gitlab.com/git-scm/git/
>
> With all the integration branches and topics broken out:
>
> https://github.com/gitster/git/
>
> Even though the preformatted documentation in HTML and man format
> are not sources, they are published in these repositories for
> convenience (replace "htmldocs" with "manpages" for the manual
> pages):
>
> git://git.kernel.org/pub/scm/git/git-htmldocs.git/
> https://github.com/gitster/git-htmldocs.git/
>
> Release tarballs are available at:
>
> https://www.kernel.org/pub/software/scm/git/
>
Hello Junio,
I've not been active on the list past few weeks, did we reach a
consensus about
20260420-refs-fsck-skip-lock-files-v1-1-c2595e206a76@gmail.com ? Or was
it missed, I thought it was in a ready state, but happy to reiterate as
needed.
Lore: https://lore.kernel.org/git/20260420-refs-fsck-skip-lock-files-v1-1-c2595e206a76@gmail.com/#t
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: Email issues
From: Kristoffer Haugsbakk @ 2026-05-15 12:02 UTC (permalink / raw)
To: Harald Nordgren, Junio C Hamano; +Cc: git, Koji Nakamaru
In-Reply-To: <20260515075611.59535-1-haraldnordgren@gmail.com>
On Fri, May 15, 2026, at 09:56, Harald Nordgren wrote:
>> Why do I get the above, which apparently is a response to my review
>> for
>>
>> [PATCH] config: suggest the correct form when key contains "="
>>
>> under this thread? Am I dealing with some sort of mechanical slop?
>
> I think the problem here is my email sending process is not good. I edit
> all the emails in Sublime text, where I keep the same file for all
> different threads.
>
> I have the subject line as the first line of the file and like you notice I
> forget to change it sometimes.
>
> I keep each of the topics bookmarked like this,
> https://lore.kernel.org/git/xmqqecjdea13.fsf@gitster.g/, and then utilize
> that like to send the email
>
> ```
> git send-email \
> --in-reply-to=xmqqecjdea13.fsf@gitster.g \
> --to=gitster@pobox.com \
> --cc=git@vger.kernel.org \
> --cc=gitgitgadget@gmail.com \
> --cc=haraldnordgren@gmail.com \
> /path/to/YOUR_REPLY
> ```
>
> I tried playing with neomutt and and email client replacement, but that
> adds the complexity of downloading a new mbox file for each reply, it
> didn't seem easier, but maybe it is.
>
> How do you handle emails?
I use the Fastmail webmail client for
regular non-patch emails. The only
things it messes up so far is long lines
in replies to patches.
I edit the emails in a text editor. And sometimes
I have left multiple drafts before sending them
and switched them around. Only to see my mistake on the Lore archive later. :)
But by and large it works just fine. I haven't had
the need for a more ergonomic setup.
--
Sent from mobile
^ permalink raw reply
* Re: [PATCH] revision: use priority queue in limit_list()
From: Derrick Stolee @ 2026-05-15 13:10 UTC (permalink / raw)
To: Kristofer Karlsson, Jeff King, Junio C Hamano
Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <CAL71e4Mfq3SCO7vnTbFCxpzH9txWPTencV-vq-aQ=wJ7dPMV2g@mail.gmail.com>
On 5/15/2026 3:47 AM, Kristofer Karlsson wrote:
> Unfortunately git.git's mostly-linear history doesn't
> trigger the quadratic behavior (the queue stays narrow). Even with
> 5,584 commits in the symmetric diff, `--left-right --count` finishes
> in ~0.4s on git.git for both baseline and patched. A 50-pair
> interleaved run shows no statistically significant difference:
>
> git rev-list --left-right --count v2.47.1...v2.54.0 (git.git, 5,584 commits)
> 50 interleaved paired runs:
>
> baseline: mean 393ms, stdev 13ms, median 392ms
> patched: mean 396ms, stdev 14ms, median 393ms
> paired t-test: +2.9ms, t=1.16, p>0.05 (not significant)
Thanks for sharing these details! Consider my curiosity sated.
> The existing t/perf tests don't cover this path. p0001 doesn't
> use --left-right and p6010 is merge-base specific. I could add a
> perf test, though it would need a merge-heavy test repo to show the
> difference. Would a synthetic one (like p6010 does) be useful?
I'm usually interested in encoding ways to repeatedly exercise
these performance gains and preventing regression in the future.
However, you've demonstrated that not all repositories have a
data shape that reveals the performance problem.
If you happen to find a publicly-available repository that shows
this improvement, then documenting the performance benefits for
that repo would be sufficient. I'm familiar with performance
work that doesn't reveal its most important gains until working
with private repositories at the proper scale, so don't sweat
not having a public example.
I don't think it's worth constructing a synthetic repo to
demonstrate this issue. I was hoping that it would be low-
hanging fruit to cover this in the perf test suite, but that
does not seem to be the case.
Thanks,
-Stolee
^ permalink raw reply
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Phillip Wood @ 2026-05-15 13:22 UTC (permalink / raw)
To: Vincent Lefevre, Junio C Hamano; +Cc: Michael Montalbo, git, j6t
In-Reply-To: <20260514095522.GA159111@qaa.vinc17.org>
On 14/05/2026 10:55, Vincent Lefevre wrote:
> On 2026-05-14 16:37:39 +0900, Junio C Hamano wrote:
>> Michael Montalbo <mmontalbo@gmail.com> writes:
>>
>>> @@ -457,6 +457,11 @@ endif::git-diff[]
>>> +
>>> Note that despite the name of the first mode, color is used to
>>> highlight the changed parts in all modes if enabled.
>>> ++
>>> +Word diff works by finding word-level changes within each hunk of
>>> +the line-level diff. The line-level alignment determines which
>>> +changed lines are compared to each other, which can affect the
>>> +word-level output.
>>
>> The added text may not say anything wrong, but I am not sure how it
>> helps the end user to know the way machinery works internally.
>
> Perhaps only the first sentence should be kept and that the following
> should be added: "Because of that, using the --ignore-space-change
> option is recommended."
>
> Note: Earlier in the discussion, Johannes Sixt suggested -w
> (--ignore-all-space), but this is wrong, as
>
> git diff --word-diff -w <(printf foo) <(printf "f o o")
>
> gives no differences while one has 1 word "foo" vs 3 words "f o o".
>
> However, --ignore-space-change is actually not even sufficient
> since
>
> git diff --ignore-space-change <(printf "foo bar") <(printf "foo\nbar")
>
> finds differences though there are only space changes (thus this
> may affect hunks in case --word-diff would be used too). However,
> I suppose that the cases where --word-diff --ignore-space-change
> would not give a "real word diff" would be quite rare in practice.
I'm a bit wary of recommending -w unconditionally in case it gives
unexpected results. I've not really found there to be a problem using
--word-diff when reviewing code patches. In the examples you gave we'd
ideally fix the problem by computing a single word-diff per hunk from
the line based diff rather than splitting the hunk at each context line.
I think we'd probably want to exclude the leading and trailing context
to keep the hunk header accurate but we'd get better results by
calculating the word diff of everything in the hunk between the first
changed line and the last changed line.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
From: Mark Levedahl @ 2026-05-15 13:33 UTC (permalink / raw)
To: Aina Boot; +Cc: Johannes Sixt, Shroom Moo, git
In-Reply-To: <20260515110027.426-1-bootaina702@gmail.com>
On 5/15/26 7:00 AM, Aina Boot wrote:
> On 5/14/26 2:33 PM, Mark Levedahl wrote:
>> set picked 0
>> +proc pick_repo {} {
>> + unset_gitdir_vars
>> + load_config 1
>> + apply_config
>> + choose_repository::pick
>> + set _gitdir [git rev-parse --absolute-git-dir]
>> + set _prefix {}
>> + set picked 1
>> +}
>> +
>>
> Here inside the proc it create vars locally, "global..." is missing.
>
> Aina
Yes, also, I missed copying the check on the return variable ::_gitdir. That proc should be:
proc pick_repo {} {
global _gitdir picked
unset_gitdir_vars
load_config 1
apply_config
choose_repository::pick
if {![file isdirectory $_gitdir]} {
exit 1
}
set _gitdir [git rev-parse --absolute-git-dir]
set picked 1
}
Mark
^ permalink raw reply
* Re: [PATCH 1/2] strbuf: use st_add3() in strbuf_grow()
From: René Scharfe @ 2026-05-15 14:30 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git List
In-Reply-To: <20260515043606.GA83595@coredump.intra.peff.net>
On 5/15/26 6:36 AM, Jeff King wrote:
> On Thu, May 14, 2026 at 10:13:19PM +0200, René Scharfe wrote:
>
>> Hmm, alloc_nr() doesn't do any overflow checking. It should, though,
>> shouldn't it?
>
> Yes, probably. It's a known blind spot in the overflow checking, but
> I think is OK in practice because:
>
> 1. We are growing an existing buffer by ~3/2. So even with ordering
> the multiplication first, an overflow implies that you have a
> single buffer consuming ~1/3 of your address space.
>
> On 64-bit systems that's impractically large, and on 32-bit systems I
> think you generally run into fragmentation and address-space issues
> first.
>
> 2. If alloc_nr(alloc) is less than the desired nr, we just use that nr
> directly. So even if we did overflow, I think the result is
> too-slow allocation, and not a buffer overflow.
> > But it would be nice to be less hand-wavy. One of the reasons I hadn't
> dug into it further is that I wanted to start making use of intrinsics
> to avoid slowdowns. But since you're already doing that (and finding
> that the compiler was doing the fast thing anyway!) it might be a good
> time to make the jump.
Didn't look at __builtin_mul_overflow() in detail; its situation could
be different than for __builtin_add_overflow(), which turned out to be
unnecessary on x64.
> That's all assuming that no overflow happens before ALLOC_GROW() gets
> the values. We also tend to do unchecked computions for the "nr" field
> there, but it's usually just "nr_foo + 1", so the same logic applies:
> you'd have to have an existing array consuming the entire address space
> minus one byte to trigger an overflow.
The use in read-cache.c::do_read_index() looks odd. Has been present
since commit one. Is the point that it over-allocates to have room for
additions right from the start? For read-only commands this only wastes
memory, no?
René
^ permalink raw reply
* Re: [PATCH v6 00/10] parseopt: add subcommand autocorrection
From: Jiamu Sun @ 2026-05-15 14:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Aaron Plattner, Karthik Nayak
In-Reply-To: <xmqqcxz2tzpr.fsf@gitster.g>
On Mon, May 11, 2026 at 12:03:12PM +0900, Junio C Hamano wrote:
> I've been carrying the following fix on top of these series since
> Apr 23 when the topic was merged to 'seen'. Can you fix these up at
> the source, so that we can move forward with this topic?
>
> Thanks.
Sorry for the delay. This email didn't reach my inbox.
By the time I saw this fix, I had already sent v6. Should I resend v6
with this fix squashed in, or bump to v7?
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>
^ permalink raw reply
* Re: [PATCH 2/2] use __builtin_add_overflow() in st_add() with Clang
From: René Scharfe @ 2026-05-15 14:36 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <20260515044059.GB83595@coredump.intra.peff.net>
On 5/15/26 6:40 AM, Jeff King wrote:
> On Thu, May 14, 2026 at 05:13:46PM +0200, René Scharfe wrote:
>
>> Clang and GCC optimize away comparisons of overflow checks by checking
>> the carry flag on x64. GCC does the same on ARM64, but Clang currently
>> (version 22.1) doesn't.
>>
>> Provide a variant of st_add() that wraps __builtin_add_overflow() to
>> help Clang optimize it. Use it on all platforms for simplicity.
>
> OK. I probably would have just used the intrinsic everywhere with
> __GNUC__, but if gcc is already figuring it out, it doesn't matter in
> practice.
Did that initially, but found it hard to justify when it has no benefit
and reduces the test coverage of the hand-made check significantly.
>> +/* Help Clang; GCC generates the same code for both variants. */
>> +#if defined(__clang__)
>> +static inline size_t st_add(size_t a, size_t b)
>> +{
>> + size_t sum;
>> + if (__builtin_add_overflow(a, b, &sum))
>> + die("size_t overflow: %"PRIuMAX" + %"PRIuMAX,
>> + (uintmax_t)a, (uintmax_t)b);
>> + return sum;
>> +}
>> +#else
>> static inline size_t st_add(size_t a, size_t b)
>> {
>> if (unsigned_add_overflows(a, b))
>
> It's a shame we can't share more code here, especially the die message.
>
> I guess the ideal primitive is probably a wrapper with the same
> interface as __builtin_add_overflow(), which could then be used
> everywhere that unsigned_add_overflows() with some minor conversion.
Junio said the same. :)
> But it gets awkward to do as a macro, and using an inline function runs
> into type questions.
Indeed. If it was easy then this wouldn't exist as a builtin. We
can approximate it somewhat, but will it be robust enough?
René
^ permalink raw reply
* Re: [PATCH] rebase: ignore non-branch update-refs
From: Phillip Wood @ 2026-05-15 15:40 UTC (permalink / raw)
To: Junio C Hamano, mail; +Cc: git, Derrick Stolee
In-Reply-To: <0911df2d-aaa2-456e-a678-345239cefc67@gmail.com>
On 10/05/2026 14:37, Phillip Wood wrote:
>
> Looking at make_script_with_merges() it also calls
> load_branch_decorations() so we should probably add something like the
> diff below. Having said that this patch is a strict improvement so we
> can always fix make_script_with_merges() as a follow up.
I've just had another look at this and even though we call
load_branch_decorations() after calling setup_revisions_from_strvec()
and prepare_revision_walk() we only load branch decorations. It turns
out that "%d" calls load_ref_decorations() the first time it formats a
commit and because we call load_branch_decorations() before the first
call to get_revisions() we haven't formatted any commits yet. So we
don't need to worry about rebase.instructionFormat changing the
decorations that get loaded when generating the todo list with
make_script_with_merges(). "rebase --update-refs" without "-r" only
calls load_branch_decorations() after we've formatted a commit which is
why it is affected.
Thanks
Phillip
>
> Thanks
>
> Phillip
>
> ---- 8< ----
>
> diff --git b/sequencer.c b/sequencer.c
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5982,6 +5982,15 @@ static int make_script_with_merges(struct
> pretty_print_context *pp,
> const char *label = label_from_message.buf;
> const struct name_decoration *decoration =
> get_name_decoration(&to_merge->item->object);
> +
> + /*
> + * If rebase.instructionFormat includes "%d"
> + * then we to skip non-local decorations as
> + * we're only interested in branch names
> + */
> + while (decoration &&
> + decoration->type != DECORATION_REF_LOCAL)
> + decoration = decoration->next;
>
> if (decoration)
> skip_prefix(decoration->name, "refs/heads/",
>
>
^ permalink raw reply
* [PATCH 0/3] Some more "Raw output format" doc improvements
From: Philippe Blain via GitGitGadget @ 2026-05-15 15:48 UTC (permalink / raw)
To: git; +Cc: Philippe Blain
Here are some small improvements to the "Raw output format" documentation
that I noticed while working on another topic.
Philippe Blain (3):
diff-format.adoc: remove mention of diff-tree specific output
diff-format.adoc: 'git diff-files' prints two lines for unmerged files
diff-format.adoc: mode and hash are 0* for unmerged paths from index
only
Documentation/diff-format.adoc | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2304%2Fphil-blain%2Fdiff-raw-format-doc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2304/phil-blain/diff-raw-format-doc-v1
Pull-Request: https://github.com/git/git/pull/2304
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/3] diff-format.adoc: remove mention of diff-tree specific output
From: Philippe Blain via GitGitGadget @ 2026-05-15 15:48 UTC (permalink / raw)
To: git; +Cc: Philippe Blain, Philippe Blain
In-Reply-To: <pull.2304.git.git.1778860091.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
In the "Raw output format" section, we start by mentioning that 'git
diff-tree' prints the hashes of what is being compared. This is only
true in --stdin mode, and is already mentioned in the description of
'--stdin' in git-diff-tree.adoc. Remove this sentence such that we only
focus on the common output between diff-tree, diff-index, diff-files and
diff --raw.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
Documentation/diff-format.adoc | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/Documentation/diff-format.adoc b/Documentation/diff-format.adoc
index 9f7e988241..7f18c64f1e 100644
--- a/Documentation/diff-format.adoc
+++ b/Documentation/diff-format.adoc
@@ -19,9 +19,7 @@ compared differs:
`git-diff-files [<pattern>...]`::
compares the index and the files on the filesystem.
-The `git-diff-tree` command begins its output by printing the hash of
-what is being compared. After that, all the commands print one output
-line per changed file.
+All the commands print one output line per changed file.
An output line is formatted this way:
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox