* Re: [PATCH v3 0/3] doc: config: fix AsciiDoc glitches
From: Junio C Hamano @ 2026-06-12 15:05 UTC (permalink / raw)
To: Jeff King; +Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <20260612045329.GA593075@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Jun 11, 2026 at 07:19:43PM +0300, Tuomas Ahola wrote:
>
>> Tuomas Ahola (3):
>> doc: config: terminate runaway lists
>> doc: config/sideband: fix description list delimiter
>> doc: git-config: escape erroneous highlight markup
>
> Thanks, this v3 looks good to me.
Yup this one nicely sidesteps the yucky \# thing, which is very
good.
Thanks.
^ permalink raw reply
* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Derrick Stolee @ 2026-06-12 15:04 UTC (permalink / raw)
To: Kristofer Karlsson; +Cc: git
In-Reply-To: <CAL71e4OmPzpCXh-zZ8NsT6L4zVKnXV1gqiFZ2w0XgMJhD=LArQ@mail.gmail.com>
On 6/12/2026 10:32 AM, Kristofer Karlsson wrote:
> The required condition must then not be simply "one side exhausted".
> The walk must also continue while non-stale P1|P2 commits remain in the
> queue, since those still need STALE propagation - they are still
> merge-base candidates.
>
> So the actual halt condition would be:
>
> no non-stale P1|P2 candidates in the queue
> AND (no pure-P1 OR no pure-P2)
And since STALE is added only after both P1 and P2 bits, the two
conditions are identical to how queue_has_nonstale() terminates the
loop.
> If this reasoning is correct, then the walk only terminates after
> merge-base candidates have either been processed or marked STALE,
> and the counterexample should produce [B] rather than [B, C].
That's the correct distinction: we need the set [B] and not [B,C]
but we need to discover that B can reach C to remove it from the
result set.
I think there is potential merit in "switching walk modes" to DFS
when all queued commits have both P1 and P2, but it comes with a
lot of complications. So tread carefully if you go down this road.
Thanks,
-Stolee
^ permalink raw reply
* RE: [ANNOUNCE] Git v2.55.0-rc0
From: rsbecker @ 2026-06-12 15:03 UTC (permalink / raw)
To: rsbecker, 'Junio C Hamano', git
In-Reply-To: <065e01dcfa75$ade00690$09a013b0$@nexbridge.com>
On June 12, 2026 10:14 AM, I wrote:
> On June 11, 2026 11:32 AM, Junio wrote:
> > An early preview release Git v2.55.0-rc0 is now available for testing
> > at the usual places. It is comprised of 397 non-merge commits since
> > v2.54.0, contributed by
> > 70 people, 22 of which are new faces [*].
>
> Cargo is not available everywhere. Build is not possible on NonStop.
>
> cargo build --release
> /usr/coreutils/bin/bash: cargo: command not found
> Makefile:3021: recipe for target 'target/release/libgitcore.a' failed
> make: *** [target/release/libgitcore.a] Error 127
>
> Is there a way around this?
Note: cargo is part of Rust. This dependency was only supposed to be added
as of git 3.0. If I may be polite and ask that this dependency be removed for
v2.55.0-rc1.
While my team is trying to do the Rust port for NonStop, it is not a quick
effort, and we expect no earlier than 2027 - given that the team is made up
entirely of volunteers.
Thank you,
Randall
^ permalink raw reply
* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Junio C Hamano @ 2026-06-12 14:41 UTC (permalink / raw)
To: Weijie Yuan
Cc: Derrick Stolee, Kristofer Karlsson via GitGitGadget, git,
Kristofer Karlsson, Patrick Steinhardt
In-Reply-To: <aivQv5FkTEWDn22i@wyuan.org>
Weijie Yuan <wy@wyuan.org> writes:
> On Thu, Jun 11, 2026 at 10:48:18AM -0700, Junio C Hamano wrote:
>> I wonder if we should talk about it in the SubmittingPatches and/or
>> MyFirstContribution document?
>
> Hi, I think it might be a good idea to cover these details in
> MyFirstContribution, then cross-reference them from the part of
> SubmittingPatches that discusses sending a new version.
Sorry to be nitpicky, but the above is omitting too much from your
quote. "it" in "talk about it" is totally unclear to a reader who
haven't seen the message you are replying to.
> Also, for the part about sending a new version, I wonder whether it
> would be better to summarize and fold in Patrick's explanation here,
> thank you Patrick:
Yup, that is a great example.
> From: Patrick Steinhardt <ps@pks.im>
> Message-ID: <aietF4BX1Ewt3cpG@pks.im>
^ permalink raw reply
* [PATCH] Add a test about broken notes handling on rebase
From: Uwe Kleine-König @ 2026-06-12 14:39 UTC (permalink / raw)
To: git
When a commit disappears during rebase because the patch content is
already there (but not by the same patch in which case the commit would
be skipped) the notes of that disappearing commit still survives and is
added to the (rebased) parent of the disappearing commit.
So with the commit graph
A -- B -- C
`
`-BD
where BD includes the changes done in B, when rebasing C on top of BD,
the note for B should disappear and not be added to BD.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,
this is a behaviour of git that really bothers me when working on big
patch series. I use notes to track the Message-Id of the patches when I
send them out. Then when rebasing to a newer upstream version, the
tracking gets confused because the Message-Id notes end up on commits
that were not sent out yet (or I got two Message-Ids in them).
I reported that already back in 2023[1], but obviously not in a way that
resulted in a fix. So I'm trying again with a patch that adds a failing
test.
Best regards
Uwe
[1] https://lore.kernel.org/git/20230530092155.3zbb5uxa7eisdzxb@pengutronix.de/
t/meson.build | 1 +
t/t3322-notes-rebase.sh | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
create mode 100644 t/t3322-notes-rebase.sh
diff --git a/t/meson.build b/t/meson.build
index c5832fee0535..6927bd9c794f 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -358,6 +358,7 @@ integration_tests = [
't3311-notes-merge-fanout.sh',
't3320-notes-merge-worktrees.sh',
't3321-notes-stripspace.sh',
+ 't3322-notes-rebase.sh',
't3400-rebase.sh',
't3401-rebase-and-am-rename.sh',
't3402-rebase-merge.sh',
diff --git a/t/t3322-notes-rebase.sh b/t/t3322-notes-rebase.sh
new file mode 100644
index 000000000000..64c40a523b50
--- /dev/null
+++ b/t/t3322-notes-rebase.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='Test notes on rebase'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ git init &&
+ echo A > A &&
+ git add A &&
+ git commit -m A &&
+ git branch branch &&
+ echo B > B &&
+ git add B &&
+ git commit -m B &&
+ git notes add -m "This is B" @ &&
+ echo C > C &&
+ git add C &&
+ git commit -m C &&
+ git checkout branch &&
+ echo B > B &&
+ echo D > D &&
+ git add B D &&
+ git commit -m BD
+'
+
+test_expect_success 'rebase B + C on top of BD' '
+ git rebase @ master
+'
+
+test_expect_failure 'assert there is no note on BD' '
+ git notes show branch
+'
+
+test_done
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
--
2.47.3
^ permalink raw reply related
* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-12 14:32 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git
In-Reply-To: <0b3f7429-a4fb-4f7a-bf7b-5a0edeb1db52@gmail.com>
On 6/12/2026 2:52 PM, Derrick Stolee wrote:
> The STALE bit is pushed from commits that have bits for both sides
> of the merge. This isn't something that we can learn from just
> walking each side: we need some amount of walking within the
> intersection.
>
> This doesn't matter if we are looking for a single merge base, but
> when we want the full set of independent merge bases, then the STALE
> bit becomes very important.
Thank you for the quick and detailed response and your counterexample
graph is exactly the right thing to worry about.
> A X
> /| __/|
> | |/ |
> | B |
> | | |
> ..........
> | | __/
> \|/
> C
>
> In this example, B can reach C through some long list of commits.
> This makes B (and X) have much higher generation number than C.
> After exhausting both sides of A...X, we have B and C in the queue
> with both side bits and neither are stale. But we need to walk
> from B to C to discover that C should be stale.
I think your response helped me identify a mistake in how I described
the halt condition.
The required condition must then not be simply "one side exhausted".
The walk must also continue while non-stale P1|P2 commits remain in the
queue, since those still need STALE propagation - they are still
merge-base candidates.
So the actual halt condition would be:
no non-stale P1|P2 candidates in the queue
AND (no pure-P1 OR no pure-P2)
In your example, B and C are both non-stale P1|P2 commits after
both sides are exhausted. Therefore the walk continues. When B is
processed it propagates STALE toward C through the d-chain, and
because the finite-generation region is processed in descending
generation order, that propagation reaches C before C is popped.
If this reasoning is correct, then the walk only terminates after
merge-base candidates have either been processed or marked STALE,
and the counterexample should produce [B] rather than [B, C].
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD
From: Junio C Hamano @ 2026-06-12 14:17 UTC (permalink / raw)
To: Matt Hunter; +Cc: git
In-Reply-To: <20260612055947.1499497-7-m@lfurio.us>
Matt Hunter <m@lfurio.us> writes:
I haven't been following the discussion, so I will not comment on
the idea, i.e., if it makes sense to add such a new option and
configuration, but if we were to add such a thing, I have some
comments on the mechanics.
By the way, do not call a "configuration variable" a "configuration option".
Let's keep the vocabulary forcused without using random synonyms.
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 3cc7efdd83a0..a21bb82274d4 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -103,6 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
>
> struct fetch_config {
> enum display_format display_format;
> + enum follow_remote_head_settings follow_remote_head;
> int all;
> int prune;
> int prune_tags;
> @@ -173,6 +174,22 @@ static int git_fetch_config(const char *k, const char *v,
> "fetch.output", v);
> }
>
> + if (!strcmp(k, "fetch.followremotehead")) {
> + if (!v)
> + return config_error_nonbool(k);
> + else if (!strcasecmp(v, "never"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
> + else if (!strcasecmp(v, "create"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
> + else if (!strcasecmp(v, "warn"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
> + else if (!strcasecmp(v, "always"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
> + else
> + die(_("invalid value for '%s': '%s'"),
> + "fetch.followRemoteHEAD", v);
> + }
I think these uses of strcasecmp() are unnecessary and actively
harms end-user experience. This is especially true because the
value given to remote.<name>.followRemoteHEAD is case sensitive.
Instead of saying "if you want X to happen, set this variable to
'create'", you have to say "'create', or any other case variations
thereof like 'CrEAte'" somehow, for very dubious gain to the end
users. If you use strcmp(), and document only all lowercase forms,
it would guarantee to avoid confusing a newbie who read the variable
to be set to 'never' on one blog and 'Never' on another and wonder
if 'NEVER' would work or not.
Admittedly values to some existing configuration variables may be
parsed case insensitively but we should aim to fix the mistake in
the longer term, and we should certainly not add more of them.
Is it sensible to die() here? If you are fetching from somewhere
without keeping a set of remote-tracking branches for it (i.e., a
single shot "git fetch https://github.com/gitster/git master"), you
do not care what garbage value is in fetch.followRemoteHEAD.
Perhaps the version of Git that is slightly newer than the version
that ships with this patch defined new valid values that this patch
does not know about, and such a user who is doing a single-shot
fetch may have that setting to help them working with their usual
non-single shot repositories, but they use a newer version of Git
for such regular work, and they are using slightly old version of
Git to perform this single-shot fetch. The point is that the
configured value will *NOT* be used for such a user, and dying only
because this piece of code does not understand the configuration that
will not be used is of dubious value.
^ permalink raw reply
* RE: [ANNOUNCE] Git v2.55.0-rc0
From: rsbecker @ 2026-06-12 14:13 UTC (permalink / raw)
To: 'Junio C Hamano', git
In-Reply-To: <xmqqik7pqeiq.fsf@gitster.g>
On June 11, 2026 11:32 AM, Junio wrote:
> An early preview release Git v2.55.0-rc0 is now available for testing at the usual
> places. It is comprised of 397 non-merge commits since v2.54.0, contributed by
> 70 people, 22 of which are new faces [*].
Cargo is not available everywhere. Build is not possible on NonStop.
cargo build --release
/usr/coreutils/bin/bash: cargo: command not found
Makefile:3021: recipe for target 'target/release/libgitcore.a' failed
make: *** [target/release/libgitcore.a] Error 127
Is there a way around this?
^ permalink raw reply
* Re: [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD
From: Matt Hunter @ 2026-06-12 14:00 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-7-m@lfurio.us>
On Fri Jun 12, 2026 at 1:55 AM EDT, Matt Hunter wrote:
> diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index eb9c8a3c4884..761bf4ba7d14 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -157,15 +157,12 @@ Blank values signal to ignore all previous values, allowing a reset of
> the list from broader config scenarios.
>
> remote.<name>.followRemoteHEAD::
> - How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
> - when fetching using the configured refspecs of a remote.
> - The default value is "create", which will create `remotes/<name>/HEAD`
> - if it exists on the remote, but not locally; this will not touch an
> - already existing local reference. Setting it to "warn" will print
> - a message if the remote has a different value than the local one;
> - in case there is no local reference, it behaves like "create".
> - A variant on "warn" is "warn-if-not-$branch", which behaves like
> - "warn", but if `HEAD` on the remote is `$branch` it will be silent.
> - Setting it to "always" will silently update `remotes/<name>/HEAD` to
> - the value on the remote. Finally, setting it to "never" will never
> - change or create the local reference.
> + When fetching this remote using its default refspec, this option determines
> + how to handle differences between the remote's `HEAD` and the local
> + `remotes/<name>/HEAD` symbolic-ref. Overrides the setting for
> + `fetch.followRemoteHEAD`. See `fetch.followRemoteHEAD` for a description of
> + accepted values.
> ++
> +In addition to the values supported by `fetch.followRemoteHEAD`, this option may
> +also take on the value "warn-if-not-`$branch`", which behaves like "warn", but
> +ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.
In hindsight, I'm wondering if $branch ought to be stylized as <branch>
to match the rest of the docs. Thoughts?
^ permalink raw reply
* [PATCH v4 2/2] graph: indent visual root in graph
From: Pablo Sabater @ 2026-06-12 13:48 UTC (permalink / raw)
To: git
Cc: ayu.chandekar, chandrapratap3519, christian.couder, gitster,
jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
Pablo Sabater
In-Reply-To: <20260612-ps-pre-commit-indent-v4-0-e8492037ebae@gmail.com>
When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.
A fix for this issue was already attempted [1] a while ago.
This happens because the commits fill the space from left to right and
when a visual root ends, its column becomes free for the following
commit even if they are not related. Once this happens the unrelated
commit is rendered below the visual root. Because there is no special
character or way to identify when a visual root is rendered making the
graph confusing.
By indenting the visual roots when there are still commits to show the
vertical adjacency can be avoided.
Add is_visual_root flag to git_graph making it visible in all graph states,
give graph_update() a new function, graph_is_visual_root() to know if the
current commit is a visual root and set is_visual_root.
The different handled cases are:
- If a visual root has children: similar to GRAPH_PRE_COMMIT state when
octopus merges need space, an edge row needs to be printed to connect
the child with the indented visual root. A new state GRAPH_PRE_ROOT is
needed to connect the child with the visual root:
* child of the visual root
\ GRAPH_PRE_ROOT
* visual root indented
- If a visual root is child-less we can skip GRAPH_PRE_ROOT state and
render the indented commit directly.
* visual root indented
* unrelated commit
- If two or more visual roots are adjacent: by having a lookahead to the
next commit that will be rendered, if the next commit is also a visual
root and we are on a visual root, meaning two visual root adjacent in
the history, the top one can omit the indent, making the one below to
indent only once, if there are more adjacent visual commits, the
indentation will increase for each adjacent one, cascading.
* visual root
* visual root
* visual root
* last commit
Even if the last commit is a root, because there is nothing that will be
rendered below we can omit the indentation on purpose.
The lookahead is not completely reliable, on graphs with filtered parents,
the walker when processing the current commit it will simplify its
parents by removing the ones that won't be shown, (They have the
TREESAME flag when filtering by path for example), but it doesn't act
for the grandparents or the next commit if it is unrelated until we move
to the next.
For example given
A visual root
B child
C parent of B, visual root FILTERED
D last commit
We would expect
A
B
D
When processing A, for the walker and the information at the renderer, B
is still a child of C, as B parent, hasn't been removed yet. This makes
cascade to not trigger as the lookahead fails to detect if the next
commit will be a visual root.
Once at B, its parent has been removed and has become a visual root, and
it just adds its indent to the one left by A. We end up with an extra
indent:
A
B
D
The output isn't broken as unrelated commits are successfully separated
by indentation, but an indent level should have been avoided.
Create a new test file for graph indentations test called
't4218-log-graph-indentation.sh'.
The filtered parents edge case is documented as a NEEDSWORK on the
lookahead function and it has its own 'test_expect_failure' at 't4218'.
[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
graph.c | 262 ++++++++++++++++++++++
t/meson.build | 1 +
t/t4218-log-graph-indentation.sh | 455 +++++++++++++++++++++++++++++++++++++++
3 files changed, 718 insertions(+)
diff --git a/graph.c b/graph.c
index 842282685f..e0d1e2a510 100644
--- a/graph.c
+++ b/graph.c
@@ -60,12 +60,23 @@ struct column {
* index into column_colors.
*/
unsigned short color;
+ /*
+ * Marks if a commit is a non-first parent of a merge. These columns are
+ * already visually connected to the merge commit and do not need
+ * indentation.
+ *
+ * The first parent is the one that inherits the column and it can need
+ * indentation if turns out to be a visual root and there's still
+ * commits to render.
+ */
+ unsigned is_merge_parent:1;
};
enum graph_state {
GRAPH_PADDING,
GRAPH_SKIP,
GRAPH_PRE_COMMIT,
+ GRAPH_PRE_ROOT,
GRAPH_COMMIT,
GRAPH_POST_MERGE,
GRAPH_COLLAPSING
@@ -315,6 +326,48 @@ struct git_graph {
* diff_output_prefix_callback().
*/
struct strbuf prefix_buf;
+
+ /*
+ * If a commit is a visual root, we need to indent it to prevent
+ * unrelated commits from being vertically adjacent to it.
+ */
+ unsigned is_visual_root:1;
+
+ /*
+ * Indentation increases for each visual root adjacent to another visual
+ * root, making visual root commits indentation cascade.
+ */
+ unsigned int visual_root_depth;
+
+ /*
+ * When a visual root is adjacent to other visual roots, the first one
+ * can avoid indentation and the rest cascades, increasing the indentation
+ * for each one.
+ */
+ unsigned visual_root_cascade:1;
+
+ /*
+ * Set when the current commit was already present in graph->columns
+ * before being processed.
+ */
+ unsigned commit_in_columns:1;
+};
+
+struct graph_lookahead_flags {
+ /*
+ * Set when there will be a commit after the current one that will be
+ * rendered.
+ */
+ unsigned int is_next_visible:1;
+ /*
+ * Set when the next visible commit is candidate to be a visual root.
+ */
+ unsigned int is_next_visual_root:1;
+ /*
+ * Set when the next visible commit will be rendered under the current
+ * commit.
+ */
+ unsigned int next_has_column:1;
};
static inline int graph_needs_truncation(struct git_graph *graph, int lane)
@@ -388,6 +441,8 @@ struct git_graph *graph_init(struct rev_info *opt)
graph->num_columns = 0;
graph->num_new_columns = 0;
graph->mapping_size = 0;
+ graph->visual_root_depth = 0;
+ graph->visual_root_cascade = 0;
/*
* Start the column color at the maximum value, since we'll
* always increment it for the first commit we output.
@@ -561,6 +616,11 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
struct commit *commit,
int idx)
{
+ /*
+ * Get the initial merge_layout before it's modified to know if this
+ * is a merge.
+ */
+ int initial_merge_layout = graph->merge_layout;
int i = graph_find_new_column_by_commit(graph, commit);
int mapping_idx;
@@ -572,6 +632,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
i = graph->num_new_columns++;
graph->new_columns[i].commit = commit;
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+ graph->new_columns[i].is_merge_parent = 0;
}
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -610,6 +671,12 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
}
graph->mapping[mapping_idx] = i;
+
+ /*
+ * Mark non-first parents of a merge.
+ */
+ if (graph->num_parents > 1 && initial_merge_layout >= 0 && idx > -1)
+ graph->new_columns[i].is_merge_parent = 1;
}
static void graph_update_columns(struct git_graph *graph)
@@ -701,10 +768,20 @@ static void graph_update_columns(struct git_graph *graph)
if (graph->num_parents == 0)
graph->width += 2;
} else {
+ int j;
graph_insert_into_new_columns(graph, col_commit, -1);
+ /*
+ * This column is not the current commit, but we need to
+ * propagate the flag until the commit is processed.
+ */
+ j = graph_find_new_column_by_commit(graph, col_commit);
+ if (j >= 0 && graph->columns[i].is_merge_parent)
+ graph->new_columns[j].is_merge_parent = 1;
}
}
+ graph->commit_in_columns = is_commit_in_columns;
+
/*
* If graph_max_lanes is set, cap the width
*/
@@ -763,9 +840,135 @@ static int graph_needs_pre_commit_line(struct git_graph *graph)
graph->expansion_row < graph_num_expansion_rows(graph);
}
+/*
+ * A commit can be a visual root when:
+ * - It has no parents.
+ *
+ * - It has parents but they are all filtered out and
+ * commit->parents arrives NULL.
+ *
+ * - It is not a boundary commit. Boundary commits also have no visible
+ * parents, but they are not selected as visual roots because they cannot
+ *. cause the ambiguity of being vertically adjacent because:
+ *
+ * 1. A boundary only appears because an included commit is its child.
+ * Children are always above, and the renderer draws an edge down to
+ * the boundary from that child. Rather than starting a column like a
+ * visual root would do, it inherits its child column.
+ *
+ * 2. Included commits cannot appear below a boundary. Boundaries are
+ * ancestors of the exclusion point; if an included commit were an
+ * ancestor of the boundary it would be excluded and not rendered.
+ * Boundaries therefore always sink to the bottom.
+ */
+static int graph_is_visual_root_candidate(struct commit *c)
+{
+ return c->parents == NULL && !(c->object.flags & BOUNDARY);
+}
+
+static int graph_is_visual_root(struct git_graph *graph,
+ struct graph_lookahead_flags *flags)
+{
+ /*
+ * This must be only called for the current commit as graph contains
+ * the state for the current commit only.
+ *
+ * To check if a commit is a visual root, call graph_is_visual_root_candidate()
+ * but we won't know if it is really a visual root until we get to the
+ * next commit state.
+ *
+ * The current commit is an actual visual root if it is a candidate and
+ * the commit is not a non-first parent of a merge.
+ *
+ * *
+ * |\
+ * | * <- it is a visual root candidate but it shouldn't be indented
+ * * because it is already connected by an edge.
+ * ^ if commit_in_columns && is_merge_parent means the commit
+ * | was put by a merge and is connected.
+ * |
+ * `-------- if !is_next_visible means we're on the last commit, avoid
+ * indentation unless the one before is a visual root, then
+ * we need to differentiate from the one above.
+ *
+ * If next_has_columns means that the next commit has
+ * already a column, so it will not be rendered below, the
+ * current commit has to act as the last commit and omit
+ * indentation.
+ */
+ return graph_is_visual_root_candidate(graph->commit) &&
+ !(graph->commit_in_columns &&
+ graph->columns[graph->commit_index].is_merge_parent) &&
+ flags->is_next_visible &&
+ (!flags->next_has_column || graph->visual_root_depth > 0);
+}
+
+/*
+ * Iterates the commits queue searching for the next visible commit, once found
+ * sets visibleness and visual-root flags.
+ * Knowing if the next commit is also a visual root avoids redundant indentations
+ *
+ * NEEDSWORK: The queue is actively being modified by the walker, for each commit
+ * its parents and itself get simplified and their flags set, but for the next
+ * unrelated commit or the grandparents they are not simplified yet, which means
+ * that a commit whose parents are all filtered will not be marked as a visual
+ * root candidate at the lookahead.
+ * This causes the lookahead to fail, failing to set the cascade flag to avoid
+ * redundant indentations.
+ * See 'test_expect_failure' at t4218-log-graph-indentation.sh.
+ */
+static void graph_peek_next_visible(struct git_graph *graph,
+ struct graph_lookahead_flags *flags)
+{
+ struct commit_list *cl;
+
+ flags->is_next_visible = 0;
+ flags->is_next_visual_root = 0;
+ flags->next_has_column = 0;
+
+ for (cl = graph->revs->commits; cl; cl = cl->next) {
+ if (get_commit_action(graph->revs, cl->item) != commit_show)
+ continue;
+ flags->is_next_visible = 1;
+ flags->next_has_column = graph_find_new_column_by_commit(graph, cl->item) >= 0;
+ /*
+ * We do not need graph->commit_in_columns or is_merge_parent,
+ * because we only need to know whether the next one might be a
+ * visual root, affecting the current commit where the cascade
+ * would have to be set and the first visual root not indented.
+ *
+ * It will set next_is_visual_root to true for merge parents that
+ * graph_is_visual_root() would return false, but if the next is
+ * a merge parent, the current commit is the child and cannot
+ * be a visual root and therefore having no effect.
+ */
+ if (!graph_is_visual_root_candidate(cl->item))
+ return;
+
+ /*
+ * The next visible commit is a visual root candidate, but
+ * only set cascade if it's not the last commit to be rendered.
+ */
+ for (cl = cl->next; cl; cl = cl->next) {
+ if (get_commit_action(graph->revs, cl->item) != commit_show)
+ continue;
+ flags->is_next_visual_root = 1;
+ return;
+ }
+ return;
+ }
+}
+
+static int graph_needs_pre_root_line(struct git_graph *graph)
+{
+ return graph->commit_in_columns && graph->is_visual_root &&
+ graph->num_columns > 0 && !graph->visual_root_cascade;
+}
+
void graph_update(struct git_graph *graph, struct commit *commit)
{
struct commit_list *parent;
+ struct graph_lookahead_flags flags;
/*
* Set the new commit
@@ -796,6 +999,23 @@ void graph_update(struct git_graph *graph, struct commit *commit)
*/
graph_update_columns(graph);
+ graph_peek_next_visible(graph, &flags);
+
+ graph->is_visual_root = graph_is_visual_root(graph, &flags);
+
+ if (graph->is_visual_root) {
+ /*
+ * If next is a visual root we can omit the indent for the first
+ * visual root and start cascading.
+ */
+ if (!graph->visual_root_depth && flags.is_next_visual_root)
+ graph->visual_root_cascade = 1;
+ graph->visual_root_depth++;
+ } else {
+ graph->visual_root_depth = 0;
+ graph->visual_root_cascade = 0;
+ }
+
graph->expansion_row = 0;
/*
@@ -813,11 +1033,16 @@ void graph_update(struct git_graph *graph, struct commit *commit)
* room for it. We need to do this only if there is a branch row
* (or more) to the right of this commit.
*
+ * If it is a visual root, we need to print an extra row to
+ * connect the indentation.
+ *
* If there are less than 3 parents, we can immediately print the
* commit line.
*/
if (graph->state != GRAPH_PADDING)
graph->state = GRAPH_SKIP;
+ else if (graph_needs_pre_root_line(graph))
+ graph->state = GRAPH_PRE_ROOT;
else if (graph_needs_pre_commit_line(graph))
graph->state = GRAPH_PRE_COMMIT;
else
@@ -1065,6 +1290,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
if (col_commit == graph->commit) {
seen_this = 1;
+ if (graph->is_visual_root) {
+ int depth = graph->visual_root_depth;
+ /*
+ * Each visual column is 2 characters wide.
+ * Omit the indentation for the first visual
+ * root in cascade mode.
+ */
+ int padding = (depth - graph->visual_root_cascade) * 2;
+ graph_line_addchars(line, ' ', padding);
+ graph->width += padding;
+ }
graph_output_commit_char(graph, line);
if (graph_needs_truncation(graph, i)) {
@@ -1436,6 +1672,29 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
graph_update_state(graph, GRAPH_PADDING);
}
+static void graph_output_pre_root_line(struct git_graph *graph, struct graph_line *line)
+{
+ /*
+ * This function adds a row before a visual root, to connect the
+ * branch to the indented commit. It should only be called on a
+ * visual root.
+ */
+ assert(graph->is_visual_root);
+
+ for (size_t i = 0; i < graph->num_columns; i++) {
+ struct column *col = &graph->columns[i];
+ if (col->commit == graph->commit) {
+ graph_line_addch(line, ' ');
+ graph_line_write_column(line, col, '\\');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
+ graph_line_addch(line, ' ');
+ }
+
+ graph_update_state(graph, GRAPH_COMMIT);
+}
+
int graph_next_line(struct git_graph *graph, struct strbuf *sb)
{
int shown_commit_line = 0;
@@ -1461,6 +1720,9 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
case GRAPH_PRE_COMMIT:
graph_output_pre_commit_line(graph, &line);
break;
+ case GRAPH_PRE_ROOT:
+ graph_output_pre_root_line(graph, &line);
+ break;
case GRAPH_COMMIT:
graph_output_commit_line(graph, &line);
shown_commit_line = 1;
diff --git a/t/meson.build b/t/meson.build
index c5832fee05..17037a8465 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -576,6 +576,7 @@ integration_tests = [
't4215-log-skewed-merges.sh',
't4216-log-bloom.sh',
't4217-log-limit.sh',
+ 't4218-log-graph-indentation.sh',
't4252-am-options.sh',
't4253-am-keep-cr-dos.sh',
't4254-am-corrupt.sh',
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
new file mode 100755
index 0000000000..f1c9584ba5
--- /dev/null
+++ b/t/t4218-log-graph-indentation.sh
@@ -0,0 +1,455 @@
+#!/bin/sh
+
+test_description='git log --graph visual root indentations'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
+
+check_graph_with_description () {
+ cat >expect &&
+ lib_test_cmp_graph --format="%s%ndescription%nsecond-line" "$@"
+}
+
+create_orphan () {
+ git checkout --orphan "$1" &&
+ { git rm -rf . || true; }
+}
+
+test_expect_success 'single root commit is not indented' '
+ create_orphan _1 && test_commit 1_A &&
+ lib_test_check_graph _1 <<-\EOF
+ * 1_A
+ EOF
+'
+
+test_expect_success 'visual root indented before unrelated branch' '
+ create_orphan _2 && test_commit 2_A && test_commit 2_B &&
+ create_orphan _3 && test_commit 3_A &&
+ lib_test_check_graph _2 _3 <<-\EOF
+ * 3_A
+ * 2_B
+ * 2_A
+ EOF
+'
+
+test_expect_success 'visual root indentation with --left-right' '
+ lib_test_check_graph --left-right _2..._3 <<-\EOF
+ > 3_A
+ < 2_B
+ < 2_A
+ EOF
+'
+
+# A better case of why indentation is still needed with '--left-right' flag is
+# that unrelated branches can be on the same side, so it's needed to
+# differentiate visual roots on the same side.
+test_expect_success 'visual root indentation with --left-right having unrelated commits on the same side' '
+ lib_test_check_graph --left-right _2..._3 _1 <<-\EOF
+ > 3_A
+ < 2_B
+ \
+ < 2_A
+ > 1_A
+ EOF
+'
+
+test_expect_success 'visual root indents the description also' '
+ check_graph_with_description _2 _3 <<-\EOF
+ * 3_A
+ description
+ second-line
+ * 2_B
+ | description
+ | second-line
+ * 2_A
+ description
+ second-line
+ EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child' '
+ create_orphan _4 && test_commit 4_A && test_commit 4_B &&
+ create_orphan _5 && test_commit 5_A && test_commit 5_B &&
+ lib_test_check_graph _4 _5<<-\EOF
+ * 5_B
+ \
+ * 5_A
+ * 4_B
+ * 4_A
+ EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child with description' '
+ check_graph_with_description _4 _5 <<-\EOF
+ * 5_B
+ | description
+ | second-line
+ \
+ * 5_A
+ description
+ second-line
+ * 4_B
+ | description
+ | second-line
+ * 4_A
+ description
+ second-line
+ EOF
+'
+
+test_expect_success 'visual roots cascade and last root does not' '
+ create_orphan _7 && test_commit 7_A && test_commit 7_B &&
+ create_orphan _8 && test_commit 8_A &&
+ create_orphan _9 && test_commit 9_A &&
+ create_orphan _10 && test_commit 10_A &&
+ lib_test_check_graph _7 _8 _9 _10 <<-\EOF
+ * 10_A
+ * 9_A
+ * 8_A
+ * 7_B
+ * 7_A
+ EOF
+'
+
+test_expect_success 'last root does not cascade' '
+ lib_test_check_graph _8 _9 _10 <<-\EOF
+ * 10_A
+ * 9_A
+ * 8_A
+ EOF
+'
+
+test_expect_success 'merge parents are roots between them but they do not indent' '
+ create_orphan _11 && test_commit 11_A &&
+ create_orphan _12 && test_commit 12_A &&
+ create_orphan _13 && test_commit 13_A &&
+ git checkout _11 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p _11 -p _12 -p _13 -m 11_octopus) &&
+ git reset --hard $MERGE &&
+ lib_test_check_graph _11 <<-\EOF
+ *-. 11_octopus
+ |\ \
+ | | * 13_A
+ | * 12_A
+ * 11_A
+ EOF
+'
+
+# The last parent of a merge can be indented if nothing related to it needs to
+# be rendered after, if it's another visual root, merge parent must not get
+# indented but rather activate cascading.
+test_expect_success 'merge then unrelated visual root and unrelated branch' '
+ create_orphan _16 && test_commit 16_A && test_commit 16_B &&
+ create_orphan _17 && test_commit 17_A &&
+ create_orphan _18 && test_commit 18_A &&
+ create_orphan _19 && test_commit 19_A &&
+ create_orphan _20 && test_commit 20_A &&
+ git checkout _18 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p _18 -p _19 -p _20 -m 18_octopus) &&
+ git reset --hard $MERGE &&
+ lib_test_check_graph _18 _17 _16 <<-\EOF
+ *-. 18_octopus
+ |\ \
+ | | * 20_A
+ | * 19_A
+ * 18_A
+ * 17_A
+ * 16_B
+ * 16_A
+ EOF
+'
+
+# The last commit root does not get indented, if the next thing after the root
+# merge parent is the last commit, indent the merge parent.
+test_expect_success 'merge then unrelated root indents merge parent' '
+ lib_test_check_graph _18 _17 <<-\EOF
+ *-. 18_octopus
+ |\ \
+ | | * 20_A
+ | * 19_A
+ \
+ * 18_A
+ * 17_A
+ EOF
+'
+
+test_expect_success 'merge then unrelated branch indents merge parent' '
+ lib_test_check_graph _18 _16 <<-\EOF
+ *-. 18_octopus
+ |\ \
+ | | * 20_A
+ | * 19_A
+ \
+ * 18_A
+ * 16_B
+ * 16_A
+ EOF
+'
+
+test_expect_success 'two-parent merge of orphans' '
+ create_orphan _21 && test_commit 21_A &&
+ create_orphan _22 && test_commit 22_A &&
+ git checkout _21 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p _21 -p _22 -m 21_merge) &&
+ git reset --hard $MERGE &&
+ lib_test_check_graph _21 <<-\EOF
+ * 21_merge
+ |\
+ | * 22_A
+ * 21_A
+ EOF
+'
+
+test_expect_success 'commit with filtered parent becomes a visual root' '
+ create_orphan _23 &&
+ echo test >other.txt &&
+ git add other.txt &&
+ git commit -m "23_A" &&
+ echo test >foo.txt &&
+ git add foo.txt &&
+ git commit -m "23_B" &&
+ create_orphan _24 &&
+ echo test >foo.txt &&
+ git add foo.txt &&
+ git commit -m "24_A" &&
+ lib_test_check_graph _23 _24 -- foo.txt <<-\EOF
+ * 23_B
+ * 24_A
+ EOF
+'
+
+# The walker simplifies the commit for the current one and its parents, removing
+# the filtered parents, but it doesn't go one step ahead, this causes some edge
+# cases with the lookahead.
+# Given A (orphan), the walker only processes A, and when we lookahead for B
+# (child of C) even tho C will be filtered, it hasn't been simplified yet, so we
+# don't see B as a visual root, therefore cascade indentation isn't applied to A.
+# (cascade indentation starts the indentation at the second visual root, to avoid
+# redundant indentation). So A gets an extra indent, and once B is processed,
+# when rendering it, C has been removed, B is a visual root and as the last commit
+# isn't considered a visual root as it cannot have unrelated commits below it,
+# cascading isn't also applied, giving B another indent.
+#
+# The final result is an extra indent for A and B:
+#
+# A
+# B
+# D
+#
+# This will happen for any case where we find ourselves with the next commit
+# being a unrelated child of a parent the will be filtered.
+#
+# instead of the expected:
+test_expect_failure 'filtered parent cascading edge case' '
+ create_orphan _25 &&
+ git rm -rf . &&
+ echo test >other.txt &&
+ git add other.txt &&
+ git commit -m "C-filtered" &&
+
+ echo test >foo.txt &&
+ git add foo.txt &&
+ git commit -m "B (child of filtered)" &&
+
+ create_orphan _26 &&
+ git rm -rf . &&
+ echo test >foo.txt &&
+ git add foo.txt &&
+ git commit -m "A (visual root)" &&
+
+ create_orphan _27 &&
+ git rm -rf . &&
+ echo test >foo.txt &&
+ git add foo.txt &&
+ git commit -m "D (last)" &&
+
+ lib_test_check_graph _25 _26 _27 -- foo.txt <<-\EOF
+ * A (visual root)
+ * B (child of filtered)
+ * D (last)
+ EOF
+'
+
+test_expect_failure 'multiple filtered parents in sequence' '
+ create_orphan _44 &&
+ git rm -rf . &&
+ echo a >other.txt && git add other.txt && git commit -m "44_F" &&
+ echo b >foo.txt && git add foo.txt && git commit -m "44_C" &&
+
+ create_orphan _45 &&
+ git rm -rf . &&
+ echo c >other.txt && git add other.txt && git commit -m "45_F" &&
+ echo d >foo.txt && git add foo.txt && git commit -m "45_C" &&
+
+ create_orphan _46 &&
+ git rm -rf . &&
+ echo e >foo.txt && git add foo.txt && git commit -m "46_A" &&
+
+ lib_test_check_graph _44 _45 _46 -- foo.txt <<-\EOF
+ * 46_A
+ * 45_C
+ * 44_C
+ EOF
+'
+
+test_expect_failure 'real orphan root followed by child of filtered parent' '
+ create_orphan _47 &&
+ git rm -rf . &&
+ echo a >foo.txt && git add foo.txt && git commit -m "47_A" &&
+
+ create_orphan _48 &&
+ git rm -rf . &&
+ echo b >other.txt && git add other.txt && git commit -m "48_filtered" &&
+ echo c >foo.txt && git add foo.txt && git commit -m "48_B" &&
+
+ create_orphan _49 &&
+ git rm -rf . &&
+ echo d >foo.txt && git add foo.txt && git commit -m "49_last" &&
+
+ lib_test_check_graph _47 _48 _49 -- foo.txt <<-\EOF
+ * 47_A
+ * 48_B
+ * 49_last
+ EOF
+'
+
+# This tests prove why there is no need to have indentation for boundary
+# commits.
+#
+# Boundary commits rather than starting a column they 'inherit' the one of
+# its child so there will always be an edge that connects it removing the
+# ambiguity.
+test_expect_success 'unrelated boundaries are not ambiguous' '
+ create_orphan _28 && test_commit 28_A && test_commit 28_B &&
+ test_commit 28_C &&
+ create_orphan _29 && test_commit 29_A && test_commit 29_B &&
+ lib_test_check_graph --boundary 28_A.._28 29_A.._29 <<-\EOF
+ * 29_B
+ | * 28_C
+ | * 28_B
+ | o 28_A
+ o 29_A
+ EOF
+'
+
+# Same structure as t6016
+test_expect_success 'boundary commits big test' '
+ # 3 commits on branch _30
+ create_orphan _30 &&
+ test_commit 30_A &&
+ test_commit 30_B &&
+ test_commit 30_C &&
+
+ # 2 commits on branch _31, started from 30_A
+ git checkout -b _31 30_A &&
+ test_commit 31_A &&
+ test_commit 31_B &&
+
+ # 2 commits on branch _32, started from 30_B
+ git checkout -b _32 30_B &&
+ test_commit 32_A &&
+ test_commit 32_B &&
+
+ # Octopus merge _31 and _32 into -30
+ git checkout _30 &&
+ git merge _31 _32 -m 30_D &&
+ git tag 30_D &&
+ test_commit 30_E &&
+
+ # More commits on _32, then merge _32 into _30
+ git checkout _32 &&
+ test_commit 32_C &&
+ test_commit 32_D &&
+ git checkout _30 &&
+ git merge -s ours _32 -m 30_F &&
+ git tag 30_F &&
+ test_commit 30_G &&
+ lib_test_check_graph --boundary _30 _31 _32 ^32_C <<-\EOF
+ * 30_G
+ * 30_F
+ |\
+ | * 32_D
+ * | 30_E
+ | |
+ | \
+ *-. \ 30_D
+ |\ \ \
+ | * | | 31_B
+ | * | | 31_A
+ * | | | 30_C
+ o | | | 30_B
+ |/ / /
+ o / / 30_A
+ / /
+ | o 32_C
+ |/
+ o 32_B
+ EOF
+'
+
+# Filter by --first-parent and then forcing the filtered parents to be shown.
+test_expect_success '--first-parent flag with the filtered parents' '
+ (
+ create_orphan _35 && test_commit 35_A && test_commit 35_B &&
+ create_orphan _36 && test_commit 36_A &&
+ create_orphan _37 && test_commit 37_A &&
+ git checkout _35 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p _35 -p _36 -p _37 -m 35_octopus) &&
+ git reset --hard $MERGE &&
+ lib_test_check_graph --first-parent _35 _36 _37 <<-\EOF
+ * 35_octopus
+ | * 37_A
+ | * 36_A
+ * 35_B
+ * 35_A
+ EOF
+ )
+'
+
+test_expect_success '--first-parent with filtered parents but one has a child' '
+ (
+ create_orphan _38 && test_commit 38_A && test_commit 38_B &&
+ create_orphan _39 && test_commit 39_A &&
+ create_orphan _40 && test_commit 40_A && test_commit 40_B &&
+ git checkout _38 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p _38 -p _39 -p _40 -m 38_octopus) &&
+ git reset --hard $MERGE &&
+ lib_test_check_graph --first-parent _38 _39 _40 <<-\EOF
+ * 38_octopus
+ | * 40_B
+ | * 40_A
+ | * 39_A
+ * 38_B
+ * 38_A
+ EOF
+ )
+'
+
+test_expect_success '--first-parent with filtered parents but both have childs' '
+ (
+ create_orphan _41 && test_commit 41_A && test_commit 41_B &&
+ create_orphan _42 && test_commit 42_A && test_commit 42_B &&
+ create_orphan _43 && test_commit 43_A && test_commit 43_B &&
+ git checkout _41 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p _41 -p _42 -p _43 -m 41_octopus) &&
+ git reset --hard $MERGE &&
+ lib_test_check_graph --first-parent _41 _42 _43 <<-\EOF
+ * 41_octopus
+ | * 43_B
+ | \
+ | * 43_A
+ | * 42_B
+ | * 42_A
+ * 41_B
+ * 41_A
+ EOF
+ )
+'
+
+test_done
--
2.54.0
^ permalink raw reply related
* [PATCH v4 1/2] lib-log-graph: move check_graph function
From: Pablo Sabater @ 2026-06-12 13:48 UTC (permalink / raw)
To: git
Cc: ayu.chandekar, chandrapratap3519, christian.couder, gitster,
jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
Pablo Sabater
In-Reply-To: <20260612-ps-pre-commit-indent-v4-0-e8492037ebae@gmail.com>
check_graph is a function shared in the test files t4215 and t6016 used
to format the output graph, but instead of being in a file called by
both test, the function code is repeated in each file.
Move check_graph to lib-log-graph.sh file which both tests already
import graph functions from, renaming it to lib_test_check_graph.
This function is needed for the following commit which includes graph
tests in a new file and requires check_graph.
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
t/lib-log-graph.sh | 5 +++++
t/t4215-log-skewed-merges.sh | 33 +++++++++++++-----------------
t/t6016-rev-list-graph-simplify-history.sh | 25 +++++++++-------------
3 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
index bf952ef920..1eae8f60c2 100644
--- a/t/lib-log-graph.sh
+++ b/t/lib-log-graph.sh
@@ -26,3 +26,8 @@ lib_test_cmp_colored_graph () {
test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
test_cmp expect.colors output.colors
}
+
+lib_test_check_graph () {
+ cat >expect &&
+ lib_test_cmp_graph --format=%s "$@"
+}
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 1612f05f1b..eebab71039 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -5,11 +5,6 @@ test_description='git log --graph of skewed merges'
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-log-graph.sh
-check_graph () {
- cat >expect &&
- lib_test_cmp_graph --format=%s "$@"
-}
-
test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
git checkout --orphan _p &&
test_commit A &&
@@ -21,7 +16,7 @@ test_expect_success 'log --graph with merge fusing with its left and right neigh
git checkout _p && git merge --no-ff _r -m G &&
git checkout @^^ && git merge --no-ff _p -m H &&
- check_graph <<-\EOF
+ lib_test_check_graph <<-\EOF
* H
|\
| * G
@@ -49,7 +44,7 @@ test_expect_success 'log --graph with left-skewed merge' '
git checkout 0_p && git merge --no-ff 0_s -m 0_G &&
git checkout @^ && git merge --no-ff 0_q 0_r 0_t 0_p -m 0_H &&
- check_graph <<-\EOF
+ lib_test_check_graph <<-\EOF
*-----. 0_H
|\ \ \ \
| | | | * 0_G
@@ -83,7 +78,7 @@ test_expect_success 'log --graph with nested left-skewed merge' '
git checkout 1_p && git merge --no-ff 1_r -m 1_G &&
git checkout @^^ && git merge --no-ff 1_p -m 1_H &&
- check_graph <<-\EOF
+ lib_test_check_graph <<-\EOF
* 1_H
|\
| * 1_G
@@ -115,7 +110,7 @@ test_expect_success 'log --graph with nested left-skewed merge following normal
git checkout -b 2_s @^^ && git merge --no-ff 2_q -m 2_J &&
git checkout 2_p && git merge --no-ff 2_s -m 2_K &&
- check_graph <<-\EOF
+ lib_test_check_graph <<-\EOF
* 2_K
|\
| * 2_J
@@ -151,7 +146,7 @@ test_expect_success 'log --graph with nested right-skewed merge following left-s
git checkout 3_p && git merge --no-ff 3_r -m 3_H &&
git checkout @^^ && git merge --no-ff 3_p -m 3_J &&
- check_graph <<-\EOF
+ lib_test_check_graph <<-\EOF
* 3_J
|\
| * 3_H
@@ -182,7 +177,7 @@ test_expect_success 'log --graph with right-skewed merge following a left-skewed
git merge --no-ff 4_p -m 4_G &&
git checkout @^^ && git merge --no-ff 4_s -m 4_H &&
- check_graph --date-order <<-\EOF
+ lib_test_check_graph --date-order <<-\EOF
* 4_H
|\
| * 4_G
@@ -218,7 +213,7 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
git checkout 5_r &&
git merge --no-ff 5_s -m 5_H &&
- check_graph <<-\EOF
+ lib_test_check_graph <<-\EOF
* 5_H
|\
| *-. 5_G
@@ -257,7 +252,7 @@ test_expect_success 'log --graph with multiple tips' '
git checkout 6_1 &&
git merge --no-ff 6_2 -m 6_I &&
- check_graph 6_1 6_3 6_5 <<-\EOF
+ lib_test_check_graph 6_1 6_3 6_5 <<-\EOF
* 6_I
|\
| | * 6_H
@@ -334,7 +329,7 @@ test_expect_success 'log --graph with multiple tips' '
git checkout -b M_7 7_1 &&
git merge --no-ff 7_2 7_3 -m 7_M4 &&
- check_graph M_1 M_3 M_5 M_7 <<-\EOF
+ lib_test_check_graph M_1 M_3 M_5 M_7 <<-\EOF
* 7_M1
|\
| | * 7_M2
@@ -371,7 +366,7 @@ test_expect_success 'log --graph with multiple tips' '
'
test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
- check_graph --graph-lane-limit=2 M_7 <<-\EOF
+ lib_test_check_graph --graph-lane-limit=2 M_7 <<-\EOF
*-. 7_M4
|\ \
| | * 7_G
@@ -388,7 +383,7 @@ test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
'
test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge' '
- check_graph --graph-lane-limit=1 M_7 <<-\EOF
+ lib_test_check_graph --graph-lane-limit=1 M_7 <<-\EOF
*-~ 7_M4
|\~
| ~ 7_G
@@ -405,7 +400,7 @@ test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge
'
test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
- check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+ lib_test_check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
* 7_M1
|\
| | * 7_M2
@@ -441,7 +436,7 @@ test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
'
test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows first of 3 parent merge' '
- check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
+ lib_test_check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
* 7_M1
|\
| | * 7_M2
@@ -478,7 +473,7 @@ test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows fir
'
test_expect_success 'log --graph --graph-lane-limit=7 check if it shows all 3 parent merge' '
- check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
+ lib_test_check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
* 7_M1
|\
| | * 7_M2
diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
index 54b0a6f5f8..e0d9c3c1ac 100755
--- a/t/t6016-rev-list-graph-simplify-history.sh
+++ b/t/t6016-rev-list-graph-simplify-history.sh
@@ -13,11 +13,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-log-graph.sh
-check_graph () {
- cat >expect &&
- lib_test_cmp_graph --format=%s "$@"
-}
-
test_expect_success 'set up rev-list --graph test' '
# 3 commits on branch A
test_commit A1 foo.txt &&
@@ -54,7 +49,7 @@ test_expect_success 'set up rev-list --graph test' '
'
test_expect_success '--graph --all' '
- check_graph --all <<-\EOF
+ lib_test_check_graph --all <<-\EOF
* A7
* A6
|\
@@ -82,7 +77,7 @@ test_expect_success '--graph --all' '
# that undecorated merges are interesting, even with --simplify-by-decoration
test_expect_success '--graph --simplify-by-decoration' '
git tag -d A4 &&
- check_graph --all --simplify-by-decoration <<-\EOF
+ lib_test_check_graph --all --simplify-by-decoration <<-\EOF
* A7
* A6
|\
@@ -114,7 +109,7 @@ test_expect_success 'setup: get rid of decorations on B' '
# Graph with branch B simplified away
test_expect_success '--graph --simplify-by-decoration prune branch B' '
- check_graph --simplify-by-decoration --all <<-\EOF
+ lib_test_check_graph --simplify-by-decoration --all <<-\EOF
* A7
* A6
|\
@@ -133,7 +128,7 @@ test_expect_success '--graph --simplify-by-decoration prune branch B' '
'
test_expect_success '--graph --full-history -- bar.txt' '
- check_graph --full-history --all -- bar.txt <<-\EOF
+ lib_test_check_graph --full-history --all -- bar.txt <<-\EOF
* A7
* A6
|\
@@ -148,7 +143,7 @@ test_expect_success '--graph --full-history -- bar.txt' '
'
test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
- check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
+ lib_test_check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
* A7
* A6
|\
@@ -161,7 +156,7 @@ test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
'
test_expect_success '--graph -- bar.txt' '
- check_graph --all -- bar.txt <<-\EOF
+ lib_test_check_graph --all -- bar.txt <<-\EOF
* A7
* A5
* A3
@@ -172,7 +167,7 @@ test_expect_success '--graph -- bar.txt' '
'
test_expect_success '--graph --sparse -- bar.txt' '
- check_graph --sparse --all -- bar.txt <<-\EOF
+ lib_test_check_graph --sparse --all -- bar.txt <<-\EOF
* A7
* A6
* A5
@@ -189,7 +184,7 @@ test_expect_success '--graph --sparse -- bar.txt' '
'
test_expect_success '--graph ^C4' '
- check_graph --all ^C4 <<-\EOF
+ lib_test_check_graph --all ^C4 <<-\EOF
* A7
* A6
* A5
@@ -202,7 +197,7 @@ test_expect_success '--graph ^C4' '
'
test_expect_success '--graph ^C3' '
- check_graph --all ^C3 <<-\EOF
+ lib_test_check_graph --all ^C3 <<-\EOF
* A7
* A6
|\
@@ -220,7 +215,7 @@ test_expect_success '--graph ^C3' '
# that important, but this test depends on it. If the ordering ever changes
# in the code, we'll need to update this test.
test_expect_success '--graph --boundary ^C3' '
- check_graph --boundary --all ^C3 <<-\EOF
+ lib_test_check_graph --boundary --all ^C3 <<-\EOF
* A7
* A6
|\
--
2.54.0
^ permalink raw reply related
* [PATCH v4 0/2] graph: indent visual roots in graph
From: Pablo Sabater @ 2026-06-12 13:48 UTC (permalink / raw)
To: git
Cc: ayu.chandekar, chandrapratap3519, christian.couder, gitster,
jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
Pablo Sabater
In-Reply-To: <20260427102838.44867-1-pabloosabaterr@gmail.com>
When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.
A fix for this issue was already attempted [1] a while ago.
This series adds indentation to the visual root commits, so they cannot be
vertically adjacent anymore making it easier to identify them.
before indentation:
* A
* B1
* B2
* C1
* C2
after indentation:
* A
* B1
\
* B2
* C1
* C2
Indents the visual root commits that have still commits to show after them, and
if they have children it connects them with an edge at a new row.
If there are multiple visual roots adjacent in history, the indentation starts
with the second one, avoiding redundant indentation of the first one and cascades
after the second.
* A
* B
* C
* D1
* D2
This series first commit is a cleanup that brings a common function from t4215
and t6016 to a graph functions file which they both use, so the new test file
for indentation, t4218, can use it as well.
The lookahead used to set the cascading and avoid extra indentation is not
completely reliable, as the walker goes through the commits it simplifies the
history of the current commit and its parents, but it doesn't simplify it
for the next unrelated or the grandparents. When the walker simplifies the
history, it removes filtered commits from the history and sets its flags.
When the next commit is an unrelated commit and its parents will be filtered
out, for the lookahead the commit is still a child of, it cannot know that the
next commit once simplified (advancing the walker) it will become a visual root.
This makes the lookahead fail, failing to set the cascading and starting it
with the first visual root, carrying an extra indent for the cascade.
given:
* A unrelated (visual root)
* B child of C
* C visual root WILL BE FILTERED OUT
* D unrelated (visual root)
the actual output is:
* A
* B
* D
A test has been added to t4218 and a NEEDSWORK to the lookahead function to
document this edge case but I'm not that familiar with revision.c. Maybe there's
a better way to make the lookahead more reliable.
[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
V3 DIFF:
- Completly changes the approach to indent the visual roots instead of the
commits after the visual roots.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Pablo Sabater (2):
lib-log-graph: move check_graph function
graph: indent visual root in graph
graph.c | 262 +++++++++++++++++
t/lib-log-graph.sh | 5 +
t/meson.build | 1 +
t/t4215-log-skewed-merges.sh | 33 +--
t/t4218-log-graph-indentation.sh | 455 +++++++++++++++++++++++++++++
t/t6016-rev-list-graph-simplify-history.sh | 25 +-
6 files changed, 747 insertions(+), 34 deletions(-)
---
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
change-id: 20260612-ps-pre-commit-indent-39ca72816382
Best regards,
--
Pablo Sabater <pabloosabaterr@gmail.com>
^ permalink raw reply
* Automated reviews by AI (was Re: [PATCH 0/5] Duplicate entry hardening)
From: Christian Couder @ 2026-06-12 13:29 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Junio C Hamano, Elijah Newren via GitGitGadget, git,
Elijah Newren, Konstantin Ryabitsev, Taylor Blau
In-Reply-To: <ah2PLBluBFy44AQI@pks.im>
On Tue, Jun 2, 2026 at 8:16 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Jun 01, 2026 at 09:33:10PM +0900, Junio C Hamano wrote:
> > This is a fix to an important corner of our system, but somehow left
> > in "Needs review" state for much longer than I would have liked, so
> > even though I am officially on vacation ;-), I took some time to
> > read these through (by the way it was a pleasant read, thank you).
>
> Honestly, I always shy away from the merge-related subsystems. It has a
> lot of subtleties that I don't have any experience with, so I never
> really consider my input to be helpful here.
>
> > I wonder if we create a rule like
> >
> > Those of you who have more than 30 commits in our project are
> > expected to review one topic (or more) from other contributors
> > for every three patches you send and ask for reviews by others.
>
> Heh, that would make me condense patch series into fewer patches ;)
>
> > it would help balance the patch vs review ratio, perhaps?
>
> It's a good question. I typically try to aim for reviewing series on the
> mailing list at least every second day, and I always encourage other
> folks in my team to do the same. But recently I (well, rather we)
> haven't really been able to due to the current situation at GitLab,
> which forces us to put almost all of our focus towards a different
> project for a while.
>
> Overall I agree that everyone who is a core contributor should also make
> reviews part of their regular worflow. At least for corporate
> contributors that might also make it easier to communicate this to their
> respective employers. Regardless of that, my expectation is that there
> will be times where it works well, and other times where it works less
> well.
Sashiko (https://github.com/sashiko-dev/sashiko) is used these days by
Linux kernel developers and seems to work well for them.
At GitLab and probably in other companies, some of us also use AI to
review our work before sending it to the mailing list. And yeah, it
helps find issues before our patches reach the mailing list.
In the same way as we require that patches must pass CI, do we want to
require that patches "pass" an AI review before they get accepted?
The benefit would be that it would hopefully catch a lot of trivial
things like indentation, typos/grammos, etc, and a lot of things a bit
more difficult to spot like memory issues. Perhaps with some amount of
prompting/configuration (for example pointing it at our
CodingGuidelines and SubmittingPatches) it could also catch issues
like style issues, commits that do too many things, refactoring
opportunities, etc.
We would likely still require at least one human review (by someone
who is not the maintainer) to validate architectural decisions, to
make sure it goes in the same direction as other efforts, and perhaps
also to make sure that AI suggestions were properly handled by the
patch author.
If we decide to require it, then there are a lot of questions that we
will have to answer.
Do we want to have our own system somehow managed by us or would we be
happy to use existing systems already in place in some companies as
long as we can still tweak them in some ways, like the current CI
systems we use?
If we use existing systems likely at GitLab and GitHub, it might be
more difficult to get coherent results as they might use different
LLMs, but maybe it could help tighten our docs to make sure everyone
is aligned, and we could get better reviews by using multiple systems
because an LLM might find an issue that the other LLM missed.
Do we want an AI review right after a patch is posted or only if there
is no human review in the next X days?
Also what if the AI makes a long concrete suggestion to improve on the
patches? Could that be incompatible with our AI policy to apply it?
Should we try to prevent the AI from making such a suggestion in the
first place?
I haven't looked at how Sashiko is used for the kernel, but maybe
there will need to be some kinds of restrictions/authentications to
avoid potential abuse.
^ permalink raw reply
* Re: [PATCH v3 2/2] compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
From: Patrick Steinhardt @ 2026-06-12 13:27 UTC (permalink / raw)
To: Dominik Loidolt; +Cc: git, gitster, asedeno, asedeno, avarab
In-Reply-To: <20260608124419.38905-2-dominik.loidolt@univie.ac.at>
On Mon, Jun 08, 2026 at 02:44:19PM +0200, Dominik Loidolt wrote:
> Replace the glibc-style bit-shift version comparison with an explicit
> major/minor comparison. This is easier to read and is consistent with
> the format already used by GIT_CLANG_PREREQ() and many BSD
It's a bit funny to use `GIT_CLANG_PREREQ()` as an argument here as
we've just added it in the preceding commit.
> <sys/cdefs.h> headers.
>
> This has no runtime impact, as the macro is evaluated at compile time.
> It is also more future-proof, as it no longer assumes that GCC version
> components stay below 65536.
I feel like all the message needs to say is "let's do it for
consistency, and it's easier to read". That would've been sufficient,
whereas this argument here feels a bit thin.
Doesn't matter much though, and I think ultimately the message is fine
as-is, even though the reasoning is a bit funny.
> diff --git a/compat/posix.h b/compat/posix.h
> index ffdfd91c7b..deefc43f28 100644
> --- a/compat/posix.h
> +++ b/compat/posix.h
> @@ -4,22 +4,24 @@
> #define _FILE_OFFSET_BITS 64
>
> /*
> - * Derived from Linux "Features Test Macro" header
> - * Convenience macros to test the versions of gcc (or
> - * a compatible compiler).
> + * Convenience macros to test the versions of GCC (or a compatible compiler).
> * Use them like this:
> * #if GIT_GNUC_PREREQ (2,8)
> - * ... code requiring gcc 2.8 or later ...
> + * ... code requiring GCC 2.8 or later ...
> * #endif
> *
> + * Note that Clang and other compilers define __GNUC__ for compatibility; use
> + * GIT_CLANG_PREREQ() to check for specific Clang versions.
> + *
> * This macro of course is not part of POSIX, but we need it for the UNUSED
> * macro which is used by some of our POSIX compatibility wrappers.
> -*/
> + */
It would've been nice to either move these changes into a preparatory
commit or at least mention them
> #if defined(__GNUC__) && defined(__GNUC_MINOR__)
> # define GIT_GNUC_PREREQ(maj, min) \
> - ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
> + ((__GNUC__ > (maj)) || \
> + (__GNUC__ == (maj) && __GNUC_MINOR__ >= (min)))
> #else
> - #define GIT_GNUC_PREREQ(maj, min) 0
> +# define GIT_GNUC_PREREQ(maj, min) 0
> #endif
The change itself makes sense to me.
I'm not sure myself whether this could use another reroll. It's all just
nits, and the intent is clear enough.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 2/4] pack-objects: support reachability bitmaps with `--path-walk`
From: Derrick Stolee @ 2026-06-12 13:24 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <ffad584a43ebf3cb2138e8dce7daef84ab72712f.1780438896.git.me@ttaylorr.com>
On 6/2/2026 6:21 PM, Taylor Blau wrote:
> When 'pack-objects' is invoked with '--path-walk', it prevents us from
> using reachability bitmaps.
My earlier response focused on the _use_ of bitmaps when creating a
packfile, but your patch also enables _writing_ bitmaps with the
--path-walk option, which is significant and potentially more
interesting from my perspective: we have evidence that --path-walk
can produce significantly smaller packfiles than the standard
algorithm, and once those packfiles are created we can benefit from
that size in later packfile creation steps by reusing those deltas.
In this sense, I think the _writing_ is more important during a
repack scenario. The fetch/clone scenarios can benefit directly even
without integrating --path-walk with --use-bitmap-indexes.
> * A path-walk repack that writes bitmaps does not give the bitmap
> selector any commits, because path-walk reveals commits through
> `add_objects_by_path()` rather than through `show_commit()`, where
> `index_commit_for_bitmap()` is normally called.
...
> * On the writing side: teach the path-walk object callback to call
> `index_commit_for_bitmap()` for commits that it adds to the pack.
> That gives the bitmap selector the commit candidates it would have
> seen from the regular traversal.
My earlier reply to this patch was focused on the performance results
when using the "reading bitmaps" case, and I expressed suspicion about
the "exact" sizes of the packfiles.
Even more important here is that we have demonstrated examples of repos
that change their packfile size when using the --path-walk method. We
should demonstrate that the size continues to shrink with --path-walk
even when producing a matching .bitmap file with --write-bitmap-index.
The other thing that I notice here is that the bitmaps will need to
compute their reachable object set independently from the path-walk
algorithm. But I suppose that already happens separately from the
revision-walk approach that normally produces the packfile contents.
Note: A lot of my thoughts around asking for more evidence here is
that this patch seems suspiciously simple for integrating two
complicated features. The test suite (especially with
GIT_TEST_PACK_PATH_WALK=1) helps to guarantee that the result is
_correct_, but with performance features like this it's not enough to
"just" be correct. I want to see that we're having the intended
results.
From my perspective, the point of integrating these two things are:
1. Reachability bitmaps make it much faster to discover the reachable
set and reuse bits of existing packfiles. (Your performance table
demonstrates this is true.)
2. The --path-walk option can shrink packfile sizes by grouping
trees and blobs by path before those paths collide in the name-hash
sort. (I haven't seen evidence that this is happening.)
With evidence of (1) and not (2), it's not clear from the data that
these features are integrating completely. Without looking at the
code, those numbers would be the same if we had instead swapped the
preference of "the --path-walk option disables bitmaps" to "bitmaps
disable --path-walk".
Finally, I'll just note that I don't expect the _bitmaps_ to change
size dramatically. The --path-walk option does change the order of
the objects for its first pass of delta compression, but then uses
the (name-hash, size) sort to finalize the object ordering, so the
final object ordering _should_ be the same (unless I'm mistaken, in
which case the bitmaps could change size due to bitmap compression
concerns).
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v4 06/16] midx: support custom `--base` for incremental MIDX writes
From: Junio C Hamano @ 2026-06-12 13:21 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Taylor Blau, git, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <aiuaf3fKJ6kIITrf@szeder.dev>
SZEDER Gábor <szeder.dev@gmail.com> writes:
>> + layer="$(git multi-pack-index write --bitmap --incremental \
>> + --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
>
> There is no 'nth_line' helper function in this test script.
Good eyes. It has been there in the file next door t5335 since
February, but not available here in t5334.
^ permalink raw reply
* Re: [PATCH v2 2/4] pack-objects: support reachability bitmaps with `--path-walk`
From: Derrick Stolee @ 2026-06-12 13:03 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <ffad584a43ebf3cb2138e8dce7daef84ab72712f.1780438896.git.me@ttaylorr.com>
On 6/2/2026 6:21 PM, Taylor Blau wrote:
> As a result, we can see significantly reduced pack sizes from p5311
> before this commit:
I mentioned this before, but the pack _sizes_ aren't changing in this
example. We are computing them more quickly, though.
> Test HEAD^ HEAD
> ----------------------------------------------------------------------------------
> 5311.38: server (1 days, --path-walk) 2.56(2.52+0.03) 0.01(0.01+0.00) -99.6%
> 5311.39: size (1 days, --path-walk) 123.9K 123.9K +0.0%
> 5311.40: client (1 days, --path-walk) 0.00(0.01+0.00) 0.00(0.00+0.00) =
> 5311.42: server (2 days, --path-walk) 2.57(2.52+0.05) 0.01(0.01+0.00) -99.6%
> 5311.43: size (2 days, --path-walk) 123.9K 123.9K +0.0%
> 5311.44: client (2 days, --path-walk) 0.00(0.00+0.00) 0.00(0.00+0.00) =
> 5311.46: server (4 days, --path-walk) 2.58(2.51+0.07) 0.01(0.01+0.00) -99.6%
> 5311.47: size (4 days, --path-walk) 123.9K 123.9K +0.0%
> 5311.48: client (4 days, --path-walk) 0.00(0.00+0.00) 0.00(0.00+0.00) =
> 5311.50: server (8 days, --path-walk) 2.58(2.53+0.04) 0.02(0.02+0.00) -99.2%
> 5311.51: size (8 days, --path-walk) 152.4K 152.4K +0.0%
> 5311.52: client (8 days, --path-walk) 0.00(0.01+0.00) 0.00(0.01+0.00) =
> 5311.54: server (16 days, --path-walk) 2.58(2.52+0.05) 0.03(0.02+0.00) -98.8%
> 5311.55: size (16 days, --path-walk) 205.3K 205.3K +0.0%
> 5311.56: client (16 days, --path-walk) 0.01(0.01+0.00) 0.01(0.01+0.00) +0.0%
> 5311.58: server (32 days, --path-walk) 2.59(2.53+0.06) 0.03(0.03+0.00) -98.8%
> 5311.59: size (32 days, --path-walk) 209.3K 209.3K +0.0%
> 5311.60: client (32 days, --path-walk) 0.01(0.02+0.00) 0.01(0.02+0.00) +0.0%
> 5311.62: server (64 days, --path-walk) 2.70(2.76+0.06) 0.16(0.24+0.04) -94.1%
> 5311.63: size (64 days, --path-walk) 4.1M 4.1M +0.0%
> 5311.64: client (64 days, --path-walk) 0.44(0.50+0.02) 0.44(0.51+0.02) +0.0%
> 5311.66: server (128 days, --path-walk) 2.88(3.20+0.05) 0.34(0.65+0.05) -88.2%
> 5311.67: size (128 days, --path-walk) 9.0M 9.0M -0.0%
> 5311.68: client (128 days, --path-walk) 0.93(1.22+0.07) 0.93(1.20+0.08) +0.0%
Since we are testing --path-walk on both sides, the change across this
commit is that we are using the bitmaps for the "counting objects" phase
and then potentially using the --path-walk algorithm to construct the
packfile.
The fact that the packfile sizes are _identical_ is suspicious to me. I'd
expect some amount of difference here due to the change in algorithm. It's
possible that this could be explained by the repository shape not getting
any benefit from --path-walk because there are no name-hash collisions to
worry about.
The one thing that might be hinting towards _some_ difference is that the
relative sizes are showing as both "+0.0%" and "-0.0%", so perhaps the
exact sizes do have differences that are hidden behind the human-readable
sizes: 4.1M -> 4.1M is +0.0% but 9.0M -> 9.0M is -0.0%.
> We get the same size of output pack, but this commit allows us to do so
> in a significantly shorter amount of time.
Ok, you have the correct interpretation here, just a lingering typo in
the earlier sentence before the table.
> Intuitively, we're generating
> the same pack (hence the unchanged 'test_size' output from run to run),
> but varying how we get there. Before this commit, pack-objects prefers
> '--path-walk' to '--use-bitmap-index', so we generate the output pack by
> performing a normal '--path-walk' traversal. With this commit, we are
> operating over a *repacked* state (that itself was done with a
> '--path-walk' traversal), but are able to perform pack-reuse on that
> repacked state via bitmaps.
And I wonder if the test setup creates a situation where we are always
reusing deltas from the underlying packfile, so the --path-walk algorithm
isn't doing anything to help with delta compression at this point and the
difference in this patch is that we are replacing the object reachability
calculation entirely with bitmaps.
I suppose what I'm really worried about is that I'm hoping to see some
evidence from a large-scale test that demonstrates that the two algorithms
are working in tandem in a non-trivial way. I haven't seen it yet, but I
also don't have evidence that they _aren't_ working together.
Thanks,
-Stolee
^ permalink raw reply
* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Derrick Stolee @ 2026-06-12 12:52 UTC (permalink / raw)
To: Kristofer Karlsson, git
In-Reply-To: <CAL71e4Mp7ewv0UGS8j=iTq6quyxLXzrr0uNDbWR8JKaOsTSVyA@mail.gmail.com>
On 6/12/2026 7:15 AM, Kristofer Karlsson wrote:
> I have an idea to make it faster for fetching all merge-bases for
> common flows in large repos, as long as the commit graph is
> reasonably up to date.
>
> The key part is the exit condition in paint_down_to_common.
> Instead of waiting for the queue to only contain stale entries,
> it is enough to wait for one of the sides to be exhausted,
> i.e. side 1 is exhausted if no more commits exist in the
> traversal queue flagged with only PARENT1. For example, if
> the two sides are origin/HEAD and a small PR branch, the PR
> branch will quickly become exhausted at the merge-base, while
> the main side will continue.
Generally, you'd replace the queue_has_nonstale() condition
with a more generic queue_can_halt() condition.
> Now you may ask: why is that a safe condition?
>
> The traversal in paint_down_to_common has two logical phases
> due to the priority queue ordering:
>
> 1. Process all commits with infinite generation numbers.
> This includes all commits when there is no commit-graph.
> 2. Process all commits with finite generation numbers.
>
> These happen in strict order -- all INFINITY commits are popped
> before any finite-generation commit.
>
> The optimization only applies after the walk enters the second phase.
> In the first phase, the traversal behaves exactly as today
> and uses the existing termination condition.
This would mean that queue_can_halt() would need to know the
following:
1. If we peek at the top of the queue, is that a commit with
infinite generation number? If so, then we can only use
queue_has_nonstale().
2. Otherwise, we know that all commits in the queue are ordered
topologically and can use a different, faster check. To start,
we need to keep going as long as at least one commit has only
one side
> In the second phase, traversal follows strict topological
> order -- descendants are processed before ancestors. Paint flags
> propagate from each processed commit to its parents, which have
> strictly lower generation and are therefore not yet examined.
>
> A new merge-base candidate can only form when a PARENT1-only path
> meets a PARENT2-only path. Once a commit acquires both paint flags
> in this phase, any descendant carrying both paint flags would
> already have been processed.
>
> Once one side is exhausted from the queue, no new meeting between
> pure sides can occur. Any commit that subsequently acquires both
> paint flags must inherit them from a commit that already had both
> flags -- it is deeper in the graph and cannot affect the final
> merge-base set. We can stop.
The important thing to realize at this point is that commits in the
queue have received flags from their children (children were walked
topologically and "push" flags to their parents).
The STALE bit is pushed from commits that have bits for both sides
of the merge. This isn't something that we can learn from just
walking each side: we need some amount of walking within the
intersection.
This doesn't matter if we are looking for a single merge base, but
when we want the full set of independent merge bases, then the STALE
bit becomes very important.
> On a large monorepo with previously expensive merge-base and
> merge-tree queries, I observed speedups ranging from roughly 300x
> to 1000x. The nice thing is that this works for merge-base --all
> and every internal caller of paint_down_to_common -- we no longer
> have to restrict the optimization to finding just the first merge-base.
>
> Does the correctness argument above hold?
I think that it doesn't work when trying to get all merge bases. It
requires
A X
/| __/|
| |/ |
| B |
| | |
..........
| | __/
\|/
C
In this example, B can reach C through some long list of commits.
This makes B (and X) have much higher generation number than C.
After exhausting both sides of A...X, we have B and C in the queue
with both side bits and neither are stale. But we need to walk
from B to C to discover that C should be stale.
> Happy to come back with a patch later if the logic holds and the
> overall approach is wanted.
You are identifying a point where optimizations are possible, based
on your measurements of the time spent in this walk waiting for
queue_has_nonstale() to end the loop. Specifically, the cost of using
a BFS approach is costing time.
One place that I would recommend here is to take the work you are
doing to investigate the behavior of tips_reachable_from_bases()
or get_reachable_subset() to see if we can use a DFS-based approach
_in this case_ where we have exhausted both side and are only caring
about the STALE bit checking these cases.
Remember that the DFS idea only helps in the case where we find a
path between commits (B to C in this case) without walking all of the
commits above the minimum generation (generation of C). In an alternate
case where B and C are truly independent, this would not save any time.
But these "they are mutually unreachable" cases always require walking
the full set based on the generation number. The good news is that the
vast majority of cases do not actually have multiple independent merge
bases, so there is potential here.
Thanks,
-Stolee
^ permalink raw reply
* Re: [GIT PULL] git-gui: repo discovery with rev-parse; pick and gui subcommands; silent make -s
From: Junio C Hamano @ 2026-06-12 12:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <ca428e6e-c840-4ee6-9fcf-39889fc07400@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> The following changes since commit bb52cdac6254c006e06bf0bb820268dcf024fc22:
>
> git-gui: grey out comment lines in commit message (2026-03-04 08:04:37 +0100)
>
> are available in the Git repository at:
>
> https://github.com/j6t/git-gui.git master
>
> for you to fetch changes up to 1b2c2a2edbaa1638becef4c3755b3e0633b9c304:
>
> Merge branch 'ml/repo-discovery' (2026-06-12 11:05:28 +0200)
Thanks. Will pull together with the gitk updates.
^ permalink raw reply
* Re: [PATCH v3] ref-filter: restore prefix-scoped iteration
From: Patrick Steinhardt @ 2026-06-12 11:48 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, ZheNing Hu
In-Reply-To: <20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.com>
On Wed, Jun 10, 2026 at 05:29:49AM -0700, Tamir Duberstein wrote:
> dabecb9db2 (for-each-ref: introduce a '--start-after' option,
> 2025-07-15) changed branch, remote-tracking branch, and tag enumeration
> from constructing an iterator with the namespace prefix to constructing
> an unscoped iterator and seeking to the prefix.
>
> The files backend constructs its loose-ref iterator with cache priming
> enabled. cache_ref_iterator_begin() immediately applies the construction
> prefix through cache_ref_iterator_set_prefix(), reading loose refs
> beneath it before packed refs are opened. An empty prefix therefore
> reads every loose ref, and a later seek cannot undo that I/O.
>
> For these single-kind filters, construct the iterator with the namespace
> prefix when start_after is not set. Keep the existing unscoped
> construction for start_after, whose seek position may differ from the
> namespace prefix.
>
> With 10,000 unrelated loose refs, the p6300 tests improve as follows:
>
> before after
> branch 2.74 s 0.11 s
> branch --remotes 2.81 s 0.12 s
> tag 3.01 s 0.11 s
>
> Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
> Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/
> Link: https://lore.kernel.org/r/CAOLa=ZRHKNNymXGk31YgECjUmF9nZ8GsPUdQb7aKBH5DKMz7=w@mail.gmail.com
I honestly have no idea what you want to say with these links, as they
seem to just link to random reviews mails when the above mentioned
commit was reviewed. In general, we typically try to embed references
like this into the explanation, like:
In [1], we discussed... and this is relevant because of ...
[1]: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
Just dropping the links as-is without much of an explanation isn't
helpful.
> diff --git a/ref-filter.c b/ref-filter.c
> index 1da4c0e60d..9b04e3af85 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
>
> if (prefix) {
> struct ref_iterator *iter;
> + struct ref_store *store = get_main_ref_store(the_repository);
>
> - iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
> - "", NULL, 0, 0);
> -
> - if (filter->start_after)
> + if (filter->start_after) {
> + iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
> ret = start_ref_iterator_after(iter, filter->start_after);
> - else
> - ret = ref_iterator_seek(iter, prefix,
> - REF_ITERATOR_SEEK_SET_PREFIX);
> + } else {
> + iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
> + }
>
> if (!ret)
> ret = do_for_each_ref_iterator(iter, fn, cb_data);
The patch itself seems sensible to me.
> diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
> index fa7289c752..ed9c1c6a19 100755
> --- a/t/perf/p6300-for-each-ref.sh
> +++ b/t/perf/p6300-for-each-ref.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
>
> -test_description='performance of for-each-ref'
> +test_description='performance of ref-filter users'
> . ./perf-lib.sh
>
> test_perf_fresh_repo
> @@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
> '
> run_tests "packed"
>
> +test_expect_success REFFILES 'setup many unrelated loose refs' '
> + git init scoped &&
> + test_commit -C scoped --no-tag base &&
> + test_seq $ref_count_per_type |
> + sed "s,.*,update refs/custom/unrelated_& HEAD," |
> + git -C scoped update-ref --stdin &&
> + git -C scoped update-ref refs/remotes/origin/main HEAD &&
> + git -C scoped update-ref refs/tags/only HEAD
> +'
I've already called this out before on other patches, but the REFFILES
prerequisite just doesn't make any sense here as this test logic is
generic.
Patrick
^ permalink raw reply
* Re: [PATCH 2/6] SubmittingPatches: discuss non-ident trailers
From: Patrick Steinhardt @ 2026-06-12 11:35 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <non-ident_trailers.8f5@msgid.xyz>
On Thu, Jun 11, 2026 at 12:22:45AM +0200, kristofferhaugsbakk@fastmail.com wrote:
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 0b12badf86d..51c308a89a8 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -474,7 +474,10 @@ These are the common trailers in use:
>
> While you can also create your own trailer if the situation warrants it, we
> encourage you to instead use one of the common trailers in this project
> -highlighted above.
> +highlighted above. A trailer that credits someone might be more likely
> +to be accepted since these are the most common ones. But another kind of
> +trailer might be relevant, for example to link to an issue tracker
> +belonging to a downstream project that is affected by a bug in Git.
Hm, I wonder whether this is a bit too vague to really be helpful for a
newcomer. Instead of alluding to such trailers, wouldn't it be
preferable if we added those as actual examples to the list of known
trailers and then tell folks that they can invent their own ones if
there is a good reason to do so?
Patrick
^ permalink raw reply
* Re: [PATCH v2 0/7] setup: drop global state
From: Patrick Steinhardt @ 2026-06-12 11:25 UTC (permalink / raw)
To: Toon Claes; +Cc: Justin Tobler, git
In-Reply-To: <87ldckyygk.fsf@emacs.iotcl.com>
On Fri, Jun 12, 2026 at 10:06:35AM +0200, Toon Claes wrote:
> Justin Tobler <jltobler@gmail.com> writes:
>
> > I find the additional explaination here quite helpful. Thanks.
>
> Yeah, hard to follow series looking at the code only, but commit
> messages make more bearable.
>
> > The changes in this version of the series looks good to me.
>
> I've only reviewed v2 and I agree this version looks good.
There's only a single change to a commit message I've queued, so I'll
for now not send another iteration.
Thanks for your review!
Patrick
^ permalink raw reply
* [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-12 11:15 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee
Hi! I previously sent a patch[1] to optimize paint_down_to_common
for the single merge-base case. I believe I have found a stronger
optimization, but before sending a patch I wanted to discuss the
correctness argument.
The main problem to solve is that computing merge-bases is slow today
in some scenarios, especially large monorepos with complex graphs.
This affects multiple operations, including merge-base and merge-tree.
The previous patch improved it for the special case of the
merge-base being part of the commit-graph and the caller only
needing to know about one merge-base.
I have an idea to make it faster for fetching all merge-bases for
common flows in large repos, as long as the commit graph is
reasonably up to date.
The key part is the exit condition in paint_down_to_common.
Instead of waiting for the queue to only contain stale entries,
it is enough to wait for one of the sides to be exhausted,
i.e. side 1 is exhausted if no more commits exist in the
traversal queue flagged with only PARENT1. For example, if
the two sides are origin/HEAD and a small PR branch, the PR
branch will quickly become exhausted at the merge-base, while
the main side will continue.
Now you may ask: why is that a safe condition?
The traversal in paint_down_to_common has two logical phases
due to the priority queue ordering:
1. Process all commits with infinite generation numbers.
This includes all commits when there is no commit-graph.
2. Process all commits with finite generation numbers.
These happen in strict order -- all INFINITY commits are popped
before any finite-generation commit.
The optimization only applies after the walk enters the second phase.
In the first phase, the traversal behaves exactly as today
and uses the existing termination condition.
In the second phase, traversal follows strict topological
order -- descendants are processed before ancestors. Paint flags
propagate from each processed commit to its parents, which have
strictly lower generation and are therefore not yet examined.
A new merge-base candidate can only form when a PARENT1-only path
meets a PARENT2-only path. Once a commit acquires both paint flags
in this phase, any descendant carrying both paint flags would
already have been processed.
Once one side is exhausted from the queue, no new meeting between
pure sides can occur. Any commit that subsequently acquires both
paint flags must inherit them from a commit that already had both
flags -- it is deeper in the graph and cannot affect the final
merge-base set. We can stop.
On a large monorepo with previously expensive merge-base and
merge-tree queries, I observed speedups ranging from roughly 300x
to 1000x. The nice thing is that this works for merge-base --all
and every internal caller of paint_down_to_common -- we no longer
have to restrict the optimization to finding just the first merge-base.
Does the correctness argument above hold?
Happy to come back with a patch later if the logic holds and the
overall approach is wanted.
Thanks,
Kristofer
[1] https://lore.kernel.org/git/pull.2109.v4.git.1778504352.gitgitgadget@gmail.com/
^ permalink raw reply
* [PATCH 2/2] push: suggest <remote> <branch> for a slash slip
From: Harald Nordgren via GitGitGadget @ 2026-06-12 11:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2331.git.git.1781262619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git push origin/main" is treated as a repository and dies with
"'origin/main' does not appear to be a git repository", with no hint
that a space was meant instead of a slash.
When the argument is not an existing path or configured remote but its
part before the first slash names one, suggest the intended
"git push <remote> <branch>" form. The suggestion is shown as advice so
it can be silenced with advice.pushRepoLooksLikeRef.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/advice.adoc | 5 +++++
advice.c | 1 +
advice.h | 1 +
builtin/push.c | 26 +++++++++++++++++++++++++-
t/t5529-push-errors.sh | 31 +++++++++++++++++++++++++++++++
5 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..fa77a5110e 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -90,6 +90,11 @@ all advice messages.
Shown when linkgit:git-push[1] rejects a forced update of
a branch when its remote-tracking ref has updates that we
do not have locally.
+ pushRepoLooksLikeRef::
+ Shown when the repository given to linkgit:git-push[1] is not
+ a configured remote but looks like a `<remote>/<branch>` ref,
+ suggesting that the remote and branch be given as separate
+ arguments.
pushUnqualifiedRefname::
Shown when linkgit:git-push[1] gives up trying to
guess based on the source and destination refs what
diff --git a/advice.c b/advice.c
index 0018501b7b..63bf8b0c5f 100644
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
[ADVICE_PUSH_NON_FF_CURRENT] = { "pushNonFFCurrent" },
[ADVICE_PUSH_NON_FF_MATCHING] = { "pushNonFFMatching" },
[ADVICE_PUSH_REF_NEEDS_UPDATE] = { "pushRefNeedsUpdate" },
+ [ADVICE_PUSH_REPO_LOOKS_LIKE_REF] = { "pushRepoLooksLikeRef" },
[ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName" },
[ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected" },
[ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward" }, /* backwards compatibility */
diff --git a/advice.h b/advice.h
index 8def280688..66f6cd6a77 100644
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
ADVICE_PUSH_NON_FF_CURRENT,
ADVICE_PUSH_NON_FF_MATCHING,
ADVICE_PUSH_REF_NEEDS_UPDATE,
+ ADVICE_PUSH_REPO_LOOKS_LIKE_REF,
ADVICE_PUSH_UNQUALIFIED_REF_NAME,
ADVICE_PUSH_UPDATE_REJECTED,
ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..c21febadbe 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -8,6 +8,7 @@
#include "advice.h"
#include "branch.h"
#include "config.h"
+#include "dir.h"
#include "environment.h"
#include "gettext.h"
#include "hex.h"
@@ -744,6 +745,29 @@ int cmd_push(int argc,
if (repo) {
if (!add_remote_or_group(repo, &remote_group)) {
+ const char *slash = strchr(repo, '/');
+ struct remote *r;
+
+ /*
+ * A "<remote>/<branch>" argument that does not name
+ * a path is likely a slip for the separate
+ * "<remote> <branch>" form, so suggest that instead.
+ */
+ if (slash && slash[1] && !file_exists(repo)) {
+ struct strbuf name = STRBUF_INIT;
+
+ strbuf_add(&name, repo, slash - repo);
+ if (remote_is_configured(remote_get(name.buf), 0)) {
+ int code = die_message(_("'%s' is not a valid push target"), repo);
+ advise_if_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF,
+ _("Did you mean to use: git push %s %s?"),
+ name.buf, slash + 1);
+ strbuf_release(&name);
+ exit(code);
+ }
+ strbuf_release(&name);
+ }
+
/*
* Not a configured remote name or group name.
* Try treating it as a direct URL or path, e.g.
@@ -753,7 +777,7 @@ int cmd_push(int argc,
* from the URL so the loop below can handle it
* identically to a named remote.
*/
- struct remote *r = pushremote_get(repo);
+ r = pushremote_get(repo);
if (!r)
die(_("bad repository '%s'"), repo);
string_list_append(&remote_group, r->name);
diff --git a/t/t5529-push-errors.sh b/t/t5529-push-errors.sh
index 80b06a0cd2..cfb294305d 100755
--- a/t/t5529-push-errors.sh
+++ b/t/t5529-push-errors.sh
@@ -54,6 +54,37 @@ test_expect_success 'detect empty remote with targeted refspec' '
grep "fatal: bad repository ${SQ}${SQ}" stderr
'
+test_expect_success 'suggest <remote> <branch> for a <remote>/<branch> slip' '
+ test_must_fail git push origin/main 2>stderr &&
+ grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
+ grep "hint: Did you mean to use: git push origin main?" stderr &&
+ test_must_fail git -c advice.pushRepoLooksLikeRef=false push origin/main 2>stderr &&
+ ! grep "Did you mean" stderr
+'
+
+test_expect_success 'suggest <remote> <branch> when the branch has slashes' '
+ test_must_fail git push origin/feature/x 2>stderr &&
+ grep "hint: Did you mean to use: git push origin feature/x?" stderr
+'
+
+test_expect_success 'no suggestion when prefix is not a configured remote' '
+ test_must_fail git push not-a-remote/main 2>stderr &&
+ ! grep "Did you mean" stderr
+'
+
+test_expect_success 'no suggestion for a trailing slash with no branch' '
+ test_must_fail git push origin/ 2>stderr &&
+ ! grep "Did you mean" stderr
+'
+
+test_expect_success 'no suggestion when the argument is an existing path' '
+ test_when_finished "rm -rf origin" &&
+ git init --bare origin/main &&
+ git push origin/main HEAD:refs/heads/pushed 2>stderr &&
+ ! grep "Did you mean" stderr &&
+ git -C origin/main rev-parse --verify refs/heads/pushed
+'
+
test_expect_success 'detect ambiguous refs early' '
git branch foo &&
git tag foo &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip
From: Harald Nordgren via GitGitGadget @ 2026-06-12 11:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2331.git.git.1781262619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git branch --set-upstream-to origin main" reads the trailing word as
the local branch to operate on and dies with "branch 'main' does not
exist", pointing at the wrong problem.
When that branch is missing and "<remote>/<branch>" names a real
remote-tracking ref, suggest the intended
"git branch --set-upstream-to=<remote>/<branch>" form.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 17 +++++++++++++++++
t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..7ad3efb908 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -957,6 +957,23 @@ int cmd_branch(int argc,
if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
die(_("no commit on branch '%s' yet"), branch->name);
+ if (argc == 1 && !strchr(new_upstream, '/') &&
+ remote_is_configured(remote_get(new_upstream), 0)) {
+ struct strbuf remote_ref = STRBUF_INIT;
+
+ strbuf_addf(&remote_ref, "refs/remotes/%s/%s",
+ new_upstream, argv[0]);
+ if (refs_ref_exists(get_main_ref_store(the_repository),
+ remote_ref.buf)) {
+ int code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+ advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+ _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+ new_upstream, argv[0]);
+ strbuf_release(&remote_ref);
+ exit(code);
+ }
+ strbuf_release(&remote_ref);
+ }
die(_("branch '%s' does not exist"), branch->name);
}
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e7829c2c4b..e2682a83a0 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1022,6 +1022,44 @@ test_expect_success '--set-upstream-to fails on a missing dst branch' '
test_cmp expect err
'
+test_expect_success '--set-upstream-to suggests <remote>/<branch> on slip' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ git update-ref refs/remotes/slip-remote/slip-feature HEAD &&
+ test_must_fail git branch --set-upstream-to slip-remote slip-feature 2>err &&
+ test_grep "takes a single <remote>/<branch> argument" err &&
+ test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip-feature?" err &&
+ test_must_fail git -c advice.setUpstreamFailure=false \
+ branch --set-upstream-to slip-remote slip-feature 2>err &&
+ test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to does not suggest when no matching remote ref' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ test_must_fail git branch --set-upstream-to slip-remote no-such-branch 2>err &&
+ test_grep "branch ${SQ}no-such-branch${SQ} does not exist" err &&
+ test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to to a local branch is not mistaken for a slip' '
+ git branch slip-local-upstream &&
+ git branch slip-local-target &&
+ git branch --set-upstream-to=slip-local-upstream slip-local-target 2>err &&
+ test_grep ! "Did you mean" err &&
+ echo refs/heads/slip-local-upstream >expect &&
+ git config branch.slip-local-target.merge >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--set-upstream-to slip suggestion keeps a slashed branch name' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ git update-ref refs/remotes/slip-remote/slip/feature HEAD &&
+ test_must_fail git branch --set-upstream-to slip-remote slip/feature 2>err &&
+ test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip/feature?" err
+'
+
test_expect_success '--set-upstream-to fails on a missing src branch' '
test_must_fail git branch --set-upstream-to does-not-exist main 2>err &&
test_grep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox