* Re: [PATCH 1/2] strbuf: use st_add3() in strbuf_grow()
From: Jeff King @ 2026-05-15 16:50 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git List
In-Reply-To: <459f5f2b-2565-4dae-9f9f-8848a5cb9d94@web.de>
On Fri, May 15, 2026 at 04:30:34PM +0200, René Scharfe wrote:
> > 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?
Hmm, yeah, that is weird, and unusual to use alloc_nr() directly. We are
presumably picking up istate->cache_nr from the on-disk file, so it
could be anything, and that alloc_nr() could overflow.
We'd store the too-small value in alloc, so we _know_ it's too small. So
later when we use ALLOC_GROW(), the problem would be resolved as we grow
the array. I'm not convinced the initial load might not overflow the
array, though.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] use __builtin_add_overflow() in st_add() with Clang
From: René Scharfe @ 2026-05-15 16:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jeff King
In-Reply-To: <fceded1f-60a2-48d2-91fc-5d2161272868@web.de>
On 5/14/26 10:17 PM, René Scharfe wrote:
> On 5/14/26 9:12 PM, Junio C Hamano wrote:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>> Provide a variant of st_add() that wraps __builtin_add_overflow() to
>>> help Clang optimize it. Use it on all platforms for simplicity.
>>> ...
>>> +/* 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))
>>> @@ -621,6 +632,7 @@ static inline size_t st_add(size_t a, size_t b)
>>> (uintmax_t)a, (uintmax_t)b);
>>> return a + b;
>>> }
>>> +#endif
>>
>> Makes me wonder if we tweaked unsigned_add_overflows() to take an
>> extra *dst parameter to match __builtin_add_overflow(), which of
>> course requires us to all of 18 callsites, it might make the whole
>> thing a bit simpler. New uses of unsigned_add_overflows(), if we
>> ever add them, would automatically benefit, right?
>
> Hmm. It sounds like a lot of churn, but it would make sure that
> we use the checked result and not check a + b and then go on and
> use x + y because the code de-synced at some point.
>
> How to do it, though? It needs to be generic and evaluate its
> arguments only once. Perhaps like this?
>
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index ca89cfb0b3..27fbb622d7 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -103,6 +103,21 @@ struct strbuf;
> #define unsigned_add_overflows(a, b) \
> ((b) > maximum_unsigned_value_of_type(a) - (a))
>
> +static bool uint_add_overflow(uintmax_t a, uintmax_t b,
> + uintmax_t *out, size_t out_size)
> +{
> + if (b > UINTMAX_MAX - a)
> + return true;
> + a += b;
> + if (a > (UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * out_size)))
> + return true;
> + *out = a;
> + return false;
> +}
> +
> +#define UINT_ADD_OVERFLOW(a, b, out) \
> + uint_add_overflow((a), (b), (out), sizeof(a))
> +
> /*
> * Returns true if the multiplication of "a" and "b" will
> * overflow. The types of "a" and "b" must match and must be unsigned.
> @@ -616,10 +631,11 @@ int git_open_cloexec(const char *name, int flags);
>
> static inline size_t st_add(size_t a, size_t b)
> {
> - if (unsigned_add_overflows(a, b))
> + size_t ret;
> + if (UINT_ADD_OVERFLOW(a, b, &ret))
Type mismatch of third argument: pointer to size_t given, pointer to
uintmax_t expected.
> die("size_t overflow: %"PRIuMAX" + %"PRIuMAX,
> (uintmax_t)a, (uintmax_t)b);
> - return a + b;
> + return ret;
> }
> #define st_add3(a,b,c) st_add(st_add((a),(b)),(c))
> #define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))
Perhaps like this instead?
diff --git a/git-compat-util.h b/git-compat-util.h
index ae1bdc90a4..23ea42f373 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -103,6 +103,25 @@ struct strbuf;
#define unsigned_add_overflows(a, b) \
((b) > maximum_unsigned_value_of_type(a) - (a))
+static inline uintmax_t uint_add_overflow(uintmax_t a, uintmax_t b,
+ uintmax_t max, bool *overflow)
+{
+ *overflow = a > max || b > max - a;
+ return a + b;
+}
+
+#ifdef __clang__
+#define UINT_ADD_OVERFLOW(a, b, out, overflow) \
+ (*(overflow) = __builtin_add_overflow((a), (b), (out)))
+#else
+#define UINT_ADD_OVERFLOW(a, b, out, overflow) ( \
+ *(out) = uint_add_overflow((a), (b), \
+ maximum_unsigned_value_of_type(*(out)), \
+ (overflow)), \
+ *(overflow) \
+)
+#endif
+
/*
* Returns true if the multiplication of "a" and "b" will
* overflow. The types of "a" and "b" must match and must be unsigned.
@@ -616,10 +635,12 @@ int git_open_cloexec(const char *name, int flags);
static inline size_t st_add(size_t a, size_t b)
{
- if (unsigned_add_overflows(a, b))
+ bool overflow;
+ size_t ret;
+ if (UINT_ADD_OVERFLOW(a, b, &ret, &overflow))
die("size_t overflow: %"PRIuMAX" + %"PRIuMAX,
(uintmax_t)a, (uintmax_t)b);
- return a + b;
+ return ret;
}
#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))
#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))
^ permalink raw reply related
* Re: [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository
From: Johannes Sixt @ 2026-05-15 16:06 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260514143322.865587-8-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui attempts to use env(GIT_DIR) directly as the git repository,
> accepting GIT_DIR if it is a directory. Only if that fails is git
> rev-parse used to discover the repository. But, this avoids all of
> git-core's validity checking on a repository, thus possibly deferring an
> error to a later step, possibly unexpected. Repository validation should
> be part of initial setup so that later processing does not need error
> trapping for configuration errors.
OK. If the user gave us GIT_DIR with our without GIT_WORK_TREE, then
that combination better be workable.
>
> Let's just invoke rev-parse so all error checking is done. Stop here if
> the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
> behavior and show the repository picker.
OK. But the paragraph is confusing, because a big "If an error occurs"
is missing after the first sentence.
>
> Also, remove a later check on whether _gitdir is a directory: that code
> cannot be reached without rev-parse having validating the repository.
Good.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 2e2ddc0..81789dd 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -374,6 +374,7 @@ set _gitdir {}
> set _gitworktree {}
> set _isbare {}
> set _githtmldir {}
> +set _prefix {}
> set _reponame {}
> set _shellpath {@@SHELL_PATH@@}
>
> @@ -1167,19 +1168,18 @@ proc pick_repo {} {
> set picked 1
> }
>
> +# find repository.
> if {[catch {
> - set _gitdir $env(GIT_DIR)
> - set _prefix {}
> - }]
> - && [catch {
> - # beware that from the .git dir this sets _gitdir to .
> - # and _prefix to the empty string
> - set _gitdir [git rev-parse --absolute-git-dir]
> - set _prefix [git rev-parse --show-prefix]
> - } err]} {
> + set _gitdir [git rev-parse --absolute-git-dir]
Please do also set _prefix. It should fix the bug that the file chooser
uses an empty prefix after
cd lib
GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser master .
(this is an old bug.)
Please keep the additional indentation of the catch body.
> +} err]} {
> + if {[is_gitvars_error $err]} {
> + exit 1
> + } else {
> pick_repo
> + }
Treat the 'if' as an early exist without an else, and we don't need the
previously strange indentation of 'pick_repo'.
> }
>
> +
> # Use object format as hash algorithm (either "sha1" or "sha256")
> set hashalgorithm [git rev-parse --show-object-format]
> if {$hashalgorithm eq "sha1"} {
> @@ -1191,12 +1191,6 @@ if {$hashalgorithm eq "sha1"} {
> exit 1
> }
>
> -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
(Stopping the review here for today.)
-- Hannes
^ permalink raw reply
* Re: [PATCH v1 05/11] git-gui: use --absolute-git-dir
From: Johannes Sixt @ 2026-05-15 16:00 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260514143322.865587-6-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui uses git rev-parse --git-dir to get the pathname of the
> discovered git repository. The returned value can be relative, and is
> '.' if the current directory is the top of the repository directory
> itself. git-gui has code to change '.' to [pwd] in this case so that
> subsequent logic runs.
>
> But, git rev-parse supports --absolute-git-dir from fac60b8925
> ("rev-parse: add option for absolute or relative path formatting",
> 2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
> this more useful form is always available. Use --absolute-git-dir to
> always get an absolute path, avoiding the need for other checks.
Nice!
However, the patch is incomplete. We set _gitdir also from
lib/choose_repository.tcl. I think it would be best to swap this patch
with patch 4/11, remove the _gitdir setters from the picker
implementation, and call `rev-parse --absolute-git-dir` like you did in
4/11. This depends on that the picker sets the current directory to the
top-level of the working tree with the embeded .git directory.
BTW, missing sign-off.
> ---
> git-gui.sh | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 0b73c35..c2cf5f1 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1156,7 +1156,7 @@ if {[catch {
> && [catch {
> # beware that from the .git dir this sets _gitdir to .
> # and _prefix to the empty string
> - set _gitdir [git rev-parse --git-dir]
> + set _gitdir [git rev-parse --absolute-git-dir]
> set _prefix [git rev-parse --show-prefix]
> } err]} {
> pick_repo
> @@ -1173,18 +1173,12 @@ if {$hashalgorithm eq "sha1"} {
> exit 1
> }
>
> -# we expand the _gitdir when it's just a single dot (i.e. when we're being
> -# run from the .git dir itself) lest the routines to find the worktree
> -# get confused
> -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
-- Hannes
^ permalink raw reply
* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
From: Johannes Sixt @ 2026-05-15 15:59 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260514143322.865587-5-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui includes a 'repository picker', which allows creating a new
> repository + worktree, or selecting a worktree from a recent list.
> git-gui runs the picker when a valid git repository is not found. All of
> the code for this is embedded in the discovery process block, making the
> latter more difficult to read, and also making things more difficult if
> we want to have an explicit 'pick' subcommand to force this to run.
OK, let's see how useful this becomes.
>
> Let's move this invocation and supporting code to a separate proc,
> aiding in subsequent refactoring. Assure GIT_DIR and GIT_WORK_TREE are
> unset, configuration is loaded, ant that _gitdir is correctly set
s/ant/and/
> afterwards. As this is invoked before worktree discovery, later code
> will set that anyway so need not be included here.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 387cad6..0b73c35 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1139,6 +1139,16 @@ proc unset_gitdir_vars {} {
> }
>
> 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
> +}
> +
So, this isn't intended as a plain move of code? Since we set _gitdir
here, we could remove the corresonding lines from lib/choose_repository.tcl.
Is the variable "picked" only needed for this particular picker
invocation? Then it should not be set in the function, but at the call site.
> if {[catch {
> set _gitdir $env(GIT_DIR)
> set _prefix {}
> @@ -1149,13 +1159,7 @@ if {[catch {
> set _gitdir [git rev-parse --git-dir]
> set _prefix [git rev-parse --show-prefix]
> } err]} {
> - load_config 1
> - apply_config
> - choose_repository::pick
> - if {![file isdirectory $_gitdir]} {
> - exit 1
> - }
> - set picked 1
> + pick_repo
The indentation is off here.
> }
>
> # Use object format as hash algorithm (either "sha1" or "sha256")
-- Hannes
^ permalink raw reply
* Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
From: Johannes Sixt @ 2026-05-15 15:58 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260514143322.865587-4-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
> environment, and furthmore unconditionally unsets these in many places.
> But, GIT_WORK_TREE should be set only if it is not {} as the empty
> value, really meaning no work-tree is found, causes git to throw fatal
> errors (git-gui gets the error from branch --show-current). Fixing this
> is required to allow blame and browser to operate from a repository
> without a worktree.
>
> Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
> the environment, avoiding any error if they do not exist. Also, add a
> function to export these, but export GIT_WORK_TREE only if not empty.
Good. But as I said in a parallel thread, I actually concur with your
assessment in the coverletter of this patch series that GIT_WORK_TREE
should be not set at all. At least in the modes that require a working tree.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index a951fcd..387cad6 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1122,6 +1122,22 @@ unset argv0dir
> ##
> ## repository setup
>
> +proc set_gitdir_vars {} {
> + global _gitdir _gitworktree env
> + if {$_gitdir ne {}} {
> + set env(GIT_DIR) $_gitdir
> + }
> + if {$_gitworktree ne {}} {
> + set env(GIT_WORK_TREE) $_gitworktree
> + }
> +}
> +
> +proc unset_gitdir_vars {} {
> + global env
> + catch {unset env(GIT_DIR)}
> + catch {unset env(GIT_WORK_TREE)}
> +}
> +
> set picked 0
> if {[catch {
> set _gitdir $env(GIT_DIR)
> @@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end]
> }
>
> -set env(GIT_DIR) $_gitdir
> -set env(GIT_WORK_TREE) $_gitworktree
> +# Export the final paths
> +set_gitdir_vars
>
> ######################################################################
> ##
> @@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
> # 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)
> + unset_gitdir_vars
> }
> safe_exec_bg [concat $cmd $revs "--" "--"]
>
> - set env(GIT_DIR) $_gitdir
> - set env(GIT_WORK_TREE) $_gitworktree
> + set_gitdir_vars
> cd $pwd
>
> if {[info exists main_status]} {
> @@ -2084,16 +2098,14 @@ proc do_git_gui {} {
>
> # see note in do_gitk about unsetting these vars when
> # running tools in a submodule
> - unset env(GIT_DIR)
> - unset env(GIT_WORK_TREE)
> + unset_gitdir_vars
>
> set pwd [pwd]
> cd $current_diff_path
>
> safe_exec_bg [concat $exe gui]
>
> - set env(GIT_DIR) $_gitdir
> - set env(GIT_WORK_TREE) $_gitworktree
> + set_gitdir_vars
> cd $pwd
>
> set status_operation [$::main_status \
After these changes, a 'global env' probably becomes stale and could be
removed.
-- Hannes
^ permalink raw reply
* Re: [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing
From: Johannes Sixt @ 2026-05-15 15:56 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260514143322.865587-3-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui has subcommands blame and browser, both of which accept a
> pathname, possibly preceded by a commit-ish item to specify a revision.
> Also, blame can take a first argument that gives a line number to focus.
>
> The command line parser for the above is more complex than needed, and
> cannot work without a worktree as the pathname objects are checked
> against the current worktree for existence. This also precludes naming a
> directory or file that does not exist on the currently checked out
> branch.
While the old browser isn't simple, it implements the strategy "revs
before paths, no revs after the first path or '--'" that is applied by
every git command. The rewritten parser is only slightly simpler. It
should not ignore "--". Furthermore, the old parser ignored excessive
trailing arguments, while the new parser ignores excessive leading
arguments. Neither is desirable, and we should report an incorrect
argument list (if we don't, then I prefer the old behavior).
>
> So, replace this with a simpler parser that looks at argument number and
> number of arguments to know what value to expect. The blame and browser
> backends already have error checking with diagnostic information, so
> defer most error checking to those. Also, allow a line-number selection
> to be given and silently ignored for the browser, further simplifying
> this code.
The line number selection isn't ignored by the browser, but reported as
an incorrect usage before and after this patch.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 66 +++++++++++++-----------------------------------------
> 1 file changed, 16 insertions(+), 50 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 6048f92..a951fcd 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2986,51 +2986,34 @@ blame {
> set head {}
> set path {}
> set jump_spec {}
> - set is_path 0
> + set nargs [llength $argv]
> + if {$nargs < 1} {
> + usage
> + }
> + set argn 0
> foreach a $argv {
> - set p [file join $_prefix $a]
> + set argn [expr {$argn + 1}]
>
> - if {$is_path || [file exists $p]} {
> - if {$path ne {}} usage
> - set path [normalize_relpath $p]
> - break
> - } elseif {$a eq {--}} {
> - if {$path ne {}} {
> - if {$head ne {}} usage
> - set head $path
> - set path {}
> + if {$argn < $nargs} {
> + # revision or line number
> + if {[regexp {^--line=(\d+)$} $a a lnum]} {
> + set jump_spec [list $lnum]
> + } else {
> + set head $a
> }
> - set is_path 1
> - } elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
> - if {$jump_spec ne {} || $head ne {}} usage
> - set jump_spec [list $lnum]
> - } elseif {$head eq {}} {
> - if {$head ne {}} usage
> - set head $a
> - set is_path 1
> - } else {
> - usage
> - }
> - }
> - unset is_path
> -
> - if {$head ne {} && $path eq {}} {
> - if {[string index $head 0] eq {/}} {
> - set path [normalize_relpath $head]
> - set head {}
> } else {
> - set path [normalize_relpath $_prefix$head]
> - set head {}
> + set path [normalize_relpath $a]
This loses the capability to request a browser relative to a
subdirectory of the working tree. For example, "git gui browser main ."
now shows the top-level directory instead of the subdirectory when
invoked from a subdirectory.
> }
> }
>
> if {$head eq {}} {
> load_current_branch
> + set head $current_branch
> } else {
> if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
> if {[catch {
> - set head [git rev-parse --verify $head]
> - } err]} {
> + set head [git rev-parse --verify $head]
> + } err]} {
Please leave the indentation unchanged.
> if {[tk windowingsystem] eq "win32"} {
> tk_messageBox -icon error -title [mc Error] -message $err
> } else {
> @@ -3046,26 +3029,9 @@ blame {
> switch -- $subcommand {
> browser {
> if {$jump_spec ne {}} usage
> - if {$head eq {}} {
> - if {$path ne {} && [file isdirectory $path]} {
> - set head $current_branch
> - } else {
> - set head $path
> - set path {}
> - }
> - }
> browser::new $head $path
> }
> blame {
> - if {$head eq {} && ![file exists $path]} {
> - catch {wm withdraw .}
> - tk_messageBox \
> - -icon error \
> - -type ok \
> - -title [mc "git-gui: fatal error"] \
> - -message [mc "fatal: cannot stat path %s: No such file or directory" $path]
> - exit 1
> - }
> blame::new $head $path $jump_spec
> }
> }
The check for the existence of files is actually necessary to
disambiguate the meaning of the argument. If a file "maint" exists, then
the argument is to be interpreted as path, not as the ref "maint", even
if that exists, too.
I suggest to protect the "file exists" calls with ($_gitworktree ne {}
&& ...) or (![is_bare] && ...) to handle being invoked from a bare
repository. That is, in a bare repository we treat arguments the same as
files that do not exist in the currently checked-out branch.
-- Hannes
^ permalink raw reply
* Re: [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser
From: Johannes Sixt @ 2026-05-15 15:54 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260514143322.865587-2-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> Invoking "git-gui browser rev ." should show the file browser for the
> commitish rev, starting at the root directory. This errors out in
> normalize_relpath because the '.' is removed, yielding an empty list as
> argument to [file join ...]. Fix this.
Good catch!
The description isn't precise, though. '.' means to list the current
directory. The mentioned problem happens only if this is also the root
of the working tree.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 23fe76e..6048f92 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2965,7 +2965,11 @@ proc normalize_relpath {path} {
> }
> lappend elements $item
> }
> - return [eval file join $elements]
> + if {$elements ne {}} {
> + return [eval file join $elements]
> + } else {
> + return {}
> + }
> }
>
> # -- Not a normal commit type invocation? Do that instead!
-- Hannes
^ permalink raw reply
* [PATCH 3/3] diff-format.adoc: mode and hash are 0* for unmerged paths from index only
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 mention that the 'mode' and
'sha1' for "src" and "dst" are 0* if "(creation|deletion) or unmerged".
For unmerged entries, 'mode' and 'sha1' are in fact 0* only when we are
looking at the index, i.e. on the left side for 'git diff-files' and on
the right side for 'git diff-index --cached'. Be more precise by
mentioning this, and while at it uniformize the wording of the "work
tree out of sync with the index" case.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
Documentation/diff-format.adoc | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/diff-format.adoc b/Documentation/diff-format.adoc
index 43d91ef868..ef5df140fe 100644
--- a/Documentation/diff-format.adoc
+++ b/Documentation/diff-format.adoc
@@ -37,13 +37,13 @@ unmerged :000000 000000 0000000 0000000 U file6
That is, from the left to the right:
. a colon.
-. mode for "src"; 000000 if creation or unmerged.
+. mode for "src"; 000000 if creation, or if "src" is from the index and is unmerged.
. a space.
-. mode for "dst"; 000000 if deletion or unmerged.
+. mode for "dst"; 000000 if deletion, or if "dst" is from the index and is unmerged.
. a space.
-. sha1 for "src"; 0\{40\} if creation or unmerged.
+. sha1 for "src"; 0\{40\} if creation, or if "src" is from the index and is unmerged.
. a space.
-. sha1 for "dst"; 0\{40\} if deletion, unmerged or "work tree out of sync with the index".
+. sha1 for "dst"; 0\{40\} if deletion, if "dst" is from the index and is unmerged, or if "dst" is from the work tree and is out of sync with the index.
. a space.
. status, followed by optional "score" number.
. a tab or a NUL when `-z` option is used.
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/3] diff-format.adoc: 'git diff-files' prints two lines for unmerged files
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>
Since 10637b84d9 (diff-files: -1/-2/-3 to diff against unmerged stage.,
2005-11-29), for unmerged entries 'git diff-files' print both an
"unmerged" line ('U'), as well as an "in-place edit" line ('M')
comparing stage 2 (by default) with the working tree. The "Raw output
format" documentation however mentions that all commands print a single
line per changed file. Adjust diff-format.adoc to also mention this
special case, for completeness.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
Documentation/diff-format.adoc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-format.adoc b/Documentation/diff-format.adoc
index 7f18c64f1e..43d91ef868 100644
--- a/Documentation/diff-format.adoc
+++ b/Documentation/diff-format.adoc
@@ -19,7 +19,9 @@ compared differs:
`git-diff-files [<pattern>...]`::
compares the index and the files on the filesystem.
-All the commands print one output line per changed file.
+All the commands print one output line per changed file,
+except `git diff-files` in the case of an unmerged file, which prints
+both an "unmerged" and an "in-place edit" line.
An output line is formatted this way:
--
gitgitgadget
^ permalink raw reply related
* [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
* [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
* 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
* 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 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 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 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: [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] 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: 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: 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: [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: 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] 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: [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
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