* Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Kristofer Karlsson @ 2026-07-07 14:57 UTC (permalink / raw)
To: Taylor Blau; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak0D44nhSH/98WYD@nand.local>
On Tue, 7 Jul 2026 at 15:49, Taylor Blau <me@ttaylorr.com> wrote:
>
> I think that there is a more permanent fix, though, which would have not
> allowed this bug to evade both its author, and reviewer (me). I *think*
> that we may clear up some scoping issues if we removed g->topo_levels
> entirely, and instead stored it in the write_commit_graph_ctx struct.
>
> I haven't thought through the implications of doing so completely, so
> it's entirely possible that this idea is bunk for some other reason. But
> it was the first thing that came to mind, and so feels worth exploring
> to see if it might have prevented something like this from ever
> happening in the first place.
>
I looked into the structural change you suggested and I think
it's doable, though not quite as simple as just moving
it into ctx (since fill_commit_graph_info() doesn't have ctx).
I found three approaches:
(a) Thread topo_levels through the call chain. This would
affect:
- fill_commit_graph_info()
- fill_commit_in_graph()
- parse_commit_in_graph_one()
- parse_commit_in_graph()
- load_commit_graph_info()
- lookup_commit_in_graph().
This is the most direct approach, but it touches many functions
and some callers would need to pass in NULL which makes it a bit
noisy.
(b) Move topo_levels to struct object_database. Since
fill_commit_graph_info() can already reach the odb via
g->odb_source->odb, no signature changes are needed.
The write side becomes a single assignment:
ctx.r->objects->topo_levels = &topo_levels;
and cleanup becomes:
ctx.r->objects->topo_levels = NULL;
No chain walk needed and the diff is fairly small.
I am not sure about the semantics of it though -- should the odb
have a reference to topo_levels?
(c) Introduce a struct for the chain as a whole, separating it from
the per-layer struct commit_graph. Right now struct commit_graph
represents a single layer but also serves as the chain head, so
chain-wide state like topo_levels gets duplicated on every layer
(only logically -- the actual overhead is still small).
A dedicated chain struct could own topo_levels and the linked list
of layers. IMO this is the cleanest model but a larger refactoring.
I have a prototype of (b) that compiles and passes the test suite.
For now though, I think the minimal bugfix is the right thing to do.
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH v2 00/13] setup: split up repository discovery and setup
From: Justin Tobler @ 2026-07-07 15:02 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
In-Reply-To: <20260707-pks-setup-split-discovery-and-setup-v2-0-aab372cd227c@pks.im>
On 26/07/07 09:21AM, Patrick Steinhardt wrote:
> Changes in v2:
> - Expand commit message to talk about precedence order between
> the "GIT_SHALLOW_FILE" environment variable and the "--shallow-file"
> command line switch.
> - Remove a now-unused parameter in `set_alternate_shallow_file()`.
> - Fix a typo.
> - Link to v1: https://patch.msgid.link/20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im
The changes in this version look good to me. Thanks.
-Justin
^ permalink raw reply
* Re: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*'
From: Patrick Steinhardt @ 2026-07-07 14:51 UTC (permalink / raw)
To: Marcelo Machado Lage; +Cc: git, Vinicius Lira de Freitas, Junio C Hamano
In-Reply-To: <CAO=vGZpMe3dxyzFVwR7BWBxaAZ-z9Kw3CqQ0kAe5ZZGSQszkzw@mail.gmail.com>
On Mon, Jul 06, 2026 at 12:00:00PM -0300, Marcelo Machado Lage wrote:
> Em sex., 3 de jul. de 2026 às 05:20, Patrick Steinhardt <ps@pks.im> escreveu:
> > On Thu, Jul 02, 2026 at 11:07:04AM -0300, Marcelo Machado Lage wrote:
> > > diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
> > > index 7614dfbd95..93d6b4c479 100755
> > > --- a/t/t9811-git-p4-label-import.sh
> > > +++ b/t/t9811-git-p4-label-import.sh
> > > @@ -62,9 +62,9 @@ test_expect_success 'basic p4 labels' '
> > >
> > > cd main &&
> > > git checkout TAG_F1_ONLY &&
> > > - ! test -f f2 &&
> > > + test_path_is_missing f2 &&
> > > git checkout TAG_WITH\$_SHELL_CHAR &&
> > > - test -f f1 && test -f f2 && test -f file_with_\$metachar &&
> > > + test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar &&
> >
> > While at it we could split this line into three lines -- it's getting
> > overly long, and we typically don't chain multiple commands on one line
> > nowadays.
>
> We'll do this for v2 as well and make it into a patch series to
> separate test interface modernization from formatting changes.
>
> While on this, there are some other places in the file where multiple
> commands in a && chain appear in a single line, e.g. in line 244:
> > p4 edit f2 && date >f2 && p4 submit -d "change" f2 &&
> Should we split these into multiple lines as well, even though they
> are under the 80 characters limit?
Sure, if you want to convert this into a patch series anyway then I
think it makes sense to adapt all such locations in this test suite.
Patrick
^ permalink raw reply
* Re: [PATCH 1/7] hash: use git_hash_init() consistently
From: Junio C Hamano @ 2026-07-07 14:39 UTC (permalink / raw)
To: Jeff King; +Cc: git, Patrick Steinhardt, brian m. carlson
In-Reply-To: <20260707050141.GA1288294@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> We'd like to add more logic to git_hash_init(), but many callers skip it
> and call algop->init_fn() directly. Let's make sure we're consistently
> using the wrapper by adding a coccinelle rule.
>
> Besides the coccinelle file itself, this is a purely mechanical
> conversion based on the patch it generates. There should be no bare
> init_fn() calls left (except for the one in the wrapper).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> It feels like the "expression ALGO" in the rule should be a
> "git_hash_algo", but I had trouble getting coccinelle to recognize all
> cases when I did that. Probably not worth digging too far into, as
> the presence of the git_hash_ctx type means we should never hit any
> false positives.
Thanks. May conversions do look simple and straight-forward, but
some look a bit curious.
> diff --git a/object-file.c b/object-file.c
> index e3c68cfb66..f292683c2d 100644
> --- a/object-file.c
> +++ b/object-file.c
> ...
> - algo->init_fn(c);
> - if (compat && compat_c)
> - compat->init_fn(compat_c);
> + git_hash_init(c, algo);
> + if (compat && compat_c) {
> + git_hash_init(compat_c, compat);
> + }
For example, it is a mystery how Coccinelle decided to add a pair of
braces around this single statement. It should be obvious that the
corresponding single statement in the original did not need one.
> diff --git a/rerere.c b/rerere.c
> index 8232542585..2e932439a4 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -438,8 +438,9 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz
> struct git_hash_ctx ctx;
> struct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;
> int has_conflicts = 0;
> - if (hash)
> - the_hash_algo->init_fn(&ctx);
> + if (hash) {
> + git_hash_init(&ctx, the_hash_algo);
> + }
Ditto.
^ permalink raw reply
* Re: [PATCH 0/7] git_hash_*() quality-of-life improvements
From: Patrick Steinhardt @ 2026-07-07 14:26 UTC (permalink / raw)
To: Jeff King; +Cc: git, brian m. carlson
In-Reply-To: <20260707045556.GA1288172@coredump.intra.peff.net>
On Tue, Jul 07, 2026 at 12:55:56AM -0400, Jeff King wrote:
> This implements the "idempotent git_hash_discard()" discussed in this
> subthread:
>
> https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/
>
> with associated cleanups.
>
> It should be applied on top of jk/hash-algo-leak-fixes.
Thanks, this was a pleasant read. I have two minor nits, but other than
that I'm happy with this series!
Patrick
^ permalink raw reply
* Re: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions
From: Patrick Steinhardt @ 2026-07-07 14:26 UTC (permalink / raw)
To: Jeff King; +Cc: git, brian m. carlson
In-Reply-To: <20260707050952.GG1288294@coredump.intra.peff.net>
On Tue, Jul 07, 2026 at 01:09:52AM -0400, Jeff King wrote:
> It only makes sense to call git_hash_update(), etc, on a hash context
> that has been initialized but not yet finalized or discarded. This is an
> unlikely error to make, but it's easy for us to catch it and complain.
>
> It's especially important because it would quietly "work" for many hash
> backends (like sha1dc, which is just manipulating some bytes) but would
> cause undefined behavior with others (like OpenSSL, which puts the
> context onto the heap). Checking the flag lets us catch problems
> consistently on every build.
>
> Note that we can't do the same for git_init_hash(). Even though it would
You probably mean `git_hash_init()`?
> cause a leak to call it twice (without an intervening final/discard),
> the point of the function is that the contents of the struct are
> undefined before the call. But calling it twice is an even less likely
> error to make, so not covering it is OK.
Right. We could of course enforce that the structure must be zeroed
before calling this function. But I agree that this would become quite
awkward.
Patrick
^ permalink raw reply
* Re: [PATCH 3/7] hash: document function pointers and wrappers
From: Patrick Steinhardt @ 2026-07-07 14:26 UTC (permalink / raw)
To: Jeff King; +Cc: git, brian m. carlson
In-Reply-To: <20260707050557.GC1288294@coredump.intra.peff.net>
On Tue, Jul 07, 2026 at 01:05:57AM -0400, Jeff King wrote:
> diff --git a/hash.h b/hash.h
> index 0a23ef4dfd..5686914b71 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -341,12 +334,40 @@ struct git_hash_algo {
> };
> extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];
>
> +/*
> + * Prepare an uninitialized hash context for use. You must eventually release
> + * the context with with git_hash_final() (or final_oid()) or by calling
s/with with/with/
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS
From: Kristofer Karlsson @ 2026-07-07 14:08 UTC (permalink / raw)
To: Taylor Blau; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak0DUx5Y/5y1OINz@nand.local>
On Tue, 7 Jul 2026 at 15:47, Taylor Blau <me@ttaylorr.com> wrote:
>
> > Add a test that demonstrates the problem: with a two-layer
> > split commit-graph, writing a new incremental layer for a
> > commit whose parent is in the base layer walks all the way
> > down to the root (7 steps for 5 base commits) instead of
> > reading the existing topo level and stopping immediately
> > (1 step).
>
> This paragraph only describes verbatim what is already included in the
> patch. I think we could easily do without it, but I do not feel so
> strongly about it.
I also don't feel strongly about it, I could remove it entirely.
> > + intmax_t steps = 0;
>
> Any reason that this should be signed? Obviously in practice, I don't
> think we're going to wrap around with a greater-than-INT_MAX number of
> commits here, but perhaps we would at the very least prefer uintmax_t.
>
> I guess trace2 only has a data_intmax() function, so perhaps the point
> is moot. Regardless, it seems that we would want to have a convenience
> wrapper to be able to print out unsigned integer values which are
> otherwise un-representable as signed integers.
Yes, my only rationale here was to match the type that
trace2_data_intmax expects - and as you say, it's very
unlikely that we'll need to use all bits anyway, and since
this is only used for testing and debugging, and overflows
would be noticed that way and would not affect general
correctness.
> Instead of writing "# BUG ..." and then an incorrect assertion, I
> would suggest that you write the assertion you expect:
>
> test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
>
> , but mark the test as "test_expect_failure".
I started with this actually and then changed my mind in order
to demonstrate exactly how the counter changed, not just that it
changed from failure to success. But I'd be happy to change this
too if needed - it would effectively reduce the second commit to
just the bugfix line and switching from test_expect_failure
to test_expect_success.
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Kristofer Karlsson @ 2026-07-07 14:02 UTC (permalink / raw)
To: Taylor Blau; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak0D44nhSH/98WYD@nand.local>
On Tue, 7 Jul 2026 at 15:49, Taylor Blau <me@ttaylorr.com> wrote:
>
> > g = prepare_commit_graph(ctx.r);
> > for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
> > - g->topo_levels = &topo_levels;
> > + chain->topo_levels = &topo_levels;
> >
>
> Looks obviously good.
>
> I think that there is a more permanent fix, though, which would have not
> allowed this bug to evade both its author, and reviewer (me). I *think*
> that we may clear up some scoping issues if we removed g->topo_levels
> entirely, and instead stored it in the write_commit_graph_ctx struct.
I think that sounds feasible, but it would be a larger change.
I wanted to keep this fix minimal and restore
the code to match the pre-regression state. I can maybe look
into a refactoring as followup (or help review someone elses
refactoring?), though I would also be happy just to get that
extra 4 seconds back on every fetch for now :)
Thanks,
Kristofer
^ permalink raw reply
* [PATCH v2] t1410-reflog.sh: avoid suppressing git's exit code in pipelines
From: Gatla Vishweshwar Reddy @ 2026-07-07 13:55 UTC (permalink / raw)
To: git; +Cc: Gatla Vishweshwar Reddy
In-Reply-To: <xmqqechf8ryu.fsf@gitster.g>
Piping git commands directly to wc -l suppresses the exit code of
git, hiding potential failures from the test suite. Capture the
output to a temporary file first, then count the lines separately
to preserve the exit code. Where the expected count is known ahead
of time, use test_stdout_line_count instead.
Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
t/t1410-reflog.sh | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index ce71f9a30a..8e018d172b 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -244,26 +244,30 @@ test_expect_success 'delete' '
test_tick &&
git commit -m tiger C &&
- HEAD_entry_count=$(git reflog | wc -l) &&
- main_entry_count=$(git reflog show main | wc -l) &&
-
- test $HEAD_entry_count = 5 &&
- test $main_entry_count = 5 &&
-
+ test_stdout_line_count = 5 git reflog &&
+ git reflog >reflog_output &&
+ HEAD_entry_count=$(wc -l <reflog_output) &&
+ test_stdout_line_count = 5 git reflog show main &&
+ git reflog show main >reflog_main_output &&
+ main_entry_count=$(wc -l <reflog_main_output) &&
git reflog delete main@{1} &&
git reflog show main > output &&
test_line_count = $(($main_entry_count - 1)) output &&
- test $HEAD_entry_count = $(git reflog | wc -l) &&
+ git reflog >reflog_output &&
+ test $HEAD_entry_count = $(wc -l <reflog_output) &&
! grep ox < output &&
main_entry_count=$(wc -l < output) &&
git reflog delete HEAD@{1} &&
- test $(($HEAD_entry_count -1)) = $(git reflog | wc -l) &&
- test $main_entry_count = $(git reflog show main | wc -l) &&
+ git reflog >reflog_output &&
+ test $(($HEAD_entry_count -1)) = $(wc -l <reflog_output) &&
+ git reflog show main >reflog_main_output &&
+ test $main_entry_count = $(wc -l <reflog_main_output) &&
- HEAD_entry_count=$(git reflog | wc -l) &&
+ git reflog >reflog_output &&
+ HEAD_entry_count=$(wc -l <reflog_output) &&
git reflog delete main@{07.04.2005.15:15:00.-0700} &&
git reflog show main > output &&
@@ -319,13 +323,12 @@ test_expect_success 'git reflog expire unknown reference' '
test_must_fail git reflog expire does-not-exist 2>stderr &&
test_grep "error: reflog could not be found: ${SQ}does-not-exist${SQ}" stderr
'
-
test_expect_success 'checkout should not delete log for packed ref' '
- test $(git reflog main | wc -l) = 4 &&
+ test_stdout_line_count = 4 git reflog main &&
git branch foo &&
git pack-refs --all &&
git checkout foo &&
- test $(git reflog main | wc -l) = 4
+ test_stdout_line_count = 4 git reflog main
'
test_expect_success 'stale dirs do not cause d/f conflicts (reflogs on)' '
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Taylor Blau @ 2026-07-07 13:49 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <f9c1482a76493520b948a2e918de7a5481fa1043.1783418384.git.gitgitgadget@gmail.com>
On Tue, Jul 07, 2026 at 09:59:43AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> diff --git a/commit-graph.c b/commit-graph.c
> index 4e39a048c4..c2a711cceb 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source,
>
> g = prepare_commit_graph(ctx.r);
> for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
> - g->topo_levels = &topo_levels;
> + chain->topo_levels = &topo_levels;
>
> if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
> ctx.changed_paths = 1;
Looks obviously good.
I think that there is a more permanent fix, though, which would have not
allowed this bug to evade both its author, and reviewer (me). I *think*
that we may clear up some scoping issues if we removed g->topo_levels
entirely, and instead stored it in the write_commit_graph_ctx struct.
I haven't thought through the implications of doing so completely, so
it's entirely possible that this idea is bunk for some other reason. But
it was the first thing that came to mind, and so feels worth exploring
to see if it might have prevented something like this from ever
happening in the first place.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS
From: Taylor Blau @ 2026-07-07 13:46 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <b865c2bcff53a32637aac426dd2c6ef4a4c27077.1783418384.git.gitgitgadget@gmail.com>
On Tue, Jul 07, 2026 at 09:59:42AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
>
> Add a step counter and trace2_data_intmax call to
> compute_reachable_generation_numbers() to make the cost of
> the generation number DFS observable. This exposes a
> regression introduced in 199d452758 (commit-graph: fix
> "filling in" topological levels, 2025-04-07) where
> incremental commit-graph writes re-walk the entire commit
> ancestry instead of reading topo levels from lower graph
> layers.
Makes sense.
> Add a test that demonstrates the problem: with a two-layer
> split commit-graph, writing a new incremental layer for a
> commit whose parent is in the base layer walks all the way
> down to the root (7 steps for 5 base commits) instead of
> reading the existing topo level and stopping immediately
> (1 step).
This paragraph only describes verbatim what is already included in the
patch. I think we could easily do without it, but I do not feel so
strongly about it.
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> commit-graph.c | 5 +++++
> t/t5324-split-commit-graph.sh | 28 ++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 801471a098..4e39a048c4 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1653,6 +1653,7 @@ static void compute_reachable_generation_numbers(
> {
> int i;
> struct commit_list *list = NULL;
> + intmax_t steps = 0;
Any reason that this should be signed? Obviously in practice, I don't
think we're going to wrap around with a greater-than-INT_MAX number of
commits here, but perhaps we would at the very least prefer uintmax_t.
I guess trace2 only has a data_intmax() function, so perhaps the point
is moot. Regardless, it seems that we would want to have a convenience
wrapper to be able to print out unsigned integer values which are
otherwise un-representable as signed integers.
That is outside the scope of your patch, though, so what you have
below here is fine in my opinion.
> + # BUG: topo levels from lower graph layers are not
> + # propagated, so the DFS re-walks from base-3 down to
> + # the root (7 steps) instead of reading topo levels
> + # from the existing graph (1 step).
> + test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
Instead of writing "# BUG ..." and then an incorrect assertion, I
would suggest that you write the assertion you expect:
test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
, but mark the test as "test_expect_failure".
Thanks,
Taylor
^ permalink raw reply
* AW: CVE-2026-55200 libssh2
From: Berner Martin @ 2026-07-07 13:25 UTC (permalink / raw)
To: 'Johannes Schindelin'; +Cc: 'git@vger.kernel.org'
In-Reply-To: <26531fd0-4a21-c8ef-84a9-25c871cde303@gmx.de>
Hello Johannes,
Thank you for clarifying that the use of Git in any way cannot lead to the use of libssh2 and that, therefore, the situation is not as critical as it initially seemed to me.
I apologize for the wording that may have sounded demanding. That was not my intention and was partly due to the translator. My days as a software developer are quite a long time ago, and the languages in which Git is written were not among those I worked with. As a result, my ability to contribute directly to the open-source community is rather limited. However, my employer supports the open-source community in other ways, so we are certainly not just beneficiaries.
Thank you also for your explanations regarding MSYS2.
Kind regards,
Martin
-----Ursprüngliche Nachricht-----
Von: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Gesendet: Dienstag, 7. Juli 2026 13:56
An: Berner Martin <martin.berner@qualitasag.ch>
Cc: 'git@vger.kernel.org' <git@vger.kernel.org>
Betreff: Re: CVE-2026-55200 libssh2
Hi Martin,
On Tue, 7 Jul 2026, Berner Martin wrote:
> The libssh2 library appears to be relevant in the Git for Windows build.
For some definition of "relevant" ;-)
In Git for Windows, `libssh2` is only used by `libcurl`, and the way Git
uses `libcurl`, there is no code path to using libssh2 functionality.
Therefore, I do not consider this critical enough to rush out a new Git
for Windows version with a fix.
Besides...
> Git depends on libcurl, and libcurl in turn depends on libssh2.
> However, even in the latest build, the version still appears to be
> 1.11.1, which I understand may be affected by vulnerability
> CVE-2026-55200.
>
> Is that correct? If so, when can a patched build be expected?
That language "when can a patched build be expected" can very, very easily
be perceived as quite entitled, and hence have the exact opposite effect
of what you intended. You might want to be more careful in the future when
you plan on not even offering to help while demanding work to be done in
an Open Source project.
Back to your question why Git for Windows still only includes v1.11.1 of
libssh2. The answer is rather trivial: MSYS2 (on which Git for Windows is
based through a healthy collaboration) includes only that version:
https://packages.msys2.org/base/mingw-w64-libssh2
And the reason for _that_ might be rooted in the fact that both the
repository as well as the website of libssh2 list that as the very latest
available version:
- https://github.com/libssh2/libssh2/releases/latest currently redirects
to https://github.com/libssh2/libssh2/releases/tag/libssh2-1.11.1
- https://libssh2.org/ says:
Download
libssh2 1.11.1, released on 2024-10-16. *link to Changelog*
Easy explanation, right?
Ciao,
Johannes
^ permalink raw reply
* Re: CVE-2026-55200 libssh2
From: Johannes Schindelin @ 2026-07-07 11:55 UTC (permalink / raw)
To: Berner Martin; +Cc: 'git@vger.kernel.org'
In-Reply-To: <ZR5P278MB19814B2CA717210492C13A73F0F02@ZR5P278MB1981.CHEP278.PROD.OUTLOOK.COM>
Hi Martin,
On Tue, 7 Jul 2026, Berner Martin wrote:
> The libssh2 library appears to be relevant in the Git for Windows build.
For some definition of "relevant" ;-)
In Git for Windows, `libssh2` is only used by `libcurl`, and the way Git
uses `libcurl`, there is no code path to using libssh2 functionality.
Therefore, I do not consider this critical enough to rush out a new Git
for Windows version with a fix.
Besides...
> Git depends on libcurl, and libcurl in turn depends on libssh2.
> However, even in the latest build, the version still appears to be
> 1.11.1, which I understand may be affected by vulnerability
> CVE-2026-55200.
>
> Is that correct? If so, when can a patched build be expected?
That language "when can a patched build be expected" can very, very easily
be perceived as quite entitled, and hence have the exact opposite effect
of what you intended. You might want to be more careful in the future when
you plan on not even offering to help while demanding work to be done in
an Open Source project.
Back to your question why Git for Windows still only includes v1.11.1 of
libssh2. The answer is rather trivial: MSYS2 (on which Git for Windows is
based through a healthy collaboration) includes only that version:
https://packages.msys2.org/base/mingw-w64-libssh2
And the reason for _that_ might be rooted in the fact that both the
repository as well as the website of libssh2 list that as the very latest
available version:
- https://github.com/libssh2/libssh2/releases/latest currently redirects
to https://github.com/libssh2/libssh2/releases/tag/libssh2-1.11.1
- https://libssh2.org/ says:
Download
libssh2 1.11.1, released on 2024-10-16. *link to Changelog*
Easy explanation, right?
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] sideband: allow ANSI SGR with colon-separated subfields
From: Johannes Schindelin @ 2026-07-07 11:45 UTC (permalink / raw)
To: grawity; +Cc: git, Mantas Mikulėnas, Junio C Hamano
In-Reply-To: <20260513070803.163546-1-grawity@nullroute.lt>
[-- Attachment #1: Type: text/plain, Size: 1837 bytes --]
Hi Mantas,
On Wed, 13 May 2026, grawity@nullroute.lt wrote:
> From: Mantas Mikulėnas <grawity@gmail.com>
>
> The SGR values used for 256-color formatting are officially defined to
> be a single field with :-separated subfields (e.g. "\e[1;38:5:XX;40m")
> despite the more common but kludgy use of separate values (which then
> become context-dependent and lead to misinterpretation by incompatible
> terminals).
>
> See also: https://github.com/ThomasDickey/xterm-snapshots/blob/6380a3eaed857c182ea6cfa78cd706966b2628d0/charproc.c#L2047-L2118
This change seems well-motivated and well-executed to me. Just in case
anybody was waiting for my objections, there ain't any coming ;-)
Ciao,
Johannes
>
> Signed-off-by: Mantas Mikulėnas <grawity@gmail.com>
> ---
> sideband.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sideband.c b/sideband.c
> index 04282a568e..6cf70ef6f6 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -163,6 +163,10 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)
> *
> * ESC [ [<n> [; <n>]*] m
> *
> + * where <n> can be either zero-length, or a decimal number, or a
> + * series of decimal numbers separated by a colon (for 256-color or
> + * true-color codes).
> + *
> * These are part of the Select Graphic Rendition sequences which
> * contain more than just color sequences, for more details see
> * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.
> @@ -210,7 +214,7 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)
> strbuf_add(dest, src, i + 1);
> return i;
> }
> - if (!isdigit(src[i]) && src[i] != ';')
> + if (!isdigit(src[i]) && src[i] != ':' && src[i] != ';')
> break;
> }
>
> --
> 2.54.0
>
>
^ permalink raw reply
* Re: [PATCH v2] config: retry acquiring config.lock, configurable via core.configLockTimeout
From: Johannes Schindelin @ 2026-07-07 11:39 UTC (permalink / raw)
To: Joerg Thalheim; +Cc: git, Junio C Hamano, Patrick Steinhardt
In-Reply-To: <f449d0db-0434-f870-c69f-793f2b096816@gmx.de>
Hi,
On Thu, 28 May 2026, Johannes Schindelin wrote:
> On Sun, 17 May 2026, Joerg Thalheim wrote:
>
> > I matched the core.filesRefLockTimeout naming rather than reusing
> > microsoft/git's core.configWriteLockTimeoutMS, but can switch if the
> > downstream compat matters more.
>
> I see that there is quite a bit of precedent for naming a config setting
> `*Timeout` and implying that it specifies milliseconds, e.g.
> https://git-scm.com/docs/git-config#Documentation/git-config.txt-corefilesRefLockTimeout
>
> In general, I am pretty wary of unit-less numbers [*1*], that's why I
> chose that "MS" suffix. However, the prior art in Git is clear, and I
> should not have missed it. Therefore, I have no objections against
> `core.configLockTimeout` as-is; I'll take care of providing a smooth
> upgrade path in Microsoft Git.
For the record: I meant this feedback as _supporting_ the patch. Now I see
it is stalled... I do not really see any reason for this to be blocked
from promoting to `next` and then `master`, though.
Ciao,
Johannes
^ permalink raw reply
* [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Kristofer Karlsson via GitGitGadget @ 2026-07-07 9:59 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2170.git.1783418384.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Fix a regression introduced in 199d452758 (commit-graph: fix
"filling in" topological levels, 2025-04-07) where the loop
propagating the topo_levels slab to each layer of the
commit-graph chain always assigned to `g->topo_levels`
(the topmost layer) instead of `chain->topo_levels` (the
current iteration variable).
This meant only the topmost layer had its topo_levels pointer
set. When compute_reachable_generation_numbers() ran for an
incremental write, commits parsed from lower layers had their
topo levels left at zero in the slab, since
fill_commit_graph_info() could not store them without the
pointer. The DFS then re-walked the entire commit ancestry
instead of stopping at commits with known levels.
On a repository with 2.78M commits and a multi-layer split
commit-graph, this caused a single incremental commit-graph
write to spend ~3.7 seconds in the generation DFS instead of
microseconds.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-graph.c | 2 +-
t/t5324-split-commit-graph.sh | 6 +-----
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 4e39a048c4..c2a711cceb 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source,
g = prepare_commit_graph(ctx.r);
for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
- g->topo_levels = &topo_levels;
+ chain->topo_levels = &topo_levels;
if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
ctx.changed_paths = 1;
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index f9c57760f4..9e5ab7dbd0 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -738,11 +738,7 @@ test_expect_success 'incremental write reads topo levels from all layers' '
GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
git commit-graph write --reachable --split=no-merge &&
- # BUG: topo levels from lower graph layers are not
- # propagated, so the DFS re-walks from base-3 down to
- # the root (7 steps) instead of reading topo levels
- # from the existing graph (1 step).
- test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
+ test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
)
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS
From: Kristofer Karlsson via GitGitGadget @ 2026-07-07 9:59 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2170.git.1783418384.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add a step counter and trace2_data_intmax call to
compute_reachable_generation_numbers() to make the cost of
the generation number DFS observable. This exposes a
regression introduced in 199d452758 (commit-graph: fix
"filling in" topological levels, 2025-04-07) where
incremental commit-graph writes re-walk the entire commit
ancestry instead of reading topo levels from lower graph
layers.
Add a test that demonstrates the problem: with a two-layer
split commit-graph, writing a new incremental layer for a
commit whose parent is in the base layer walks all the way
down to the root (7 steps for 5 base commits) instead of
reading the existing topo level and stopping immediately
(1 step).
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-graph.c | 5 +++++
t/t5324-split-commit-graph.sh | 28 ++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/commit-graph.c b/commit-graph.c
index 801471a098..4e39a048c4 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1653,6 +1653,7 @@ static void compute_reachable_generation_numbers(
{
int i;
struct commit_list *list = NULL;
+ intmax_t steps = 0;
for (i = 0; i < info->commits->nr; i++) {
struct commit *c = info->commits->items[i];
@@ -1671,6 +1672,7 @@ static void compute_reachable_generation_numbers(
int all_parents_computed = 1;
timestamp_t max_gen = 0;
+ steps++;
for (parent = current->parents; parent; parent = parent->next) {
repo_parse_commit(info->r, parent->item);
gen = info->get_generation(parent->item, info->data);
@@ -1694,6 +1696,9 @@ static void compute_reachable_generation_numbers(
}
}
}
+
+ trace2_data_intmax("commit-graph", info->r,
+ "generation-dfs-steps", steps);
}
static timestamp_t get_topo_level(struct commit *c, void *data)
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index 49a057cc2e..f9c57760f4 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -718,6 +718,34 @@ test_expect_success 'write generation data chunk when commit-graph chain is repl
)
'
+test_expect_success 'incremental write reads topo levels from all layers' '
+ git init topo-from-lower &&
+ (
+ cd topo-from-lower &&
+
+ for i in $(test_seq 5)
+ do
+ test_commit base-$i || return 1
+ done &&
+ git commit-graph write --reachable &&
+
+ test_commit extra &&
+ git commit-graph write --reachable --split=no-merge &&
+
+ git checkout base-3 &&
+ test_commit new-branch &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
+ git commit-graph write --reachable --split=no-merge &&
+
+ # BUG: topo levels from lower graph layers are not
+ # propagated, so the DFS re-walks from base-3 down to
+ # the root (7 steps) instead of reading topo levels
+ # from the existing graph (1 step).
+ test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
+ )
+'
+
test_expect_success 'temporary graph layer is discarded upon failure' '
git init layer-discard &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] commit-graph: fix topo_levels slab propagation regression
From: Kristofer Karlsson via GitGitGadget @ 2026-07-07 9:59 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson
When fetch.writeCommitGraph is enabled (or git maintenance runs after
fetch), an incremental commit-graph write computes generation numbers for
the newly added commits. For commits already in the graph, their topo levels
should be read from the existing layers, making the DFS proportional to the
number of new commits.
199d452758 (commit-graph: return the prepared commit graph from
prepare_commit_graph(), 2025-04-07), part of the ps/commit-graph-via-source
series [1], refactored the loop that propagates the topo_levels slab to each
layer of the commit-graph chain. The original code used a single variable
that advanced through the chain:
while (g) {
g->topo_levels = &topo_levels;
g = g->base_graph;
}
The refactored code introduced a separate iteration variable but did not
update the loop body to match:
for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
g->topo_levels = &topo_levels;
This always assigns to the topmost layer instead of the current one. The
other loops in the same refactoring all correctly use chain in their bodies:
for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
ctx.num_commit_graphs_before++;
for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
ctx.commit_graph_filenames_before[--i] = xstrdup(chain->filename);
With only the topmost layer having topo_levels set, fill_commit_graph_info()
cannot store topo levels for commits parsed from lower layers.
compute_reachable_generation_numbers() then sees GENERATION_NUMBER_ZERO for
those commits and re-walks their entire ancestry.
On a large repo with a 4-layer split commit-graph, the cost of a single
incremental commit-graph write drops from 4133ms to 233ms after the fix,
which directly impacts every git fetch when commit-graph maintenance is
enabled.
[1]
https://lore.kernel.org/git/aMNTELw0Wk8jWoPc@nand.local/T/#mb55b5f0e1ccf82d969ac1d8144c56ecf87b833e8
Kristofer Karlsson (2):
commit-graph: add trace2 instrumentation for generation DFS
commit-graph: propagate topo_levels slab to all chain layers
commit-graph.c | 7 ++++++-
t/t5324-split-commit-graph.sh | 24 ++++++++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2170%2Fspkrka%2Fkrka%2Ffix-topo-levels-slab-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2170/spkrka/krka/fix-topo-levels-slab-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2170
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v7 0/5] history: add squash subcommand to fold a range
From: Phillip Wood @ 2026-07-07 9:48 UTC (permalink / raw)
To: Harald Nordgren, phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Patrick Steinhardt,
Junio C Hamano, Matt Hunter
In-Reply-To: <CAHwyqnVxa34iGmFvL4Ujrc2dTbmtF+7j7b=q5v95y=-pzUf0NA@mail.gmail.com>
Hi Harald
On 07/07/2026 08:51, Harald Nordgren wrote:
>> There was some discussion [1] about making that the default and renaming
>> it - was that overlooked? If not it would be helpful to comment on those
>> discussions to explain why you don't think it is a good idea.
>
> Not overlooked, but I side-stepped it because the discussion died
> down, and yes I don't agree that it needs to be the default. I could
> have mentioned my thinking in the cover letter.
>
>>> now builds the same editor template git rebase -i shows
>>> for a squash (a combination of N commits banner with each folded message
>>> under its own header) and follows autosquash for markers: a fixup!
>>> message falls out (commented under a will be skipped header), while a
>>> squash! or amend! keeps its body with only the marker subject commented
>>> so its remark can be reworded in. Only the message text is affected,
>>> every commit's changes are always folded in.
>>
>> Rebase re-orders commits so that fixups immediately follow their target
>> - do you do that here? I think that is very relevant because here we may
>> be dealing with several different commits each being targeted by a set
>> of fixups and presenting them mixed together will be confusing.
>
> No, I'm not doing that now, but I can take a look at that.
That's great, it is fine to punt things like this which require quite a
bit of work to implement to a later re-roll but please be clear in the
cover letter so reviewers know what to expect.
>> I think it should allow squashing a bunch of fixups together though. I
>> thought there was a plan [3] to refuse to squash a fixup unless the
>> range included its target.
>
> I attempted this with reject_fixupish_oldest(), assuming only the
> first commit needs to be checked as not being a fixup/squash/amend.
>
> But now I realize that maybe we need to check all of the commits, and
> also check if the target is in the range or not. It just makes the
> logic a lot bigger.
Yes it is a bit more involved. If the first commit is a fixup! then we
should allow the user to squash other fixups with the same target and
take the message from the last "amend!" commit if we see one. If there
are other commits it the range then we should refuse to squash as you do
here.
If the first target is not a fixup then we should refuse fixup commits
whose target we have not seen. As well as exact subject matches "git
rebase" accepts prefix matches and "fixup! $objectid". I think it is
fine to skip the prefix matches to start with here. The $objectid
matches shouldn't be too much extra work and I think they are worth
supporting because if I remember correctly git-gui creates them. Another
gotcha is that fixuping up a fixup prepends a "fixup!" to the subject
line so you need to be able to handle things like
fixup! fixup! the real target
fixup! amend! the real target
squash! fixup! the real target
etc. Hopefully looking at the code that handles fixups in the sequencer
will help
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v7 0/5] history: add squash subcommand to fold a range
From: Phillip Wood @ 2026-07-07 9:30 UTC (permalink / raw)
To: Harald Nordgren, phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Patrick Steinhardt,
Junio C Hamano, Matt Hunter
In-Reply-To: <CAHwyqnVd2OsmD-Y4YKVr9GsYdHRRNDot5EKSSESoRM-mf82YSg@mail.gmail.com>
Hi Harald
On 07/07/2026 09:55, Harald Nordgren wrote:
>>> The range-diff does not show any input sanitization - what happens when
>>> the user passes "--reverse" for example? As I said in [4] we should copy
>>> what "git replay" does to sanity check the rev-list options, otherwise
>>> we've got no idea whether the parent of the first commit returned by
>>> get_revision() is the commit we want to use as the parent of the
>>> squashed commit.
>>
>> Yeah, good point.
>
> Well, the code already blocks "--reverse" and other unknown options,
> but I can clarify that better in the commit message.
Well it accepts
git history squash -- --reverse ...
because after calling parse_options() everything after the "--" is
passed to setup_revisions(). There was some discussion about accepting
rev-list options [2] so it would have been helpful to reference that in
the cover letter. The cover letter should explain both the changes you
have made and the suggestions that were discussed that have not been
implemented so readers can get an overview of how this version relates
to the previous discussion. Without that it is impossible to know if you
disagree with a suggestion or have just forgotten it.
"git replay" supports arbitrary rev-list options by passing
PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT
to parse_options(), then passing the remaining options to
setup_revisions(). After that it checks the various members of `struct
rev_info` that it cares about are still set appropriately.
Thanks
Phillip
[1] https://lore.kernel.org/git/xmqqzf0dwalx.fsf@gitster.g
^ permalink raw reply
* Re: [PATCH v7 0/5] history: add squash subcommand to fold a range
From: Harald Nordgren @ 2026-07-07 8:55 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Patrick Steinhardt,
Junio C Hamano, Matt Hunter
In-Reply-To: <CAHwyqnVxa34iGmFvL4Ujrc2dTbmtF+7j7b=q5v95y=-pzUf0NA@mail.gmail.com>
> > The range-diff does not show any input sanitization - what happens when
> > the user passes "--reverse" for example? As I said in [4] we should copy
> > what "git replay" does to sanity check the rev-list options, otherwise
> > we've got no idea whether the parent of the first commit returned by
> > get_revision() is the commit we want to use as the parent of the
> > squashed commit.
>
> Yeah, good point.
Well, the code already blocks "--reverse" and other unknown options,
but I can clarify that better in the commit message.
Harald
^ permalink raw reply
* Re: [PATCH GSoC v15 02/13] git-compat-util: add `strtoumax_szt()` with error handling
From: Pablo Sabater @ 2026-07-07 8:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, chandrapratap3519, chriscool, eric.peijian, jltobler,
karthik.188, peff, toon
In-Reply-To: <xmqqse62obwh.fsf@gitster.g>
El mié, 1 jul 2026 a las 19:30, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > From: Eric Ju <eric.peijian@gmail.com>
> >
> > We already have `strtoul_ui()` and similar functions that provide proper
> > error handling using `strtoul` from the standard library. However,
> > there isn't currently a variant that returns a `size_t`.
> >
> > Using `strtoul` is unreliable because `size_`t is platform-dependent,
> > `unsigned long` could be too big to fit into a `size_t` or too small to
> > hold a `size_t`.
>
> It is somehow annoying to see that the commit log desciption, which
> is *clearly* meant to be plaintext, is so heavily riddled with
> backquoted references to code/program symbols. Yes, `literal` is a
> correct way to format them in both AsciiDoc and Markdown, so we very
> much welcome them in our documentation, but not in proposed log
> messages.
>
> In any case, you dropped 't' in 'size_t' outside the pair of
> backquotes.
Ok, I'll drop the backquotes for the commit messages of this series.
>
> > Use `strtoumax` which returns a `uintmax_t` guaranteed to be at least as
>
> `strtoumax()`, as the convention you used above for strtoul_ui() is
> to suffix function names with ().
ACK.
>
> > large as `size_t`, add a range check against `SIZE_MAX` to prevent
> > `size_t` overflow.
>
> OK.
>
> > This variant is needed in a subsequent commit to enable returning a
> > `size_t` with proper error handling.
> >
> > Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> > Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> > git-compat-util.h | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index 8809776407..5ecce5bbd2 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -975,6 +975,26 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
> > return 0;
> > }
> >
> > +/*
> > + * Convert a string to a size_t using the standard library's strtoumax, with
> > + * additional error handling to ensure robustness.
> > + */
> > +static inline int strtoumax_szt(char const *s, int base, size_t *result)
> > +{
> > + uintmax_t uim;
> > + char *p;
> > +
> > + errno = 0;
> > + /* negative values would be accepted by strtoul */
> > + if (strchr(s, '-'))
> > + return -1;
>
> Hmph, wouldn't
>
> if (*s == '-' || !*s)
> return -1
>
> cut it? Since your call to strtoumax() checks that the string was
> parsed to the end by insisting *p is NUL?
>
> If you are trying to more explicitly insist that s[] has only
> digits, which may not be a bad idea, as that is what we generally
> expect, then
>
> if (!s[0] || s[strspn(s, "0123456789")])
> return -1;
>
> perhaps.
I like the idea of only digits but, even though in this series I only
use this function in base 10, I want the function to work in other
bases, that's why I left the base in the function signature instead of
hardcoding it. strspn(s, "0123456789") rejects bases >10 ("ff" for
base 16) while strtoumax does support higher ones.
I think that it would be better to explicitly reject what we don't
want similarly to "-":
if (!*s || isspace((unsigned char)*s) || *s == '-' || *s == '+')
return -1;
About that, strtoumax works fine with "+" and ignores starting
whitespaces, but for consistency (we reject "-" and whitespaces
between or at the end) rejecting whitespaces and +/- will be better
and make the caller format it correctly.
I'll do that for the next version.
>
> > + uim = strtoumax(s, &p, base);
> > + if ((errno || *p || p == s) || uim > SIZE_MAX)
> > + return -1;
>
> And with !s[0] upfront, we can discard (p==s) case from here. Other
> strto*() wrappers we have may need the "cannot be empty" check,
> because they do not need any upfront validation of s[] like we do
> here (we do so to reject negative numbers), but since we do need to
> check s[] before calling the system strto*() function anyway, it is
> OK to be different here from the others.
Agreed
>
> If uintmax_t and size_t are of the same width, then (SIZE_MAX < uim)
> becomes mathmatically impossible, but hopefully no compiler or
> static checker is stupid enough to warn against it.
>
> > + *result = uim;
> > + return 0;
> > +}
> > +
> > static inline int strtol_i(char const *s, int base, int *result)
> > {
> > long ul;
Thanks for the feedback,
Pablo
^ permalink raw reply
* CVE-2026-55200 libssh2
From: Berner Martin @ 2026-07-07 8:38 UTC (permalink / raw)
To: 'git@vger.kernel.org'
Hello,
The libssh2 library appears to be relevant in the Git for Windows build. Git depends on libcurl, and libcurl in turn depends on libssh2.
However, even in the latest build, the version still appears to be 1.11.1, which I understand may be affected by vulnerability CVE-2026-55200.
Is that correct? If so, when can a patched build be expected?
Kind regards,
Martin Berner
^ permalink raw reply
* Re: [PATCH v7 0/5] history: add squash subcommand to fold a range
From: Harald Nordgren @ 2026-07-07 7:51 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Patrick Steinhardt,
Junio C Hamano, Matt Hunter
In-Reply-To: <5a5dbfae-4525-4b00-9e44-936be606ee85@gmail.com>
> There was some discussion [1] about making that the default and renaming
> it - was that overlooked? If not it would be helpful to comment on those
> discussions to explain why you don't think it is a good idea.
Not overlooked, but I side-stepped it because the discussion died
down, and yes I don't agree that it needs to be the default. I could
have mentioned my thinking in the cover letter.
> > now builds the same editor template git rebase -i shows
> > for a squash (a combination of N commits banner with each folded message
> > under its own header) and follows autosquash for markers: a fixup!
> > message falls out (commented under a will be skipped header), while a
> > squash! or amend! keeps its body with only the marker subject commented
> > so its remark can be reworded in. Only the message text is affected,
> > every commit's changes are always folded in.
>
> Rebase re-orders commits so that fixups immediately follow their target
> - do you do that here? I think that is very relevant because here we may
> be dealing with several different commits each being targeted by a set
> of fixups and presenting them mixed together will be confusing.
No, I'm not doing that now, but I can take a look at that.
> I think it should allow squashing a bunch of fixups together though. I
> thought there was a plan [3] to refuse to squash a fixup unless the
> range included its target.
I attempted this with reject_fixupish_oldest(), assuming only the
first commit needs to be checked as not being a fixup/squash/amend.
But now I realize that maybe we need to check all of the commits, and
also check if the target is in the range or not. It just makes the
logic a lot bigger.
> The range-diff does not show any input sanitization - what happens when
> the user passes "--reverse" for example? As I said in [4] we should copy
> what "git replay" does to sanity check the rev-list options, otherwise
> we've got no idea whether the parent of the first commit returned by
> get_revision() is the commit we want to use as the parent of the
> squashed commit.
Yeah, good point.
Harald
^ 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