* Re: [PATCH 0/4] diff: reject negative context values
From: Junio C Hamano @ 2026-05-10 1:01 UTC (permalink / raw)
To: Michael Montalbo via GitGitGadget; +Cc: git, Michael Montalbo
In-Reply-To: <pull.2105.git.1778022144.gitgitgadget@gmail.com>
"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Negative values for -U and --inter-hunk-context are silently accepted
> and produce structurally invalid diff output.
>
> Malformed hunk headers:
>
> $ wc -l GIT-VERSION-GEN
> 106
> $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
> @@ -503,999- +503,999- @@
It may not matter in the cover letter, but why do you need ~60
whitespace characters at the end of the command line, and many other
lines in the message?
>
>
> Line 503 of a 106-line file, count "999-" is not a valid integer.
>
> Overlapping hunks that cannot be applied:
>
> $ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2 \
> -- git-compat-util.h | git apply --check --reverse
> (success)
>
> $ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \
> -- git-compat-util.h | git apply --check --reverse
> error: patch failed: git-compat-util.h:118
> error: git-compat-util.h: patch does not apply
>
>
> Both options were originally parsed via opt_arg() which gated on
> isdigit(), making negative values impossible. When they were converted
> to OPT_INTEGER_F / OPT_CALLBACK in d473e2e0e8 (diff.c: convert
> -U|--unified, 2019-01-27) and 16ed6c97cc (diff-parseopt: convert
> --inter-hunk-context, 2019-03-24), the implicit rejection was lost.
> PARSE_OPT_NONEG was added but only prevents the --no-* boolean form,
> not negative numeric arguments.
>
> This series restores the original invariant with stronger guarantees:
>
> 1/4 diff: reject negative values for --inter-hunk-context
> Change type to unsigned int, switch to OPT_UNSIGNED.
>
> 2/4 diff: reject negative values for -U/--unified
> Change type to unsigned int, add range check in callback.
>
> 3/4 xdiff: guard against negative context lengths
> BUG() in xdl_get_hunk() as defense in depth.
>
> 4/4 parse-options: clarify PARSE_OPT_NONEG does not reject
> negative numbers
> Documentation fix.
>
>
> The config variables diff.context and diff.interHunkContext have
> always rejected negative values. This series brings the CLI options in line.
>
> Michael Montalbo (4):
> diff: reject negative values for --inter-hunk-context
> diff: reject negative values for -U/--unified
> xdiff: guard against negative context lengths
> parse-options: clarify PARSE_OPT_NONEG does not reject negative
> numbers
>
> diff.c | 25 ++++++++++++++-----------
> diff.h | 4 ++--
> parse-options.h | 5 ++++-
> t/t4032-diff-inter-hunk-context.sh | 6 ++++++
> t/t4055-diff-context.sh | 5 +++++
> xdiff/xemit.c | 16 ++++++++++++----
> 6 files changed, 43 insertions(+), 18 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2105
^ permalink raw reply
* Re: [PATCH] rebase: ignore non-branch update-refs
From: Junio C Hamano @ 2026-05-10 1:11 UTC (permalink / raw)
To: mail; +Cc: git, Derrick Stolee
In-Reply-To: <20260506023944.90691-1-mail@abhinavg.net>
mail@abhinavg.net writes:
> diff --git a/sequencer.c b/sequencer.c
> index b7d8dca47f..25bcfc5da0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6428,6 +6428,16 @@ static int add_decorations_to_list(const struct commit *commit,
> const char *path;
> size_t base_offset = ctx->buf->len;
>
> + /*
> + * The global decoration table may contain names loaded by
> + * a previous pretty format such as "%d".
> + * This will result in refs such as "HEAD" being present.
> + */
Your long topic branch may have local unannotated tags that point
into the middle of it, marking strategic points in the topic.
With this change, the command no longer moves them when it rebases
the entire topic. Isn't it a regression?
> + if (decoration->type != DECORATION_REF_LOCAL) {
> + decoration = decoration->next;
> + continue;
> + }
In other words, what you want to prevent from appearing in the insn
stream may be "HEAD", but if so, "must be DECORATION_REF_LOCAL" is
too broad a net to catch it, and causing unintended collateral damage.
As to the style, as the body of the new conditional works
identically with the existing code to exclude the current branch, I
wonder why it shouldn't read more like this? The following
illustration still uses "must be DECORATION_REF_LOCAL" and that may
have to be corrected, of course.
sequencer.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git i/sequencer.c w/sequencer.c
index b7d8dca47f..1ba95fbae1 100644
--- i/sequencer.c
+++ w/sequencer.c
@@ -6429,10 +6429,12 @@ static int add_decorations_to_list(const struct commit *commit,
size_t base_offset = ctx->buf->len;
/*
- * If the branch is the current HEAD, then it will be
- * updated by the default rebase behavior.
+ * Exclude the "current" branch, which will be updated
+ * by the default rebase behavior. Exclude non-branch
+ * decorations as well.
*/
- if (head_ref && !strcmp(head_ref, decoration->name)) {
+ if ((head_ref && !strcmp(head_ref, decoration->name)) ||
+ (decoration->type != DECORATION_REF_LOCAL)) {
decoration = decoration->next;
continue;
}
^ permalink raw reply related
* Re: [PATCH v3 0/6] mingw: stop using nedmalloc
From: Junio C Hamano @ 2026-05-10 2:31 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Patrick Steinhardt, Johannes Schindelin
In-Reply-To: <pull.2104.v3.git.1778244661.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> Changes since v2:
>
> * Reworded the last 4 patches as recommended by Junio, in preparation for
> squashing them on his end.
Thanks. Applied.
^ permalink raw reply
* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
From: Junio C Hamano @ 2026-05-10 2:41 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git,
Derrick Stolee, Jeff King
In-Reply-To: <20260508190947.GA25792@tb-raspi4>
Torsten Bögershausen <tboegi@web.de> writes:
> On Fri, May 08, 2026 at 09:36:53AM +0200, Johannes Schindelin wrote:
>> Hi Torsten,
>>
>> On Tue, 5 May 2026, Torsten Bögershausen wrote:
>>
>> > On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
>> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > >
>> > > [...]
>> > > @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
>> > > struct object_id *oid)
>> > > {
>> > > unsigned char *p;
>> > > - unsigned long size, c;
>> > > + size_t size;
>> > > + unsigned long c;
>> >
>> > Does this look a little bit strange ?
>>
>> Good point.
>>
>> > p points to an unsigned char (better would be *uint8_t)
>> > then it is dereferenced into an "unsigned long".
>> > Then it is masked with 0x7f
>> > In short: should "c" be declared as uint8_t ?
>>
>> Almost. It should be a `size_t`, so that we don't have to cast it when
>> shifting it. I'll include a fix in the next iteration.
>
> I think I was not very clear here.
> De-referencing a long (or size_t) from any address is something
> I would try to avoid:
> Some processors do not like to read a 32 or 64 bit value from
> an uneven address (and throw an exeption).
> x86 processors just handle it, using more than one bus cycle.
>
> In short: Please keep the up-cast and simply read an uint8_t.
Hmph, I do not think there is "up-cast" to keep. And we do not
dereference a random pointer that would be suitable for unsigned
char * as if it were "unsigned long *" or "size_t *" in this code.
This came from commit 48fb7deb5bbd87933e7d314b73d7c1b52667f80f
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed Jun 17 17:22:27 2009 -0700
Fix big left-shifts of unsigned char
Shifting 'unsigned char' or 'unsigned short' left can result in sign
extension errors, since the C integer promotion rules means that the
unsigned char/short will get implicitly promoted to a signed 'int' due to
the shift (or due to other operations).
This normally doesn't matter, but if you shift things up sufficiently, it
will now set the sign bit in 'int', and a subsequent cast to a bigger type
(eg 'long' or 'unsigned long') will now sign-extend the value despite the
original expression being unsigned.
One example of this would be something like
unsigned long size;
unsigned char c;
size += c << 24;
where despite all the variables being unsigned, 'c << 24' ends up being a
signed entity, and will get sign-extended when then doing the addition in
an 'unsigned long' type.
You could rewrite Linus's example to
unsigned char *cp;
unsigned long size;
unsigned char c;
c = *cp;
size += ((unsigned long)c) << 24;
While I am sympathetic to that position, I also would not mind
unsigned char *cp;
unsigned long size;
unsigned long c;
c = *cp;
size += c << 24;
all that much. In any case, such a "clean-up" has little to do with
the topic under discussion, and itshould be discussed separately on
its own merit, most likely when the dust settles after this topic
lands. Let's not contaminate the patches that is "a trivial rewrite
that is so obviously correct to fix the assumption that ulong and
size_t are of the same size everywhere" with unrelated clean-up.
Thanks.
^ permalink raw reply
* Re: [PATCH 4/4] parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers
From: Michael Montalbo @ 2026-05-10 2:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqq8q9sb5uc.fsf@gitster.g>
On Sat, May 9, 2026 at 3:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Michael Montalbo <mmontalbo@gmail.com>
> >
> > The name "NONEG" can be misread as "no negative [values]" when it
> > actually means "no [boolean] negation" (the --no-* form).
> >
> > When --inter-hunk-context and -U/--unified were converted from a
> > custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
> > and 16ed6c97cc, the implicit rejection of negative values (via
> > isdigit() in the old opt_arg() parser) was silently lost. The
> > previous commits in this series fix the resulting bugs.
>
> I do not think _NONEG has anything to do with the bug. It was
> purely to reject --no-unified and --no-inter-hunk-context.
>
You are right this was a mistaken assumption on my part.
> And there was no change to remove PARSE_OPT_NONEG from anywhere and
> use OPT_UNSIGNED instead to fix any of the bugs fixed in this
> series, ...
>
Right, PARSE_OPT_NONEG is still enabled implicitly by OPT_UNSIGNED so
PARSE_OPT_NONEG really has no part in the story.
> >
> > Add a clarifying note to the flag documentation.
> >
> > Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> > ---
> > parse-options.h | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/parse-options.h b/parse-options.h
> > index 706de9729f..c0a3a3dcae 100644
> > --- a/parse-options.h
> > +++ b/parse-options.h
> > @@ -116,7 +116,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
> > * mask of parse_opt_option_flags.
> > * PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
> > * PARSE_OPT_NOARG: says that this option does not take an argument
> > - * PARSE_OPT_NONEG: says that this option cannot be negated
> > + * PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
> > + * prevents --no-<option> boolean form). Does not reject
> > + * negative numeric values like --option=-1. Use
> > + * OPT_UNSIGNED for options that must be non-negative.
>
> ... I do not think the two additional sentences are warranted. Stop
> at clarifying what negated _means_ (i.e., rejects "--no-<option>"),
> without adding what negated does _not_ mean.
>
Ok, will remove the latter portion of the change in a follow-up.
>
> > * PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
> > * shown only in the full usage.
> > * PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default
^ permalink raw reply
* Re: [PATCH 0/4] diff: reject negative context values
From: Michael Montalbo @ 2026-05-10 2:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqqv7cw9ixu.fsf@gitster.g>
On Sat, May 9, 2026 at 6:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Negative values for -U and --inter-hunk-context are silently accepted
> > and produce structurally invalid diff output.
> >
> > Malformed hunk headers:
> >
> > $ wc -l GIT-VERSION-GEN
> > 106
> > $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
> > @@ -503,999- +503,999- @@
>
> It may not matter in the cover letter, but why do you need ~60
> whitespace characters at the end of the command line, and many other
> lines in the message?
>
Apologies, this is a mistake I made between formatting my cover letter
and moving
it to GitGitGadget. I will clean up the cover letter and be more
careful about that in
the future. Thank you for pointing this out.
>
>
> >
> >
> > Line 503 of a 106-line file, count "999-" is not a valid integer.
> >
> > Overlapping hunks that cannot be applied:
> >
> > $ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2 \
> > -- git-compat-util.h | git apply --check --reverse
> > (success)
> >
> > $ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \
> > -- git-compat-util.h | git apply --check --reverse
> > error: patch failed: git-compat-util.h:118
> > error: git-compat-util.h: patch does not apply
> >
> >
> > Both options were originally parsed via opt_arg() which gated on
> > isdigit(), making negative values impossible. When they were converted
> > to OPT_INTEGER_F / OPT_CALLBACK in d473e2e0e8 (diff.c: convert
> > -U|--unified, 2019-01-27) and 16ed6c97cc (diff-parseopt: convert
> > --inter-hunk-context, 2019-03-24), the implicit rejection was lost.
> > PARSE_OPT_NONEG was added but only prevents the --no-* boolean form,
> > not negative numeric arguments.
> >
> > This series restores the original invariant with stronger guarantees:
> >
> > 1/4 diff: reject negative values for --inter-hunk-context
> > Change type to unsigned int, switch to OPT_UNSIGNED.
> >
> > 2/4 diff: reject negative values for -U/--unified
> > Change type to unsigned int, add range check in callback.
> >
> > 3/4 xdiff: guard against negative context lengths
> > BUG() in xdl_get_hunk() as defense in depth.
> >
> > 4/4 parse-options: clarify PARSE_OPT_NONEG does not reject
> > negative numbers
> > Documentation fix.
> >
> >
> > The config variables diff.context and diff.interHunkContext have
> > always rejected negative values. This series brings the CLI options in line.
> >
> > Michael Montalbo (4):
> > diff: reject negative values for --inter-hunk-context
> > diff: reject negative values for -U/--unified
> > xdiff: guard against negative context lengths
> > parse-options: clarify PARSE_OPT_NONEG does not reject negative
> > numbers
> >
> > diff.c | 25 ++++++++++++++-----------
> > diff.h | 4 ++--
> > parse-options.h | 5 ++++-
> > t/t4032-diff-inter-hunk-context.sh | 6 ++++++
> > t/t4055-diff-context.sh | 5 +++++
> > xdiff/xemit.c | 16 ++++++++++++----
> > 6 files changed, 43 insertions(+), 18 deletions(-)
> >
> >
> > base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/2105
^ permalink raw reply
* [PATCH v4] submodule-config: fix reading submodule.fetchJobs
From: Saagar Jha via GitGitGadget @ 2026-05-10 3:50 UTC (permalink / raw)
To: git; +Cc: Pablo, Saagar Jha, Saagar Jha
In-Reply-To: <pull.2287.v3.git.git.1777816327842.gitgitgadget@gmail.com>
From: Saagar Jha <saagar@saagarjha.com>
update_clone_config_from_gitmodules() passes &max_jobs to
config_from_gitmodules(), but max_jobs is already a pointer. This causes
the config value to be written to the wrong address and get dropped.
Pass max_jobs directly.
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
---
submodule-config: fix reading submodule.fetchJobs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v4
Pull-Request: https://github.com/git/git/pull/2287
Range-diff vs v3:
1: 70fb2ede0f ! 1: 415c17f4bb submodule-config: fix reading submodule.fetchJobs
@@ t/t7406-submodule-update.sh: test_expect_success 'submodule update can be run in
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
-+ (cd super3 &&
-+ git config -f .gitmodules submodule.fetchJobs 67 &&
-+ GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
-+ grep "67 tasks" trace.out
-+ )
++ git -C super3 config -f .gitmodules submodule.fetchJobs 67 &&
++ GIT_TRACE="$(pwd)/trace.out" git -C super3 submodule update --init &&
++ test_grep "67 tasks" trace.out
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
submodule-config.c | 2 +-
t/t7406-submodule-update.sh | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1f19fe2077..57b190678e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 3adab12091..6abb00876a 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1055,6 +1055,14 @@ test_expect_success 'submodule update can be run in parallel' '
)
'
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
+ git -C super3 config -f .gitmodules submodule.fetchJobs 67 &&
+ GIT_TRACE="$(pwd)/trace.out" git -C super3 submodule update --init &&
+ test_grep "67 tasks" trace.out
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
test_when_finished "rm -rf super4" &&
GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
From: Torsten Bögershausen @ 2026-05-10 9:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git,
Derrick Stolee, Jeff King
In-Reply-To: <xmqqik8w9eb8.fsf@gitster.g>
>
> Hmph, I do not think there is "up-cast" to keep. And we do not
> dereference a random pointer that would be suitable for unsigned
> char * as if it were "unsigned long *" or "size_t *" in this code.
>
> This came from commit 48fb7deb5bbd87933e7d314b73d7c1b52667f80f
>
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed Jun 17 17:22:27 2009 -0700
>
> Fix big left-shifts of unsigned char
>
> Shifting 'unsigned char' or 'unsigned short' left can result in sign
> extension errors, since the C integer promotion rules means that the
> unsigned char/short will get implicitly promoted to a signed 'int' due to
> the shift (or due to other operations).
>
> This normally doesn't matter, but if you shift things up sufficiently, it
> will now set the sign bit in 'int', and a subsequent cast to a bigger type
> (eg 'long' or 'unsigned long') will now sign-extend the value despite the
> original expression being unsigned.
>
> One example of this would be something like
>
> unsigned long size;
> unsigned char c;
>
> size += c << 24;
>
> where despite all the variables being unsigned, 'c << 24' ends up being a
> signed entity, and will get sign-extended when then doing the addition in
> an 'unsigned long' type.
>
>
> You could rewrite Linus's example to
>
> unsigned char *cp;
> unsigned long size;
> unsigned char c;
>
> c = *cp;
> size += ((unsigned long)c) << 24;
>
> While I am sympathetic to that position, I also would not mind
>
> unsigned char *cp;
> unsigned long size;
> unsigned long c;
>
> c = *cp;
> size += c << 24;
>
> all that much. In any case, such a "clean-up" has little to do with
> the topic under discussion, and itshould be discussed separately on
> its own merit, most likely when the dust settles after this topic
> lands. Let's not contaminate the patches that is "a trivial rewrite
> that is so obviously correct to fix the assumption that ulong and
> size_t are of the same size everywhere" with unrelated clean-up.
>
> Thanks.
>
Sorry for the confusion and noise.
My brain insisted to read
c = *cp; // fetch 8 bits from memory, upcast to unsigned long
as if we have written
c = *(long*)cp; // fetch 32/64 bits from memory
which is a completely different thing.
In short: all is good.
Thanks for digging and the patience.
^ permalink raw reply
* [PATCH] sideband: clear full line when printing remote messages
From: René Scharfe @ 2026-05-10 12:42 UTC (permalink / raw)
To: Chris Torek, Hugo Osvaldo Barrera; +Cc: git
In-Reply-To: <CAPx1Gvf5Vts3oS2BdFQ4PpCR-UY=5cYW7fgOkRuQpi8ug2JXDg@mail.gmail.com>
demultiplex_sideband() can write its remote output over active local
progress lines. That's why it has been using ANSI code Erase in Line on
smart terminals to clear the remainder of lines it writes since
ebe8fa738d (fix display overlap between remote and local progress,
2007-11-04).
This erases the last character of remote lines that span the full width
of the terminal, though, as the cursor is stuck at the rightmost column
for them. It's the same effect as in the following command, which
clears the 1 and shows just the leading zeros:
$ EL="\033[K"
$ printf "%0${COLUMNS}d${EL}\n" 1
If we move the ANSI code to the start we get to see the 1 as well:
$ printf "${EL}%0${COLUMNS}d\n" 1
So do the same in demultiplex_sideband() and emit the ANSI code as a
prefix instead of a suffix to show messages in full even if they happen
to fill the whole width of a smart terminal.
Reported-by: Hugo Osvaldo Barrera <hugo@whynothugo.nl>
Suggested-by: Chris Torek <chris.torek@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
sideband.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/sideband.c b/sideband.c
index ea7c25211e..48ed4c8099 100644
--- a/sideband.c
+++ b/sideband.c
@@ -120,7 +120,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
#define DISPLAY_PREFIX "remote: "
-#define ANSI_SUFFIX "\033[K"
+#define ANSI_PREFIX "\033[K"
#define DUMB_SUFFIX " "
int demultiplex_sideband(const char *me, int status,
@@ -129,15 +129,18 @@ int demultiplex_sideband(const char *me, int status,
struct strbuf *scratch,
enum sideband_type *sideband_type)
{
- static const char *suffix;
+ static const char *prefix, *suffix;
const char *b, *brk;
int band;
if (!suffix) {
- if (isatty(2) && !is_terminal_dumb())
- suffix = ANSI_SUFFIX;
- else
+ if (isatty(2) && !is_terminal_dumb()) {
+ prefix = ANSI_PREFIX DISPLAY_PREFIX;
+ suffix = "";
+ } else {
+ prefix = DISPLAY_PREFIX;
suffix = DUMB_SUFFIX;
+ }
}
if (status == PACKET_READ_EOF) {
@@ -171,8 +174,7 @@ int demultiplex_sideband(const char *me, int status,
case 3:
if (die_on_error)
die(_("remote error: %s"), buf + 1);
- strbuf_addf(scratch, "%s%s", scratch->len ? "\n" : "",
- DISPLAY_PREFIX);
+ strbuf_addf(scratch, "%s%s", scratch->len ? "\n" : "", prefix);
maybe_colorize_sideband(scratch, buf + 1, len);
*sideband_type = SIDEBAND_REMOTE_ERROR;
@@ -203,7 +205,7 @@ int demultiplex_sideband(const char *me, int status,
strbuf_addstr(scratch, suffix);
if (!scratch->len)
- strbuf_addstr(scratch, DISPLAY_PREFIX);
+ strbuf_addstr(scratch, prefix);
/*
* A use case that we should not add clear-to-eol suffix
@@ -229,8 +231,8 @@ int demultiplex_sideband(const char *me, int status,
}
if (*b) {
- strbuf_addstr(scratch, scratch->len ?
- "" : DISPLAY_PREFIX);
+ if (!scratch->len)
+ strbuf_addstr(scratch, prefix);
maybe_colorize_sideband(scratch, b, strlen(b));
}
return 0;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] rebase: ignore non-branch update-refs
From: Phillip Wood @ 2026-05-10 13:37 UTC (permalink / raw)
To: Junio C Hamano, mail; +Cc: git, Derrick Stolee
In-Reply-To: <xmqqqznk9ih8.fsf@gitster.g>
On 10/05/2026 02:11, Junio C Hamano wrote:
> mail@abhinavg.net writes:
>
>> diff --git a/sequencer.c b/sequencer.c
>> index b7d8dca47f..25bcfc5da0 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -6428,6 +6428,16 @@ static int add_decorations_to_list(const struct commit *commit,
>> const char *path;
>> size_t base_offset = ctx->buf->len;
>>
>> + /*
>> + * The global decoration table may contain names loaded by
>> + * a previous pretty format such as "%d".
>> + * This will result in refs such as "HEAD" being present.
>> + */
>
> Your long topic branch may have local unannotated tags that point
> into the middle of it, marking strategic points in the topic.
>
> With this change, the command no longer moves them when it rebases
> the entire topic. Isn't it a regression?
sequencer.c:todo_list_add_update_ref_commands() calls
load_branch_decorations() so it does not update tags and the patch is
correct.
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.
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: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Derrick Stolee @ 2026-05-10 16:08 UTC (permalink / raw)
To: Taylor Blau, Jeff King; +Cc: jean-christophe manciot, Patrick Steinhardt, git
In-Reply-To: <af+snTGFeoUUyfPU@nand.local>
On 5/9/26 5:52 PM, Taylor Blau wrote:
> On Sat, May 09, 2026 at 01:52:49PM -0400, Jeff King wrote:
>> I don't think this affected the old "git gc --detach" because it takes
>> the lock after daemonizing[1]. We can't do the same here, though, since we
>> need to hold the lock for the foreground tasks. So either we need to
>> release and re-take the lock between foreground and background tasks, or
>> we need to teach the daemonize() function to update the "owner" field on
>> all of the tempfiles to the new child[2].
>
> I agree. We can't simply do what was done in 329e6e8794c (gc: save log
> from daemonized gc --auto and print it next time, 2015-09-19), for
> exactly the reason that you stated.
>
> Dropping and re-acquiring the lock is possible, but it's racy since
> there is a gap during the critical window while we fork(). I believe
> that the only airtight way to do this is to update the owner field of
> the tempfiles we want to pass down during daemonization.
I agree that this drop/reacquire pattern would not be a safe choice.
> (As an aside, do we want to do that for all tempfiles? It might be nice
> to have a "->reassign_on_fork" flag or something on the tempfile struct
> in case there are instances where the parent wants to retain ownership
> of the tempfile after fork()-ing, but I can't think of any off the top
> of my head. If we do introduce such a field, it should probably default
> to "true" to avoid any foot-guns.)
>
>> Ultimately fixing the lock bug will solve that. Though if doing so is
>> too complicated for a quick maint release, I'm tempted to say we should
>> consider reverting 452b12c2e0 for a potential v2.54.1 (as there were a
>> few other regression fixes so far, I assume we'll have one soon-ish).
>
> I think something like the following (untested) would do the trick:
>
> --- 8< ---
> diff --git a/setup.c b/setup.c
> index 7ec4427368a..c07aeac4f7d 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -22,6 +22,7 @@
> #include "chdir-notify.h"
> #include "path.h"
> #include "quote.h"
> +#include "tempfile.h"
> #include "trace.h"
> #include "trace2.h"
> #include "worktree.h"
> @@ -2162,12 +2163,17 @@ int daemonize(void)
> errno = ENOSYS;
> return -1;
> #else
> - switch (fork()) {
> + pid_t ppid = getpid();
> + pid_t pid;
> +
> + switch ((pid = fork())) {
> case 0:
> + reassign_tempfile_ownership(ppid, getpid());
> break;
> case -1:
> die_errno(_("fork failed"));
> default:
> + reassign_tempfile_ownership(ppid, pid);
> exit(0);
> }
> if (setsid() == -1)
> diff --git a/tempfile.c b/tempfile.c
> index 82dfa3d82f2..f0fdf582794 100644
> --- a/tempfile.c
> +++ b/tempfile.c
> @@ -373,3 +373,15 @@ int delete_tempfile(struct tempfile **tempfile_p)
>
> return err ? -1 : 0;
> }
> +
> +void reassign_tempfile_ownership(pid_t from, pid_t to)
> +{
> + volatile struct volatile_list_head *pos;
> +
> + list_for_each(pos, &tempfile_list) {
> + struct tempfile *p = list_entry(pos, struct tempfile, list);
> +
> + if (is_tempfile_active(p) && p->owner == from)
> + p->owner = to;
> + }
> +}
I worry that we're assigning _all_ tempfiles to the child in this
case, not just specific ones like the maintenance lock. But since
this is specifically called only in daemonize() then it may be
fine.
Is there any concern that since the child didn't create a tempfile
that the atexit() may not be initialized in the child process? Or
will that carry over from the fork() automatically? (This is hard
to test without something causing a die() in the detached process.)
> That's not too terrible to write, and I would feel OK about putting it
> in a 2.54.1 release soon-ish provided that others think it is reasonable.
I'd rather revert the geometric maintenance default for a point
release and let something more complicated like this percolate
until the next release window.
> Simply reverting 452b12c2e0 (builtin/maintenance: use "geometric"
> strategy by default, 2026-02-24) feels somewhat unsatisfying, since it
> is merely making the bug less likely rather than eliminating it
> entirely.
It does limit the behavior to those who previously chose to opt-in to
this behavior, and likely that's a low number or else we'd have more
bug reports.
> So in that sense I would prefer to "fix forward" here rather than to
> mask over the bug. But even the relatively short diff above is not so
> straightforward to reason through, review, or test, so I'm open to other
> ideas on how to proceed here.
I initially worried about cross-platform support, thinking that we
needed to pass file descriptors / handles and Windows always has
issues with file handles. But we aren't actually keeping a handle
open but instead a record that we created the lock and should delete
it when everything resolves.
For me to be convinced that this forward fix is the right direction,
I'd need to see a test that proves the detached process will clean up
the locks on a normal process end and an early exit.
Thanks,
-Stolee
^ permalink raw reply
* Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
From: Taylor Blau @ 2026-05-10 20:00 UTC (permalink / raw)
To: Derrick Stolee
Cc: Jeff King, jean-christophe manciot, Patrick Steinhardt, git
In-Reply-To: <9ddfd37d-7d71-4359-b9be-d993fbfd138c@gmail.com>
On Sun, May 10, 2026 at 12:08:14PM -0400, Derrick Stolee wrote:
> I worry that we're assigning _all_ tempfiles to the child in this
> case, not just specific ones like the maintenance lock. But since
> this is specifically called only in daemonize() then it may be
> fine.
Right, I had wondered about this above, and I'm not sure what the right
behavior is here. I think in our case its fine to pass all tempfiles
down to the child, since we're about to exit.
The only other spot that uses `daemonize()` is 'gc --detach' and 'git
daemon'. I think before considering something like this we would want to
reason through what the intended behavior of each of those callers is.
If we end up pursuing a fix in this direction, I think the safest thing
that we could do would be to have callers opt-in to this behavior so we
can do the right thing in git-maintenance, and other callers retain
their existing behavior.
> Is there any concern that since the child didn't create a tempfile
> that the atexit() may not be initialized in the child process? Or
> will that carry over from the fork() automatically? (This is hard
> to test without something causing a die() in the detached process.)
The child will have its own copy of the tempfile list as well as its own
copy of the atexit() handlers. POSIX (via atexit(3p)) says[1] that
atexit() behaves according to the ISO C standard:
The functionality described on this reference page is aligned with
the ISO C standard. Any conflict between the requirements described
here and the ISO C standard is unintentional. This volume of
POSIX.1‐2017 defers to the ISO C standard.
, and atexit(3) says[2]:
When a child process is created via fork(2), it inherits copies of
its parent's registrations. Upon a successful call to one of the
exec(3) functions, all registrations are removed.
So the child has an identical copy of both the atexit() handlers and the
tempfile list.
> > That's not too terrible to write, and I would feel OK about putting it
> > in a 2.54.1 release soon-ish provided that others think it is reasonable.
>
> I'd rather revert the geometric maintenance default for a point
> release and let something more complicated like this percolate
> until the next release window.
>
> > Simply reverting 452b12c2e0 (builtin/maintenance: use "geometric"
> > strategy by default, 2026-02-24) feels somewhat unsatisfying, since it
> > is merely making the bug less likely rather than eliminating it
> > entirely.
>
> It does limit the behavior to those who previously chose to opt-in to
> this behavior, and likely that's a low number or else we'd have more
> bug reports.
I agree with what you're saying. In an ideal world we would be able to
apply a fix on top that would prevent this bug from occurring, but if
that's not trivial to do then we shouldn't rush it in the 2.54.1 window.
I think that not having this bug whatsoever (as opposed to simply
reverting 452b12c2e0) would be preferable, but as you note we haven't
seen any bug reports in a pre-452b12c2e0 world, so perhaps the risk is
low enough that we could merely revert 452b12c2e0.
> For me to be convinced that this forward fix is the right direction,
> I'd need to see a test that proves the detached process will clean up
> the locks on a normal process end and an early exit.
Yeah, I agree. Testing this is a little tricky, but I played around
with it for a while and came up with the following:
--- 8< ---
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc18..77bfa537385 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1460,6 +1460,56 @@ test_expect_success '--detach causes maintenance to run in background' '
)
'
+test_expect_success PIPE '--detach holds maintenance lock until daemonized child exits' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ mkfifo fifo &&
+
+ git config maintenance.auto false &&
+ git config core.lockfilepid true &&
+
+ git remote add origin /does/not/exist &&
+ git config set remote.origin.uploadpack \
+ "echo \$PPID >child && cat \"$(pwd)/fifo\"" &&
+
+ # Start a detached prefetch maintenance task. Note that
+ # we are backgrounding git-maintenance here in order to
+ # determine its PID to validate that the lockfile was
+ # created by the parent.
+ { git maintenance run --task=prefetch --detach & } &&
+ parent="$!" &&
+
+ # Open fifo for writing, which will block until the
+ # upload-pack helper opens it for reading. Once exec
+ # returns, we know that the daemonized child is alive
+ # and pinned.
+ exec 8>fifo &&
+
+ test_path_is_file .git/objects/maintenance.lock &&
+ test_path_is_file .git/objects/maintenance~pid.lock &&
+
+ # Verify that the maintenance.lock still exists, and
+ # that it was created by the parent process, not the
+ # child.
+ echo "pid $parent" >expect &&
+ test_cmp expect .git/objects/maintenance~pid.lock &&
+
+ # Close the write end of the FIFO, causing our upload-pack
+ # helper to quit. Wait until the grandparent (from the
+ # perspective of our upload-pack helper, the daemonized
+ # git-maintenance child)
+ exec 8>&- &&
+ gpid="$(ps -o ppid= -p $(cat child) | tr -d " ")" &&
+ test -n $gpid && wait $gpid &&
+
+ test_path_is_missing .git/objects/maintenance.lock &&
+ test_path_is_missing .git/objects/maintenance~pid.lock
+ )
+'
+
test_expect_success 'repacking loose objects is quiet' '
test_when_finished "rm -rf repo" &&
git init repo &&
--- >8 ---
I'm not sure how portable it is, though. I think that 'ps -o ppid=' is
OK, since 'start_git_in_background()' uses it, but I'm not sure if $PPID
is available on Windows.
It reliably passes with the fix that I shared earlier in the thread, and
reliably fails without it.
Thanks,
Taylor
[1]: https://man7.org/linux/man-pages/man3/atexit.3p.html
[2]: https://man7.org/linux/man-pages/man3/atexit.3.html
^ permalink raw reply related
* [BUG] git-svn: 'Duplicate specification "id|i=s" for option "i"'
From: optik @ 2026-05-10 21:30 UTC (permalink / raw)
To: git
Fedora 43 aarch64
git version 2.54.0
Installed package: "git-2.54.0-1.fc43.aarch64"
I get this "warning" ("become a fatal error in a future") while using
git-svn meanwhile:
> Duplicate specification "id|i=s" for option "i"
This problem is here in the git code:
> my $rv = GetOptions(%opts, 'h|H' => \$_help, 'version|V' => \$_version,
> 'minimize-connections' =>
\$Git::SVN::Migration::_minimize,
> 'id|i=s' => \$Git::SVN::default_ref_id,
> 'svn-remote|remote|R=s' => sub {
> $Git::SVN::no_reuse_existing = 1;
> $Git::SVN::default_repo_id = $_[1] });
Link: https://github.com/git/git/blob/master/git-svn.perl#L352
Source is a change in Getopt:Long 2.55
Installed package: "util-linux-2.41.4-7.fc43.aarch64"
> * Fix long standing bug that duplicate options were not detected when
> the options differ in case while ignore_case is in effect.
> This will now yield a warning and become a fatal error in a future
> release.
Link: https://metacpan.org/dist/Getopt-Long/changes
Currently it still works as expectd, until getopt is changed
^ permalink raw reply
* Re: [PATCH v2] doc: git-log: clarify --follow options
From: Junio C Hamano @ 2026-05-10 21:31 UTC (permalink / raw)
To: Tamir Duberstein; +Cc: git, Jean-Noël Avila
In-Reply-To: <20260507-document-log-no-follow-v2-1-ee7bcbbe612f@gmail.com>
Tamir Duberstein <tamird@gmail.com> writes:
> Subject: Re: [PATCH v2] doc: git-log: clarify --follow options
The second ':' feels quite funny. I would have expected
doc: clarify "--follow" and log.follow for "git log"
or something like that.
> The --no-follow option was added by aebbcf5797 (diff: accept --no-follow
> option, 2012-09-21), but git-log(1) only documents the positive --follow
> form.
OK. Usually we document
--no-foo::
--foo::
describe '--foo' and '--no-foo' here ...
but we do not do so here, which is a good thng to fix.
> Document --no-follow alongside --follow. While here, describe --follow
> as limited to a single pathspec, rather than a single file, and mention
> the override in the log.follow documentation.
"Single file" is more accurate than "single pathspec", isn't it?
It is not like "git log --follow builtin" follows only changes to
the paths for builtin commands across "builtin-foo.c ->
builtin/foo.c" transition that happened at 81b50f3c (Move
'builtin-*' into a 'builtin/' subdirectory, 2010-02-22).
And the way the machinery for this checkbox feature works is to notice
when the file it was given disappears and then find the other file
that the file we have been following came from, and start following
that old file.
^ permalink raw reply
* Re: [PATCH] doc: fix typos via codespell
From: Junio C Hamano @ 2026-05-10 22:21 UTC (permalink / raw)
To: Andrew Kreimer; +Cc: git
In-Reply-To: <20260506101631.18127-1-algonell@gmail.com>
Andrew Kreimer <algonell@gmail.com> writes:
> There are some typos in the documentation, comments, etc.
> Fix them via codespell.
>
> Signed-off-by: Andrew Kreimer <algonell@gmail.com>
> ---
A few observations.
* It is a bit too much to be in a single patch to review.
* The fixes would apply to different vintages of the codebase.
especially the files in git-gui/ and po/ come from trees
maintained differently and patches to them need to be separate.
The spellos in *.po files stem from 96c0caf5 (Fix spelling errors in
messages shown to users, 2019-11-05) and .po files for other
languages have been updated to match the updated text in the code
since then, but not these two languages---if they are not maintained
since 2019, we need to wonder if they are worth keeping.
> Documentation/SubmittingPatches | 2 +-
> Documentation/git-sparse-checkout.adoc | 2 +-
> Documentation/technical/build-systems.adoc | 6 +++---
> builtin/pack-objects.c | 2 +-
> commit-graph.h | 2 +-
> compat/precompose_utf8.c | 2 +-
> git-gui/git-gui.sh | 2 +-
> git-gui/lib/choose_repository.tcl | 2 +-
> git-gui/lib/themed.tcl | 2 +-
> hook.h | 2 +-
> meson_options.txt | 2 +-
> midx-write.c | 4 ++--
> odb/source.h | 2 +-
> packfile.h | 2 +-
> path.h | 2 +-
> po/el.po | 2 +-
> po/ko.po | 2 +-
I've read through the patch to above files before I ran out of time
and concentration. Hopefully somebody else can do the remainder.
Thanks.
> reftable/system.h | 2 +-
> t/README | 2 +-
> t/chainlint.pl | 2 +-
> t/chainlint/chain-break-false.expect | 2 +-
> t/chainlint/chain-break-false.test | 2 +-
> t/t1700-split-index.sh | 2 +-
> t/t3909-stash-pathspec-file.sh | 6 +++---
> t/t4052-stat-output.sh | 2 +-
> t/t4067-diff-partial-clone.sh | 2 +-
> t/t9150/svk-merge.dump | 10 +++++-----
> t/t9151/svn-mergeinfo.dump | 18 +++++++++---------
> t/unit-tests/clar/README.md | 2 +-
> 29 files changed, 46 insertions(+), 46 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index d570184ec8..35b4952c8a 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -92,7 +92,7 @@ input and avoids unnecessary churn from many rapid iterations.
> topic are appropriate, so such an incremental updates are limited to
> small corrections and polishing. After a topic cooks for some time
> (like 7 calendar days) in 'next' without needing further tweaks on
> - top, it gets merged to the 'master' branch and wait to become part
> + top, it gets merged to the 'master' branch and waits to become part
> of the next major release.
>
> In the following sections, many techniques and conventions are listed
> diff --git a/Documentation/git-sparse-checkout.adoc b/Documentation/git-sparse-checkout.adoc
> index 0d1618f161..e286584c67 100644
> --- a/Documentation/git-sparse-checkout.adoc
> +++ b/Documentation/git-sparse-checkout.adoc
> @@ -134,7 +134,7 @@ the `clean.requireForce` config option is set to `false`.
> +
> The `--dry-run` option will list the directories that would be removed
> without deleting them. Running in this mode can be helpful to predict the
> -behavior of the clean comand or to determine which kinds of files are left
> +behavior of the clean command or to determine which kinds of files are left
> in the sparse directories.
> +
> The `--verbose` option will list every file within the directories that
> diff --git a/Documentation/technical/build-systems.adoc b/Documentation/technical/build-systems.adoc
> index 3c5237b9fd..ca5b5d96f1 100644
> --- a/Documentation/technical/build-systems.adoc
> +++ b/Documentation/technical/build-systems.adoc
> @@ -47,7 +47,7 @@ Auto-detection of the following items is considered to be important:
>
> - Check for the existence of headers.
> - Check for the existence of libraries.
> - - Check for the existence of exectuables.
> + - Check for the existence of executables.
> - Check for the runtime behavior of specific functions.
> - Check for specific link order requirements when multiple libraries are
> involved.
> @@ -106,7 +106,7 @@ by the build system:
>
> - C: the primary compiled language used by Git, must be supported. Relevant
> toolchains are GCC, Clang and MSVC.
> - - Rust: candidate as a second compiled lanugage, should be supported. Relevant
> + - Rust: candidate as a second compiled language, should be supported. Relevant
> toolchains is the LLVM-based rustc.
>
> Built-in support for the respective languages is preferred over support that
> @@ -142,7 +142,7 @@ The following list of build systems are considered:
>
> === GNU Make
>
> -- Platform support: ubitquitous on all platforms, but not well-integrated into Windows.
> +- Platform support: ubiquitous on all platforms, but not well-integrated into Windows.
> - Auto-detection: no built-in support for auto-detection of features.
> - Ease of use: easy to use, but discovering available options is hard. Makefile
> rules can quickly get out of hand once reaching a certain scope.
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index dd2480a73d..806068907e 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1341,7 +1341,7 @@ static void write_pack_file(void)
> * length of them as buffer length.
> *
> * Note that we need to subtract one though to
> - * accomodate for the sideband byte.
> + * accommodate for the sideband byte.
> */
> struct hashfd_options opts = {
> .progress = progress_state,
> diff --git a/commit-graph.h b/commit-graph.h
> index f6a5433641..13ca4ff010 100644
> --- a/commit-graph.h
> +++ b/commit-graph.h
> @@ -18,7 +18,7 @@
> * This method is only used to enhance coverage of the commit-graph
> * feature in the test suite with the GIT_TEST_COMMIT_GRAPH and
> * GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS environment variables. Do not
> - * call this method oustide of a builtin, and only if you know what
> + * call this method outside of a builtin, and only if you know what
> * you are doing!
> */
> void git_test_write_commit_graph_or_die(struct odb_source *source);
> diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
> index 43b3be0114..6e709bd138 100644
> --- a/compat/precompose_utf8.c
> +++ b/compat/precompose_utf8.c
> @@ -85,7 +85,7 @@ const char *precompose_string_if_needed(const char *in)
> out = reencode_string_iconv(in, inlen, ic_prec, 0, &outlen);
> if (out) {
> if (outlen == inlen && !memcmp(in, out, outlen))
> - free(out); /* no need to return indentical */
> + free(out); /* no need to return identical */
> else
> in = out;
> }
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..40e95bccb4 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -109,7 +109,7 @@ foreach p [split $env(PATH) $_path_sep] {
> if {[file pathtype $p] ne {absolute}} {
> continue
> }
> - # Keep only the first occurence of any duplicates.
> + # Keep only the first occurrence of any duplicates.
> set norm_p [file normalize $p]
> dict set _path_seen $norm_p 1
> }
> diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl
> index 7e1462a20c..a4703af028 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -15,7 +15,7 @@ field w_recentlist ; # Listbox containing recent repositories
> field w_localpath ; # Entry widget bound to local_path
>
> field done 0 ; # Finished picking the repository?
> -field clone_ok false ; # clone succeeeded
> +field clone_ok false ; # clone succeeded
> field local_path {} ; # Where this repository is locally
> field origin_url {} ; # Where we are cloning from
> field origin_name origin ; # What we shall call 'origin'
> diff --git a/git-gui/lib/themed.tcl b/git-gui/lib/themed.tcl
> index c18e201d85..f4cffeac66 100644
> --- a/git-gui/lib/themed.tcl
> +++ b/git-gui/lib/themed.tcl
> @@ -4,7 +4,7 @@
>
> namespace eval color {
> # Variable colors
> - # Preffered way to set widget colors is using add_option.
> + # Preferred way to set widget colors is using add_option.
> # In some cases, like with tags in_diff/in_sel, we use these colors.
> variable select_bg lightgray
> variable select_fg black
> diff --git a/hook.h b/hook.h
> index 5c5628dd1f..5f0c3f19bb 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -116,7 +116,7 @@ struct run_hooks_opt {
> * While the callback allows piecemeal writing, it can also be
> * used for smaller inputs, where it gets called only once.
> *
> - * Add hook callback initalization context to `feed_pipe_ctx`.
> + * Add hook callback initialization context to `feed_pipe_ctx`.
> * Add hook callback internal state to `feed_pipe_cb_data`.
> *
> */
> diff --git a/meson_options.txt b/meson_options.txt
> index 659cbb218f..1ed228d42a 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -106,7 +106,7 @@ option('highlight_bin', type: 'string', value: 'highlight')
>
> # Documentation.
> option('docs', type: 'array', choices: ['man', 'html'], value: [],
> - description: 'Which documenattion formats to build and install.')
> + description: 'Which documentation formats to build and install.')
> option('default_help_format', type: 'combo', choices: ['man', 'html', 'platform'], value: 'platform',
> description: 'Default format used when executing git-help(1).')
> option('docs_backend', type: 'combo', choices: ['asciidoc', 'asciidoctor', 'auto'], value: 'auto',
> diff --git a/midx-write.c b/midx-write.c
> index a25cab75ab..6d6d29c6cd 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -1152,7 +1152,7 @@ static bool midx_needs_update(struct multi_pack_index *midx, struct write_midx_c
>
> /*
> * Ensure that we have a valid checksum before consulting the
> - * exisiting MIDX in order to determine if we can avoid an
> + * existing MIDX in order to determine if we can avoid an
> * update.
> *
> * This is necessary because the given MIDX is loaded directly
> @@ -1438,7 +1438,7 @@ static int write_midx_internal(struct write_midx_opts *opts)
>
> /*
> * Attempt opening the pack index to populate num_objects.
> - * Ignore failiures as they can be expected and are not
> + * Ignore failures as they can be expected and are not
> * fatal during this selection time.
> */
> open_pack_index(oldest);
> diff --git a/odb/source.h b/odb/source.h
> index f706e0608a..4958a503cf 100644
> --- a/odb/source.h
> +++ b/odb/source.h
> @@ -338,7 +338,7 @@ static inline int odb_source_read_object_stream(struct odb_read_stream **out,
> * are only iterated over once.
> *
> * The optional `request` structure serves as a template for retrieving the
> - * object info for each indvidual iterated object and will be populated as if
> + * object info for each individual iterated object and will be populated as if
> * `odb_source_read_object_info()` was called on the object. It will not be
> * modified, the callback will instead be invoked with a separate `struct
> * object_info` for every object. Object info will not be read when passing a
> diff --git a/packfile.h b/packfile.h
> index 9b647da7dd..6dea707ba4 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -124,7 +124,7 @@ struct packfile_store {
> * that packs that contain a lot of accessed objects will be located
> * towards the front.
> *
> - * This is usually desireable, but there are exceptions. One exception
> + * This is usually desirable, but there are exceptions. One exception
> * is when the looking up multiple objects in a loop for each packfile.
> * In that case, we may easily end up with an infinite loop as the
> * packfiles get reordered to the front repeatedly.
> diff --git a/path.h b/path.h
> index 0434ba5e07..4c2958a903 100644
> --- a/path.h
> +++ b/path.h
> @@ -217,7 +217,7 @@ void safe_create_dir(struct repository *repo, const char *dir, int share);
> *
> * - It always adjusts shared permissions.
> *
> - * Returns a negative erorr code on error, 0 on success.
> + * Returns a negative error code on error, 0 on success.
> */
> int safe_create_dir_in_gitdir(struct repository *repo, const char *path);
>
> diff --git a/po/el.po b/po/el.po
> index 703f46d0c7..c45560c996 100644
> --- a/po/el.po
> +++ b/po/el.po
> @@ -2748,7 +2748,7 @@ msgid "Low-level Commands / Interrogators"
> msgstr "Εντολές Χαμηλού Επιπέδου / Ερωτημάτων"
>
> #: help.c:37
> -msgid "Low-level Commands / Synching Repositories"
> +msgid "Low-level Commands / Syncing Repositories"
> msgstr "Εντολές Χαμηλού Επιπέδου / Συγχρονισμού Αποθετηρίων"
>
> #: help.c:38
> diff --git a/po/ko.po b/po/ko.po
> index 7a6847f023..6bc20a43e3 100644
> --- a/po/ko.po
> +++ b/po/ko.po
> @@ -2062,7 +2062,7 @@ msgid "Low-level Commands / Interrogators"
> msgstr "보조 명령 / 정보 획득 기능"
>
> #: help.c:37
> -msgid "Low-level Commands / Synching Repositories"
> +msgid "Low-level Commands / Syncing Repositories"
> msgstr "보조 명령 / 저장소 동기화 기능"
>
> #: help.c:38
> diff --git a/reftable/system.h b/reftable/system.h
> index c0e2cbe0ff..628232a46f 100644
> --- a/reftable/system.h
> +++ b/reftable/system.h
> @@ -84,7 +84,7 @@ struct reftable_flock {
> * to acquire the lock. If `timeout_ms` is 0 we don't wait, if it is negative
> * we block indefinitely.
> *
> - * Retrun 0 on success, a reftable error code on error. Specifically,
> + * Return 0 on success, a reftable error code on error. Specifically,
> * `REFTABLE_LOCK_ERROR` should be returned in case the target path is already
> * locked.
> */
> diff --git a/t/README b/t/README
> index adbbd9acf4..085921be4b 100644
> --- a/t/README
> +++ b/t/README
> @@ -972,7 +972,7 @@ see test-lib-functions.sh for the full list and their options.
> - test_lazy_prereq <prereq> <script>
>
> Declare the way to determine if a test prerequisite <prereq> is
> - satisified or not, but delay the actual determination until the
> + satisfied or not, but delay the actual determination until the
> prerequisite is actually used by "test_have_prereq" or the
> three-arg form of the test_expect_* functions. For example, this
> is how the SYMLINKS prerequisite is declared to see if the platform
> diff --git a/t/chainlint.pl b/t/chainlint.pl
> index f0598e3934..2d07a99700 100755
> --- a/t/chainlint.pl
> +++ b/t/chainlint.pl
> @@ -35,7 +35,7 @@
> #
> # In other languages, `1+2` would typically be scanned as three tokens
> # (`1`, `+`, and `2`), but in shell it is a single token. However, the similar
> -# `1 + 2`, which embeds whitepace, is scanned as three token in shell, as well.
> +# `1 + 2`, which embeds whitespace, is scanned as three token in shell, as well.
> # In shell, several characters with special meaning lose that meaning when not
> # surrounded by whitespace. For instance, the negation operator `!` is special
> # when standing alone surrounded by whitespace; whereas in `foo!uucp` it is
> diff --git a/t/chainlint/chain-break-false.expect b/t/chainlint/chain-break-false.expect
> index f6a0a301e9..db6f8b12a4 100644
> --- a/t/chainlint/chain-break-false.expect
> +++ b/t/chainlint/chain-break-false.expect
> @@ -1,4 +1,4 @@
> -2 if condition not satisified
> +2 if condition not satisfied
> 3 then
> 4 echo it did not work...
> 5 echo failed!
> diff --git a/t/chainlint/chain-break-false.test b/t/chainlint/chain-break-false.test
> index f78ad911fc..924c9627c0 100644
> --- a/t/chainlint/chain-break-false.test
> +++ b/t/chainlint/chain-break-false.test
> @@ -1,6 +1,6 @@
> test_expect_success 'chain-break-false' '
> # LINT: broken &&-chain okay if explicit "false" signals failure
> -if condition not satisified
> +if condition not satisfied
> then
> echo it did not work...
> echo failed!
> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index ac4a5b2734..869fb4a14e 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -502,7 +502,7 @@ test_expect_success 'do not refresh null base index' '
> git checkout main &&
> git update-index --split-index &&
> test_commit more &&
> - # must not write a new shareindex, or we wont catch the problem
> + # must not write a new shareindex, or we won't catch the problem
> git -c splitIndex.maxPercentChange=100 merge --no-edit side-branch 2>err &&
> # i.e. do not expect warnings like
> # could not freshen shared index .../shareindex.00000...
> diff --git a/t/t3909-stash-pathspec-file.sh b/t/t3909-stash-pathspec-file.sh
> index 73f2dbdeb0..3afa6bff3d 100755
> --- a/t/t3909-stash-pathspec-file.sh
> +++ b/t/t3909-stash-pathspec-file.sh
> @@ -29,7 +29,7 @@ verify_expect () {
> test_expect_success 'simplest' '
> restore_checkpoint &&
>
> - # More files are written to make sure that git didnt ignore
> + # More files are written to make sure that git didn't ignore
> # --pathspec-from-file, stashing everything
> echo A >fileA.t &&
> echo B >fileB.t &&
> @@ -47,7 +47,7 @@ test_expect_success 'simplest' '
> test_expect_success '--pathspec-file-nul' '
> restore_checkpoint &&
>
> - # More files are written to make sure that git didnt ignore
> + # More files are written to make sure that git didn't ignore
> # --pathspec-from-file, stashing everything
> echo A >fileA.t &&
> echo B >fileB.t &&
> @@ -66,7 +66,7 @@ test_expect_success '--pathspec-file-nul' '
> test_expect_success 'only touches what was listed' '
> restore_checkpoint &&
>
> - # More files are written to make sure that git didnt ignore
> + # More files are written to make sure that git didn't ignore
> # --pathspec-from-file, stashing everything
> echo A >fileA.t &&
> echo B >fileB.t &&
> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> index 7c749062e2..df4999b326 100755
> --- a/t/t4052-stat-output.sh
> +++ b/t/t4052-stat-output.sh
> @@ -420,7 +420,7 @@ test_expect_success 'merge --stat respects COLUMNS with long name' '
> # enough terminal display width, will contain the following line:
> # "<RED>|<RESET> ${FILENAME} | 0"
> # where "<RED>" and "<RESET>" are ANSI escape codes to color the text.
> -# To calculate the minimium terminal display width MIN_TERM_WIDTH so that the
> +# To calculate the minimum terminal display width MIN_TERM_WIDTH so that the
> # FILENAME in the diffstat will not be shortened, we take the FILENAME length
> # and add 9 to it.
> # To check if the diffstat width, when the line_prefix (the "<RED>|<RESET>" of
> diff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh
> index 30813109ac..a9dec84c30 100755
> --- a/t/t4067-diff-partial-clone.sh
> +++ b/t/t4067-diff-partial-clone.sh
> @@ -159,7 +159,7 @@ test_expect_success 'diff succeeds even if prefetch triggered by break-rewrites'
> # We need baz to trigger break-rewrites detection.
> git -C client reset --hard HEAD &&
>
> - # break-rewrites detction in reset.
> + # break-rewrites detection in reset.
> git -C client reset HEAD~1
> '
>
> diff --git a/t/t9150/svk-merge.dump b/t/t9150/svk-merge.dump
> index 42f70dbec7..6a8ac81b11 100644
> --- a/t/t9150/svk-merge.dump
> +++ b/t/t9150/svk-merge.dump
> @@ -77,7 +77,7 @@ Content-length: 2411
> PROPS-END
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -206,7 +206,7 @@ Content-length: 2465
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -310,7 +310,7 @@ Content-length: 2521
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -417,7 +417,7 @@ Content-length: 2593
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -534,7 +534,7 @@ Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> diff --git a/t/t9151/svn-mergeinfo.dump b/t/t9151/svn-mergeinfo.dump
> index 47cafcf528..d5e1695637 100644
> --- a/t/t9151/svn-mergeinfo.dump
> +++ b/t/t9151/svn-mergeinfo.dump
> @@ -87,7 +87,7 @@ Content-length: 2411
> PROPS-END
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -260,7 +260,7 @@ Content-length: 2465
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -365,7 +365,7 @@ Content-length: 2521
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -473,7 +473,7 @@ Content-length: 2529
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -578,7 +578,7 @@ Content-length: 2593
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -767,7 +767,7 @@ Content-length: 2593
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -948,7 +948,7 @@ Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -1172,7 +1172,7 @@ Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> @@ -1414,7 +1414,7 @@ Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> # 1461501637330902918203684832716283019655932542976 hashes do not give you
> -# enough guarantees about no collisions between objects ever hapenning.
> +# enough guarantees about no collisions between objects ever happening.
> #
> # -DNSEC if you want git to care about sub-second file mtimes and ctimes.
> # Note that you need some new glibc (at least >2.2.4) for this, and it will
> diff --git a/t/unit-tests/clar/README.md b/t/unit-tests/clar/README.md
> index 41595989ca..a45b9c8e5d 100644
> --- a/t/unit-tests/clar/README.md
> +++ b/t/unit-tests/clar/README.md
> @@ -138,7 +138,7 @@ raise errors during test execution.
> __Caution:__ If you use assertions inside of `test_suitename__initialize`,
> make sure that you do not rely on `__initialize` being completely run
> inside your `test_suitename__cleanup` function. Otherwise you might
> -encounter ressource cleanup twice.
> +encounter resource cleanup twice.
>
> ## How does Clar work?
^ permalink raw reply
* Re: [PATCH] config.mak.dev: suppress C11 extension warning for Clang on Linux
From: Junio C Hamano @ 2026-05-10 22:21 UTC (permalink / raw)
To: Pablo; +Cc: Shardul Natu via GitGitGadget, git, Shnatu
In-Reply-To: <CAN5EUNRn+SqALbGR3KE9zUKxUfuJrqvK+XJcq-t=biTw56m8kg@mail.gmail.com>
Pablo <pabloosabaterr@gmail.com> writes:
> El jue, 7 may 2026 a las 4:16, Shardul Natu via GitGitGadget
> (<gitgitgadget@gmail.com>) escribió:
>>
>> From: Shnatu <snatu@google.com>
>>
>> When building Git with Clang on Linux with DEVELOPER=1, the build fails
>> because Clang treats C11 features used in glibc headers as extensions
>> and raises errors due to -std=gnu99, -pedantic, and -Werror.
>
> Hi Shnatu!
> This is already being discussed at:
> https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/T/#u
>
> You might want to check out that thread.
>
> Hope this helps,
Yes, they aim to solve the same issue, but the approach taken by
this patch to use -Wno-c11-extensions on clang may be with less
damage than the other approach that drops -std=gnu99 from Makefile.
The other approach uses the equivalent of this patch on the meson
side, so it may be doubly so that we should use -Wno-c11-extensions
on both build systems, no?
>> Specifically, glibc's string.h uses _Generic (a C11 feature) in macros
>> like strchr. When these macros are expanded in Git's C files, Clang
>> warns about them being C11 extensions.
>>
>> GCC does not exhibit this behavior because it suppresses pedantic
>> warnings for macros defined in system headers.
>>
>> To fix this, add -Wno-c11-extensions to DEVELOPER_CFLAGS when using
>> Clang, but restrict it to Linux (uname_S == Linux). This suppresses
>> the warning for glibc headers while keeping the build strict on other
>> platforms (like macOS) to catch accidental C11 usage in Git's own code.
>>
>> Signed-off-by: Shnatu <snatu@google.com>
>> ---
>> config.mak.dev: suppress C11 extension warning for Clang on Linux
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2291%2Fkiranani%2Fnext-2-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2291/kiranani/next-2-v1
>> Pull-Request: https://github.com/git/git/pull/2291
>>
>> config.mak.dev | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/config.mak.dev b/config.mak.dev
>> index c8dcf78779..f1dcf4329a 100644
>> --- a/config.mak.dev
>> +++ b/config.mak.dev
>> @@ -87,6 +87,9 @@ endif
>> # The bug was fixed in Apple clang 12.
>> #
>> ifneq ($(filter clang1,$(COMPILER_FEATURES)),) # if we are using clang
>> +ifeq ($(uname_S),Linux)
>> +DEVELOPER_CFLAGS += -Wno-c11-extensions
>> +endif
>> ifeq ($(uname_S),Darwin) # if we are on darwin
>> ifeq ($(filter clang12,$(COMPILER_FEATURES)),) # if version < 12
>> DEVELOPER_CFLAGS += -Wno-missing-braces
>>
>> base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2
>> --
>> gitgitgadget
>>
^ permalink raw reply
* Re: [PATCH v2] doc: git-log: clarify --follow options
From: Tamir Duberstein @ 2026-05-10 22:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jean-Noël Avila
In-Reply-To: <xmqqecjj9ckc.fsf@gitster.g>
On Sun, May 10, 2026 at 5:31 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tamir Duberstein <tamird@gmail.com> writes:
>
> > Subject: Re: [PATCH v2] doc: git-log: clarify --follow options
>
> The second ':' feels quite funny. I would have expected
>
> doc: clarify "--follow" and log.follow for "git log"
>
> or something like that.
>
> > The --no-follow option was added by aebbcf5797 (diff: accept --no-follow
> > option, 2012-09-21), but git-log(1) only documents the positive --follow
> > form.
>
> OK. Usually we document
>
> --no-foo::
> --foo::
> describe '--foo' and '--no-foo' here ...
>
> but we do not do so here, which is a good thng to fix.
>
> > Document --no-follow alongside --follow. While here, describe --follow
> > as limited to a single pathspec, rather than a single file, and mention
> > the override in the log.follow documentation.
>
> "Single file" is more accurate than "single pathspec", isn't it?
Yes, for the rename-following behavior.
The part that confused me is that `--follow` is not a no-op for a directory
pathspec. `git log --follow -- builtin` gives different output from `git log --
builtin`. But that is not because Git follows `builtin/` across the 81b50f3c
move to the old `builtin-*.c` paths.
The difference comes from the traversal mode. Setting `follow_renames` makes the
revision machinery run diffs and skip the usual pathspec pruning, because a
followed path may change. That can change which commits are shown for a
directory pathspec, especially merges. But the actual path rewrite in
`try_to_follow_renames()` only happens when a rename or copy destination exactly
matches the single pathspec, so a directory pathspec is not rewritten to earlier
file names.
I will reroll to say that `--follow` follows a single file beyond renames, works
only with exactly one pathspec, and that directory pathspecs do not follow
directory renames even though they still use the same traversal mode and can
therefore show a different set of commits. I will also fix the subject and
option ordering as suggested.
> It is not like "git log --follow builtin" follows only changes to
> the paths for builtin commands across "builtin-foo.c ->
> builtin/foo.c" transition that happened at 81b50f3c (Move
> 'builtin-*' into a 'builtin/' subdirectory, 2010-02-22).
>
> And the way the machinery for this checkbox feature works is to notice
> when the file it was given disappears and then find the other file
> that the file we have been following came from, and start following
> that old file.
^ permalink raw reply
* [PATCH v3] doc: clarify --follow and log.follow for git log
From: Tamir Duberstein @ 2026-05-10 22:31 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Junio C Hamano, Tamir Duberstein
In-Reply-To: <20260507-document-log-no-follow-v2-1-ee7bcbbe612f@gmail.com>
The --no-follow option was added by aebbcf5797 (diff: accept --no-follow
option, 2012-09-21), but git-log(1) only documents the positive --follow
form.
Later, 076c98372e (log: add "log.follow" configuration variable,
2015-07-07) taught git log to act as if --follow were given when
log.follow is true and there is a single pathspec, with --no-follow
overriding that default. 1e9250b5aa (diff-parseopt: convert
--[no-]follow, 2019-03-05) preserved the negated form while moving the
option to parse-options.
Document --no-follow alongside --follow. While here, make explicit that
--follow is accepted only with a single pathspec but follows only file
renames. A directory pathspec uses the same traversal mode and can show
a different set of commits, but directory renames are not followed.
Mention the override in the log.follow documentation.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v3:
- Retitle the patch to avoid the awkward `doc: git-log:` subject.
- List `--no-follow` before `--follow`.
- Clarify that `--follow` follows a single file across renames, even
though the option is accepted with exactly one pathspec.
- Document the directory-pathspec case: directory renames are not
followed, but `--follow` still uses file-follow traversal, disabling
normal pathspec pruning and possibly changing which commits,
especially merges, are shown.
- Link to v2: https://patch.msgid.link/20260507-document-log-no-follow-v2-1-ee7bcbbe612f@gmail.com
Changes in v2:
- Document --follow as limited to a single pathspec, not a single file.
- Adjust the log.follow documentation to use the same wording.
- Link to v1: https://patch.msgid.link/20260507-document-log-no-follow-v1-1-46ce02490eba@gmail.com
---
Documentation/config/log.adoc | 9 ++++++---
Documentation/git-log.adoc | 11 +++++++++--
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/Documentation/config/log.adoc b/Documentation/config/log.adoc
index f20cc25cd7..ba9872e98a 100644
--- a/Documentation/config/log.adoc
+++ b/Documentation/config/log.adoc
@@ -52,9 +52,12 @@ This is the same as the `--decorate` option of the `git log`.
`log.follow`::
If `true`, `git log` will act as if the `--follow` option was used when
- a single <path> is given. This has the same limitations as `--follow`,
- i.e. it cannot be used to follow multiple files and does not work well
- on non-linear history.
+ a single pathspec is given. This has the same limitations as
+ `--follow`, i.e. it cannot be used with multiple pathspecs and does not
+ work well on non-linear history. When the pathspec names a directory,
+ Git does not follow directory renames, but it still uses the same
+ traversal mode as for file rename following; see `--follow` in
+ linkgit:git-log[1]. This can be overridden by `--no-follow`.
`log.graphColors`::
A list of colors, separated by commas, that can be used to draw
diff --git a/Documentation/git-log.adoc b/Documentation/git-log.adoc
index e304739c5e..0fb3279d19 100644
--- a/Documentation/git-log.adoc
+++ b/Documentation/git-log.adoc
@@ -27,9 +27,16 @@ each commit introduces are shown.
OPTIONS
-------
+`--no-follow`::
`--follow`::
- Continue listing the history of a file beyond renames
- (works only for a single file).
+ Continue listing the history of a single file beyond renames.
+ This option works only when exactly one pathspec is given. If the
+ pathspec names a directory, Git does not follow directory renames,
+ but it still uses the same traversal mode as for file rename
+ following, which disables the usual pathspec pruning and can change
+ which commits, especially merges, are shown. `--no-follow`
+ disables this behavior, including when it was enabled by the
+ `log.follow` configuration variable.
`--no-decorate`::
`--decorate[=(short|full|auto|no)]`::
---
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
change-id: 20260507-document-log-no-follow-72c33dc15017
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply related
* [PATCH v3 0/1] rebase: ignore non-branch update-refs
From: mail @ 2026-05-10 22:41 UTC (permalink / raw)
To: git, Phillip Wood, gitster; +Cc: Abhinav Gupta, Derrick Stolee
In-Reply-To: <20260508015817.86177-1-mail@abhinavg.net>
From: Abhinav Gupta <mail@abhinavg.net>
Updated per suggestion to merge the conditionals.
Phillip wrote:
> On 10/05/2026 02:11, Junio C Hamano wrote:
> > Your long topic branch may have local unannotated tags that point
> > into the middle of it, marking strategic points in the topic.
> >
> > With this change, the command no longer moves them when it rebases
> > the entire topic. Isn't it a regression?
>
> sequencer.c:todo_list_add_update_ref_commands() calls
> load_branch_decorations() so it does not update tags and the patch is
> correct.
That's right, the documented contract is that only branches are updated.
Without '%d' triggering a load_ref_decorations,
load_branch_decorations would be called and only branch refs
would be added to the rebase todo list.
Phillip wrote:
> Looking at make_script_with_merges() it also calls
> load_branch_decorations() so we should probably add something like the
> diff below.
Thinking out loud:
Instead of caller-side filtering, another option might be
to replace load_branch_decorations with a branch-specialized iterator
that relies on load_ref_decorations and silently skips non-branch decorations.
That's a more invasive change, though.
Thanks!
Abhinav Gupta (1):
rebase: ignore non-branch update-refs
sequencer.c | 8 +++++++-
t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
2.54.0
^ permalink raw reply
* [PATCH v3 1/1] rebase: ignore non-branch update-refs
From: mail @ 2026-05-10 22:41 UTC (permalink / raw)
To: git, Phillip Wood, gitster; +Cc: Abhinav Gupta
In-Reply-To: <20260510224111.64467-1-mail@abhinavg.net>
From: Abhinav Gupta <mail@abhinavg.net>
The following Git configuration breaks git rebase --update-refs:
[rebase]
instructionFormat = %s%d
The '%d' format requests all available decorations for a commit,
filling the global decoration table with all of them,
which --update-refs then uses to populate 'update-ref' instructions
in the rebase todo list.
Specifically, this results in the following instruction:
update-ref HEAD
The todo parser then rejects the instruction:
error: update-ref requires a fully qualified refname e.g. refs/heads/HEAD
error: invalid line 3: update-ref HEAD
To fix, ignore decorations that are not local branches
when scanning through the table.
This matches the documented contract:
it moves branch refs under refs/heads/
and leaves display-only decorations (HEAD, tags, etc.) alone.
Verification:
A regression test that fails without this fix is included.
Signed-off-by: Abhinav Gupta <mail@abhinavg.net>
---
Updates:
v2: incorporate suggestions to simplify the test
v3: merge two if statements into one
sequencer.c | 8 +++++++-
t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index b7d8dca47f..ca3ea863d6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6431,8 +6431,14 @@ static int add_decorations_to_list(const struct commit *commit,
/*
* If the branch is the current HEAD, then it will be
* updated by the default rebase behavior.
+ * Exclude it from the list of refs to update,
+ * as well as any non-branch decorations.
+ * Non-branch decorations may be present if the pretty format
+ * includes "%d", which would have loaded all refs
+ * into the global decoration table.
*/
- if (head_ref && !strcmp(head_ref, decoration->name)) {
+ if ((head_ref && !strcmp(head_ref, decoration->name)) ||
+ (decoration->type != DECORATION_REF_LOCAL)) {
decoration = decoration->next;
continue;
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3e44562afa..58b3bb0c27 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1960,6 +1960,24 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '
)
'
+test_expect_success '--update-refs ignores non-branch decorations' '
+ test_when_finished "git branch -D update-refs" &&
+ test_when_finished "git checkout primary" &&
+ git checkout -B update-refs no-conflict-branch &&
+ (
+ set_cat_todo_editor &&
+
+ # rebase.instructionFormat=%d loads normal log decorations before
+ # --update-refs adds its branch placeholders so we must ignore
+ # all non-local decorations.
+ test_must_fail git -c rebase.instructionFormat="%s%d" \
+ rebase -i --update-refs HEAD^ >todo
+ ) &&
+ grep ^update-ref todo >actual &&
+ test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
+ test_cmp expect actual
+'
+
test_expect_success '--update-refs updates refs correctly' '
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] sideband: clear full line when printing remote messages
From: Junio C Hamano @ 2026-05-10 23:30 UTC (permalink / raw)
To: René Scharfe; +Cc: Chris Torek, Hugo Osvaldo Barrera, git
In-Reply-To: <9826dabf-c9a6-4397-8ae6-a24f9c507f1b@web.de>
René Scharfe <l.s.r@web.de> writes:
> demultiplex_sideband() can write its remote output over active local
> progress lines. That's why it has been using ANSI code Erase in Line on
> smart terminals to clear the remainder of lines it writes since
> ebe8fa738d (fix display overlap between remote and local progress,
> 2007-11-04).
>
> This erases the last character of remote lines that span the full width
> of the terminal, though, as the cursor is stuck at the rightmost column
> for them. It's the same effect as in the following command, which
> clears the 1 and shows just the leading zeros:
>
> $ EL="\033[K"
> $ printf "%0${COLUMNS}d${EL}\n" 1
>
> If we move the ANSI code to the start we get to see the 1 as well:
>
> $ printf "${EL}%0${COLUMNS}d\n" 1
>
> So do the same in demultiplex_sideband() and emit the ANSI code as a
> prefix instead of a suffix to show messages in full even if they happen
> to fill the whole width of a smart terminal.
Makes sense. The final objective is to make sure that leftover
letters near the end of line printed by previous "print" would not
remain after the material we are printing, so it does not matter if
we print and then erase the remainder or we erase the whole line and
print. And the latter is an obvious way to make it easier to reason
about in the presense of funkiness in the ways terminals behave
around the end of line.
^ permalink raw reply
* Re: [PATCH] rebase: ignore non-branch update-refs
From: Junio C Hamano @ 2026-05-10 23:37 UTC (permalink / raw)
To: Phillip Wood; +Cc: mail, git, Derrick Stolee
In-Reply-To: <0911df2d-aaa2-456e-a678-345239cefc67@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>> Your long topic branch may have local unannotated tags that point
>> into the middle of it, marking strategic points in the topic.
>>
>> With this change, the command no longer moves them when it rebases
>> the entire topic. Isn't it a regression?
>
> sequencer.c:todo_list_add_update_ref_commands() calls
> load_branch_decorations() so it does not update tags and the patch is
> correct.
OK. And with "%d", the existing versions of Git would have produced
something like
pick 31e8fcabd8 # rebase: update-refs (HEAD -> rebase, tag: mark)
update-ref HEAD
update-ref refs/heads/rebase
update-ref refs/tags/mark
it would have failed to work due to the "HEAD" thing, so even though
existing versions of Git may have added such local tags to the insn
sequence, it would not have been a workable configuration anyway.
OK. If we never supported such a workflow to use local tags as
markers, then the strategy taken by the posted patch to limit us to
local branch refs is a very good thing, I think.
^ permalink raw reply
* Re: [PATCH v2] doc: git-log: clarify --follow options
From: Junio C Hamano @ 2026-05-10 23:48 UTC (permalink / raw)
To: Tamir Duberstein; +Cc: git, Jean-Noël Avila
In-Reply-To: <CAJ-ks9nb1pebMLqZ+GunkXLSMYRb_RmpDuBDrDsgJ+6m7nbzMg@mail.gmail.com>
Tamir Duberstein <tamird@gmail.com> writes:
> I will reroll to say that `--follow` follows a single file beyond renames, works
> only with exactly one pathspec, and that directory pathspecs do not follow
> directory renames even though they still use the same traversal mode and can
> therefore show a different set of commits. I will also fix the subject and
> option ordering as suggested.
To be quite honest, the "--follow" option being what it is (i.e., a
checkbox option to claim we do support such an operation, without a
serious design and implementation), I'd rather see our documentation
being more honest and do not claim it works with pathspec at all.
When you use "--follow", you have to give a single filename, and
that file is followed across commits that renames it from some other
name, and then that file with the old name is followed.
If multiple histories are merged and if the file being followed
turns out to have come from different files on these different
histories, the "old name" the traversal is currently following is
not kept track of per traversal path, so we cannot expect the
feature to work with anything but a linear history, either.
^ permalink raw reply
* [PATCH] ci: enable EXPENSIVE for contributor builds
From: Junio C Hamano @ 2026-05-10 23:51 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin
In-Reply-To: <2159f6a271b06d156134392ce3c44fe957c83378.1778228209.git.gitgitgadget@gmail.com>
Earlier, we enabled EXPENSIVE tests for pushes to integration
branches. As we didn't have any CI jobs that run these tests, this
was a step in the right direction.
It however is an ineffective and inefficient use of the maintainer
time, which does not scale, to allow contributors to send changes
that are less tested at the list, only to force the maintainer
notice breakages caused by their changes but only after these
changes are mixed with changes from other contributors. The
problematic topic needs to be isolated by bisecting, and it
historically has been done by the maintainer alone.
It is far better to let the problem identified early, preferably
before the problematic code leaves the hands of the original
developer. In order for it to happen, the test coverage of the
contributor tests must be at least as wide as the coverage of the
integration tests.
Enable expensive tests for CI jobs triggered by pull requests. This
will make each contributor take care of their own, which scales much
better.
Keep the expensive tests also enabled for the pushes of integration
branches, as that is the only place we can notice problems stemming
from mismerges and inter-topic interactions, even if the topics from
the contributors in isolation all passes these tests.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is to be applied on top of "ci: run expensive tests on push
builds to integration branches", currently sitting at the tip of
the js/objects-larger-than-4gb-on-windows topic.
---
ci/lib.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index a671994bdf..4ca3ecef2c 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -314,11 +314,13 @@ export DEFAULT_TEST_TARGET=prove
export GIT_TEST_CLONE_2GB=true
export SKIP_DASHED_BUILT_INS=YesPlease
-# Enable expensive tests on push builds to integration branches, but
-# not on PR builds where the extra time is not justified for every
-# iteration.
+# In order to give maximum test coverage to contributor builds,
+# preferrably even before the changes consume public review bandwidth,
+# enable "expensive" tests for PR events.
+# In order to catch bugs introduced at integration time by mismerges,
+# enable the long tests for pushes to the integration branches as well.
case "$GITHUB_EVENT_NAME,$CI_BRANCH" in
-push,*next*|push,*master*|push,*main*|push,*maint*)
+pull_request,*|push,*next*|push,*master*|push,*main*|push,*maint*)
export GIT_TEST_LONG=YesPlease
;;
esac
--
2.54.0-162-gf1ca62098f
^ permalink raw reply related
* Re: [PATCH v2] doc: git-log: clarify --follow options
From: Tamir Duberstein @ 2026-05-10 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jean-Noël Avila
In-Reply-To: <xmqqqzni967o.fsf@gitster.g>
On Sun, May 10, 2026 at 7:48 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tamir Duberstein <tamird@gmail.com> writes:
>
> > I will reroll to say that `--follow` follows a single file beyond renames, works
> > only with exactly one pathspec, and that directory pathspecs do not follow
> > directory renames even though they still use the same traversal mode and can
> > therefore show a different set of commits. I will also fix the subject and
> > option ordering as suggested.
>
> To be quite honest, the "--follow" option being what it is (i.e., a
> checkbox option to claim we do support such an operation, without a
> serious design and implementation), I'd rather see our documentation
> being more honest and do not claim it works with pathspec at all.
> When you use "--follow", you have to give a single filename, and
> that file is followed across commits that renames it from some other
> name, and then that file with the old name is followed.
I certainly agree that being honest is the right thing to do - but the
honest truth is that `--follow` changes the behavior when used with
*any* pathspec, not just when given a single file. I attempted to
capture that nuance in v3.
>
> If multiple histories are merged and if the file being followed
> turns out to have come from different files on these different
> histories, the "old name" the traversal is currently following is
> not kept track of per traversal path, so we cannot expect the
> feature to work with anything but a linear history, either.
I'm not sure how to reply to this. The ground truth today is that the
option does have an effect when used with not-just-a-single-file, yet
the documentation does not mention this at all.
^ 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