* [PATCH v11 5/7] graph: wrap cascading commits after 4 columns
From: Pablo Sabater @ 2026-07-13 16:44 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@gmail.com>
Currently the visual root commits in a graph cascade indefinitely until
a commit which is not a visual root or the last commit appears.
On filters like --author where one author might contribute mostly on
single patches this can become a visual issue.
Make the cascading wrap after 4 columns.
There are two possible cases of the wrap:
1. No ambiguity:
* A
* B
* C
* D
* E
* F
2. Ambiguous conflict:
If F happens to not be a visual root and E gets wrapped back to the
initial column then E and F would be vertically adjacent. The solution
is to forcefully indent E one level:
* A
* B
* C
* D
* E
* F
* F
The magic number 4 comes as the minimum number of columns to wrap where
the output shows clearly the commits are unrelated and doesn't cause too
much "pyramid" effects
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
graph.c | 22 +++++++++++++++++++++-
t/t4218-log-graph-indentation.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/graph.c b/graph.c
index 087094189f..e3e206170c 100644
--- a/graph.c
+++ b/graph.c
@@ -1042,6 +1042,23 @@ void graph_update(struct git_graph *graph, struct commit *commit)
*/
if (!graph->visual_root_depth && flags.is_next_visual_root)
graph->visual_root_cascade = 1;
+
+ /*
+ * We wrap the cascading at a max of four columns at most, after
+ * that we wrap it back to the initial column.
+ *
+ * This could cause ambiguity in case of the next commit not
+ * being a visual root and be at the initial column after the
+ * first wrap.
+ *
+ * In case of being a non-visual-root the next, stop the
+ * cascading to get the commit indented.
+ */
+ if (!flags.is_next_visual_root &&
+ graph->visual_root_depth &&
+ !(graph->visual_root_depth % 4))
+ graph->visual_root_cascade = 0;
+
graph->visual_root_depth++;
} else {
graph->visual_root_depth = 0;
@@ -1328,8 +1345,11 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
* Each visual column is 2 characters wide.
* Omit the indentation for the first visual
* root in cascade mode.
+ *
+ * Have a max of 4 columns when cascading, after
+ * that wrap it and repeat.
*/
- int padding = (depth - graph->visual_root_cascade) * 2;
+ int padding = ((depth - graph->visual_root_cascade) % 4) * 2;
graph_line_addchars(line, ' ', padding);
graph->width += padding;
}
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
index 60c7d84af7..d4c850c0d4 100755
--- a/t/t4218-log-graph-indentation.sh
+++ b/t/t4218-log-graph-indentation.sh
@@ -511,4 +511,33 @@ test_expect_success '--grep skipped parent makes a visual root' '
EOF
'
+# The cascading wraps after 4 columns and when wraping (column % 4 == 0) if the
+# next is a non visual-root, force indentation to avoid an ambiguous graph
+# (commit 59_A is forcefully indented)
+test_expect_success 'visual root cascading gets wrapped after 4 columns' '
+ create_orphan _58 && test_commit 58_A && test_commit 58_B &&
+ create_orphan _59 && test_commit 59_A &&
+ create_orphan _60 && test_commit 60_A &&
+ create_orphan _61 && test_commit 61_A &&
+ create_orphan _62 && test_commit 62_A &&
+ create_orphan _63 && test_commit 63_A &&
+ create_orphan _64 && test_commit 64_A &&
+ create_orphan _65 && test_commit 65_A &&
+ create_orphan _66 && test_commit 66_A &&
+ create_orphan _67 && test_commit 67_A &&
+ lib_test_check_graph _58 _59 _60 _61 _62 _63 _64 _65 _66 _67 <<-\EOF
+ * 67_A
+ * 66_A
+ * 65_A
+ * 64_A
+ * 63_A
+ * 62_A
+ * 61_A
+ * 60_A
+ * 59_A
+ * 58_B
+ * 58_A
+ EOF
+'
+
test_done
--
2.54.0
^ permalink raw reply related
* [PATCH v11 7/7] graph: add --[no-]graph-indent and log.graphIndent
From: Pablo Sabater @ 2026-07-13 16:44 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@gmail.com>
Some users may prefer to not have graph indentation.
Add "log.graphIndent" config variable to graph_read_config() to read the
default preference. By default is graph indentation is true.
Add --graph-indent and --no-graph-indent options to overwrite the
default preference.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Documentation/config/log.adoc | 4 +++
Documentation/rev-list-options.adoc | 8 ++++++
graph.c | 10 +++++--
revision.c | 9 +++++++
revision.h | 2 ++
t/t4218-log-graph-indentation.sh | 52 +++++++++++++++++++++++++++++++++++++
6 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/log.adoc b/Documentation/config/log.adoc
index 757a7be196..f7dfce69b5 100644
--- a/Documentation/config/log.adoc
+++ b/Documentation/config/log.adoc
@@ -59,6 +59,10 @@ This is the same as the `--decorate` option of the `git log`.
A list of colors, separated by commas, that can be used to draw
history lines in `git log --graph`.
+`log.graphIndent`::
+ If `true`, indent visual roots when rendering the graphs with `--graph`.
+ Set true by default. It can be overriden with `--[no-]graph-indent`.
+
`log.showRoot`::
If true, the initial commit will be shown as a big creation event.
This is equivalent to a diff against an empty tree.
diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index eaee6ee839..af74f10bb4 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1269,6 +1269,14 @@ This implies the `--topo-order` option by default, but the
By default it is set to 0 (no limit), zero and negative values
are ignored and treated as no limit.
+`--no-graph-indent`::
+`--graph-indent`::
+ When used with `--graph`, indent visual roots (commits with no parents
+ or whose parents are not shown) to differentiate them from commits that
+ are vertically adjacent but unrelated. Enabled by default. Use
+ `--no-graph-indent` to disable or set `graph.indent` to set a deafault
+ preference.
+
ifdef::git-rev-list[]
`--count`::
Print a number stating how many commits would have been
diff --git a/graph.c b/graph.c
index c14be934a0..28bef1b88f 100644
--- a/graph.c
+++ b/graph.c
@@ -419,6 +419,8 @@ void graph_setup_line_prefix(struct diff_options *diffopt)
static void graph_read_config(struct rev_info *revs)
{
+ int val;
+
if (!column_colors) {
char *string;
if (repo_config_get_string(revs->repo, "log.graphcolors", &string)) {
@@ -435,6 +437,9 @@ static void graph_read_config(struct rev_info *revs)
custom_colors.nr - 1);
}
}
+
+ if (!repo_config_get_bool(revs->repo, "log.graphIndent", &val))
+ revs->no_graph_indent = !val;
}
struct git_graph *graph_init(struct rev_info *opt)
@@ -999,7 +1004,8 @@ static void graph_peek_next_visible(struct git_graph *graph,
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;
+ graph->num_columns > 0 && !graph->visual_root_cascade &&
+ !graph->revs->no_graph_indent;
}
void graph_update(struct git_graph *graph, struct commit *commit)
@@ -1344,7 +1350,7 @@ 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) {
+ if (graph->is_visual_root && !graph->revs->no_graph_indent) {
int depth = graph->visual_root_depth;
/*
* Each visual column is 2 characters wide.
diff --git a/revision.c b/revision.c
index 258c3cf782..215cf11071 100644
--- a/revision.c
+++ b/revision.c
@@ -2627,6 +2627,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->graph = NULL;
} else if (skip_prefix(arg, "--graph-lane-limit=", &optarg)) {
revs->graph_max_lanes = parse_count(optarg);
+ } else if (!strcmp(arg, "--graph-indent")) {
+ revs->no_graph_indent = 0;
+ revs->graph_indent_set = 1;
+ } else if (!strcmp(arg, "--no-graph-indent")) {
+ revs->no_graph_indent = 1;
+ revs->graph_indent_set = 1;
} else if (!strcmp(arg, "--encode-email-headers")) {
revs->encode_email_headers = 1;
} else if (!strcmp(arg, "--no-encode-email-headers")) {
@@ -3201,6 +3207,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
if (revs->graph_max_lanes > 0 && !revs->graph)
die(_("the option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
+ if (revs->graph_indent_set > 0 && !revs->graph)
+ die(_("the option '%s' requires '%s'"), "--[no-]graph-indent", "--graph");
+
if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
diff --git a/revision.h b/revision.h
index 569b3fa1cb..acf6d06b24 100644
--- a/revision.h
+++ b/revision.h
@@ -314,6 +314,8 @@ struct rev_info {
/* Display history graph */
struct git_graph *graph;
int graph_max_lanes;
+ unsigned int no_graph_indent:1;
+ unsigned int graph_indent_set:1;
/* special limits */
int skip_count;
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
index d4c850c0d4..b69730e7ba 100755
--- a/t/t4218-log-graph-indentation.sh
+++ b/t/t4218-log-graph-indentation.sh
@@ -540,4 +540,56 @@ test_expect_success 'visual root cascading gets wrapped after 4 columns' '
EOF
'
+test_expect_success '--no-graph-indent disables indentation' '
+ lib_test_check_graph --no-graph-indent _58 _59 _60 _61 _62 _63 _64 _65 _66 _67 <<-\EOF
+ * 67_A
+ * 66_A
+ * 65_A
+ * 64_A
+ * 63_A
+ * 62_A
+ * 61_A
+ * 60_A
+ * 59_A
+ * 58_B
+ * 58_A
+ EOF
+'
+
+test_expect_success 'log.graphIndent config disables indentation' '
+ test_config log.graphIndent false &&
+ lib_test_check_graph _58 _59 _60 _61 _62 _63 _64 _65 _66 _67 <<-\EOF
+ * 67_A
+ * 66_A
+ * 65_A
+ * 64_A
+ * 63_A
+ * 62_A
+ * 61_A
+ * 60_A
+ * 59_A
+ * 58_B
+ * 58_A
+ EOF
+'
+
+test_expect_success '--graph-indent forces indentation when graph.indent is unset' '
+ test_config log.graphIndent false &&
+ lib_test_check_graph --graph-indent _58 _59 _60 _61 _62 _63 _64 _65 _66 _67 <<-\EOF
+ * 67_A
+ * 66_A
+ * 65_A
+ * 64_A
+ * 63_A
+ * 62_A
+ * 61_A
+ * 60_A
+ * 59_A
+ * 58_B
+ * 58_A
+ EOF
+'
+
+# graph.indent true and no --option is the default state.
+
test_done
--
2.54.0
^ permalink raw reply related
* [PATCH v11 6/7] graph: move config reading into graph_read_config()
From: Pablo Sabater @ 2026-07-13 16:44 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@gmail.com>
Move the repo_config_get_string() call out of graph_init() and into
graph_read_config(). This simplifies graph_init() and provides a
function for future graph-related config opt.
This commit is a preparatory commit for a subsequent one.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
graph.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/graph.c b/graph.c
index e3e206170c..c14be934a0 100644
--- a/graph.c
+++ b/graph.c
@@ -417,13 +417,11 @@ void graph_setup_line_prefix(struct diff_options *diffopt)
diffopt->output_prefix = diff_output_prefix_callback;
}
-struct git_graph *graph_init(struct rev_info *opt)
+static void graph_read_config(struct rev_info *revs)
{
- struct git_graph *graph = xmalloc(sizeof(struct git_graph));
-
if (!column_colors) {
char *string;
- if (repo_config_get_string(opt->repo, "log.graphcolors", &string)) {
+ if (repo_config_get_string(revs->repo, "log.graphcolors", &string)) {
/* not configured -- use default */
graph_set_column_colors(column_colors_ansi,
column_colors_ansi_max);
@@ -437,6 +435,13 @@ struct git_graph *graph_init(struct rev_info *opt)
custom_colors.nr - 1);
}
}
+}
+
+struct git_graph *graph_init(struct rev_info *opt)
+{
+ struct git_graph *graph = xmalloc(sizeof(struct git_graph));
+
+ graph_read_config(opt);
graph->commit = NULL;
graph->revs = opt;
--
2.54.0
^ permalink raw reply related
* [PATCH v11 4/7] graph: indent visual root in graph
From: Pablo Sabater @ 2026-07-13 16:44 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@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.
[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
Helped-by: Kristofer Karlsson <krka@spotify.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 | 244 +++++++++++++++++++
t/meson.build | 1 +
t/t4218-log-graph-indentation.sh | 514 +++++++++++++++++++++++++++++++++++++++
3 files changed, 759 insertions(+)
diff --git a/graph.c b/graph.c
index 89ebcf7540..087094189f 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 int is_merge_parent:1;
};
enum graph_state {
GRAPH_PADDING,
GRAPH_SKIP,
GRAPH_PRE_COMMIT,
+ GRAPH_PRE_ROOT,
GRAPH_COMMIT,
GRAPH_POST_MERGE,
GRAPH_COLLAPSING
@@ -323,6 +334,51 @@ struct git_graph {
*/
struct commit *lookahead[2];
int lookahead_nr;
+
+ /*
+ * If a commit is a visual root, we need to indent it to prevent
+ * unrelated commits from being vertically adjacent to it.
+ */
+ unsigned int 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 int visual_root_cascade:1;
+
+ /*
+ * Set when the current commit was already present in graph->columns
+ * before being processed.
+ */
+ unsigned int 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)
@@ -399,6 +455,8 @@ struct git_graph *graph_init(struct rev_info *opt)
graph->lookahead[0] = NULL;
graph->lookahead[1] = NULL;
graph->lookahead_nr = 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.
@@ -581,6 +639,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;
@@ -592,6 +655,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) {
@@ -630,6 +694,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)
@@ -721,10 +791,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
*/
@@ -814,9 +894,113 @@ void graph_push_lookahead(struct git_graph *graph, struct commit *c)
graph->lookahead[graph->lookahead_nr++] = c;
}
+/*
+ * 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.
+ *
+ * - Its parents are uninteresting.
+ *
+ * - 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, struct git_graph *graph)
+{
+ struct commit_list *p;
+
+ if (c->object.flags & BOUNDARY)
+ return 0;
+ for (p = c->parents; p; p = p->next)
+ if (graph_is_interesting(graph, p->item))
+ return 0;
+ return 1;
+}
+
+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) &&
+ !(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);
+}
+
+/*
+ * Peeks the next commits via the lookahead buffer and sets the lookahead flags.
+ */
+static void graph_peek_next_visible(struct git_graph *graph,
+ struct graph_lookahead_flags *flags)
+{
+ flags->is_next_visible = 0;
+ flags->is_next_visual_root = 0;
+ flags->next_has_column = 0;
+
+ if (!graph->lookahead_nr)
+ return;
+
+ flags->is_next_visible = 1;
+ flags->next_has_column =
+ graph_find_new_column_by_commit(graph, graph->lookahead[0]) >= 0;
+
+ if (!graph_is_visual_root_candidate(graph->lookahead[0], graph))
+ return;
+
+ if (graph->lookahead_nr >= 2)
+ flags->is_next_visual_root = 1;
+}
+
+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
@@ -847,6 +1031,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;
/*
@@ -864,11 +1065,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
@@ -1116,6 +1322,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)) {
@@ -1487,6 +1704,30 @@ 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 must only be called on a
+ * visual root.
+ */
+ if (!graph->is_visual_root)
+ BUG("commit must be a visual root to call pre_root_line");
+
+ for (int 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;
@@ -1512,6 +1753,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 7c3c070426..cce5ba71f9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -577,6 +577,7 @@ integration_tests = [
't4215-log-skewed-merges.sh',
't4216-log-bloom.sh',
't4217-log-limit.sh',
+ 't4218-log-graph-indentation.sh',
't4219-log-follow-merge.sh',
't4252-am-options.sh',
't4253-am-keep-cr-dos.sh',
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
new file mode 100755
index 0000000000..60c7d84af7
--- /dev/null
+++ b/t/t4218-log-graph-indentation.sh
@@ -0,0 +1,514 @@
+#!/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" &&
+ test_might_fail git rm -rf .
+}
+
+# disable commit-graph topo order to have the graph to render in different
+# ways (used in --first-parent tests to have multiple visual roots while a
+# column is active at the same time).
+unset_commit_graph () {
+ sane_unset GIT_TEST_COMMIT_GRAPH &&
+ rm -f .git/objects/info/commit-graph &&
+ rm -rf .git/objects/info/commit-graphs
+}
+
+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
+'
+
+test_expect_success 'filtered parent cascading edge case' '
+ create_orphan _27 &&
+ echo test >foo.txt &&
+ git add foo.txt &&
+ test_tick &&
+ git commit -m "D (last)" &&
+
+ create_orphan _25 &&
+ echo test >other.txt &&
+ git add other.txt &&
+ test_tick &&
+ git commit -m "C-filtered" &&
+
+ echo test >foo.txt &&
+ git add foo.txt &&
+ test_tick &&
+ git commit -m "B (child of filtered)" &&
+
+ create_orphan _26 &&
+ echo test >foo.txt &&
+ git add foo.txt &&
+ test_tick &&
+ git commit -m "A (visual root)" &&
+
+ lib_test_check_graph _25 _26 _27 -- foo.txt <<-\EOF
+ * A (visual root)
+ * B (child of filtered)
+ * D (last)
+ EOF
+'
+
+test_expect_success 'multiple filtered parents in sequence' '
+ create_orphan _44 &&
+ 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 &&
+ 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 &&
+ echo e >foo.txt && git add foo.txt && git commit -m "46_A" &&
+
+ lib_test_check_graph _44 _45 _46 -- foo.txt <<-\EOF
+ * 44_C
+ * 45_C
+ * 46_A
+ EOF
+'
+
+# These 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' '
+ (
+ unset_commit_graph &&
+ 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' '
+ (
+ unset_commit_graph &&
+ 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 children' '
+ (
+ unset_commit_graph &&
+ 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_expect_success 'two unrelated merges' '
+ create_orphan _50 && test_commit 50_A &&
+ git checkout -b _51 &&
+ test_commit 51_A && test_commit 51_B &&
+ git checkout _50 &&
+ git merge --no-ff _51 -m 50_B &&
+
+ create_orphan _52 && test_commit 52_A &&
+ git checkout -b _53 &&
+ test_commit 53_A && test_commit 53_B &&
+ git checkout _52 &&
+ git merge --no-ff _53 -m 52_B &&
+
+ lib_test_check_graph _52 _50 <<-\EOF
+ * 52_B
+ |\
+ | * 53_B
+ | * 53_A
+ |/
+ \
+ * 52_A
+ * 50_B
+ |\
+ | * 51_B
+ | * 51_A
+ |/
+ * 50_A
+ EOF
+'
+
+test_expect_success '--max-count treats the last visible commit as the last commit' '
+ lib_test_check_graph --max-count=2 _8 _9 _10 <<-\EOF
+ * 10_A
+ * 9_A
+ EOF
+'
+
+test_expect_success '--max-count=1 shows a single root without indentation' '
+ lib_test_check_graph --max-count=1 _8 _9 _10 <<-\EOF
+ * 10_A
+ EOF
+'
+
+test_expect_success '--max-count-oldest indents visual roots' '
+ lib_test_check_graph --max-count-oldest=3 _8 _9 _10 <<-\EOF
+ * 10_A
+ * 9_A
+ * 8_A
+ EOF
+'
+
+# when the graph commits are filtered with regex options like --author, the
+# commit parents do not come NULL so it is needed to check if the parents are
+# interesting.
+test_expect_success '--author skipped parent makes a visual root' '
+ create_orphan _55 &&
+ test_tick &&
+ git commit --allow-empty -m 55_A &&
+ create_orphan _54 &&
+ test_tick &&
+ git commit --allow-empty --author="Other <other@example.com>" -m 54_A &&
+ test_tick &&
+ git commit --allow-empty -m 54_B &&
+ test_tick &&
+ git commit --allow-empty -m 54_C &&
+ lib_test_check_graph --author="A U Thor" _54 _55 <<-\EOF
+ * 54_C
+ \
+ * 54_B
+ * 55_A
+ EOF
+'
+
+test_expect_success '--grep skipped parent makes a visual root' '
+ create_orphan _57 &&
+ test_tick &&
+ git commit --allow-empty -m 57_keep_A &&
+ create_orphan _56 &&
+ test_tick &&
+ git commit --allow-empty -m 56_skip &&
+ test_tick &&
+ git commit --allow-empty -m 56_keep_A &&
+ test_tick &&
+ git commit --allow-empty -m 56_keep_B &&
+ lib_test_check_graph --grep=keep _56 _57 <<-\EOF
+ * 56_keep_B
+ \
+ * 56_keep_A
+ * 57_keep_A
+ EOF
+'
+
+test_done
--
2.54.0
^ permalink raw reply related
* [PATCH v11 3/7] graph: add a 2 commit buffer for lookahead
From: Pablo Sabater @ 2026-07-13 16:44 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@gmail.com>
In a subsequent commit the graph renderer needs to know if the next
commit is a visual root or if it is the last commit to be shown. This
requires peeking 2 commits ahead.
Commits are pre-fetched in get_revision() through next_commit_to_show()
where they are also marked as SHOWN, regardless the source they come
from.
Update graph_is_interesting() so it considers commits inside the
lookahead buffer as interesting as well.
Helped-by: Kristofer Karlsson <krka@spotify.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
graph.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
graph.h | 17 +++++++++++++++++
revision.c | 18 ++++++++++++++++--
3 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/graph.c b/graph.c
index 842282685f..89ebcf7540 100644
--- a/graph.c
+++ b/graph.c
@@ -315,6 +315,14 @@ struct git_graph {
* diff_output_prefix_callback().
*/
struct strbuf prefix_buf;
+
+ /*
+ * Lookahead buffer: up to 2 pre-fetched commits that will be shown.
+ * Populated by get_revision() so graph_peek_next_visible() can use
+ * actual walk results instead of peeking at rev_info internals.
+ */
+ struct commit *lookahead[2];
+ int lookahead_nr;
};
static inline int graph_needs_truncation(struct git_graph *graph, int lane)
@@ -388,6 +396,9 @@ struct git_graph *graph_init(struct rev_info *opt)
graph->num_columns = 0;
graph->num_new_columns = 0;
graph->mapping_size = 0;
+ graph->lookahead[0] = NULL;
+ graph->lookahead[1] = NULL;
+ graph->lookahead_nr = 0;
/*
* Start the column color at the maximum value, since we'll
* always increment it for the first commit we output.
@@ -456,6 +467,15 @@ static void graph_ensure_capacity(struct git_graph *graph, int num_columns)
*/
static int graph_is_interesting(struct git_graph *graph, struct commit *commit)
{
+ /*
+ * Commits in the lookahead buffer have been pre-fetched by
+ * get_revision() and will be shown in the future. They already have
+ * the SHOWN flag set when they were pre-fetched but the graph still
+ * needs to treat them as interesting parents.
+ */
+ for (int i = 0; i < graph->lookahead_nr; i++)
+ if (graph->lookahead[i] == commit)
+ return 1;
/*
* If revs->boundary is set, commits whose children have
* been shown are always interesting, even if they have the
@@ -763,6 +783,37 @@ static int graph_needs_pre_commit_line(struct git_graph *graph)
graph->expansion_row < graph_num_expansion_rows(graph);
}
+struct commit *graph_pop_lookahead(struct git_graph *graph)
+{
+ struct commit *c;
+
+ if (!graph->lookahead_nr)
+ return NULL;
+
+ c = graph->lookahead[0];
+ if (!c)
+ BUG("lookahead buffer has %d entries but the first one is NULL",
+ graph->lookahead_nr);
+
+ graph->lookahead[0] = graph->lookahead[1];
+ graph->lookahead[1] = NULL;
+ graph->lookahead_nr--;
+ return c;
+}
+
+int graph_get_lookahead_room(struct git_graph *graph)
+{
+ return (int)ARRAY_SIZE(graph->lookahead) - graph->lookahead_nr;
+}
+
+void graph_push_lookahead(struct git_graph *graph, struct commit *c)
+{
+ if (!graph_get_lookahead_room(graph))
+ BUG("pushing into lookahead buffer when it is already full");
+
+ graph->lookahead[graph->lookahead_nr++] = c;
+}
+
void graph_update(struct git_graph *graph, struct commit *commit)
{
struct commit_list *parent;
diff --git a/graph.h b/graph.h
index 3fd1dcb2e9..1193711fb8 100644
--- a/graph.h
+++ b/graph.h
@@ -262,4 +262,21 @@ void graph_show_commit_msg(struct git_graph *graph,
FILE *file,
struct strbuf const *sb);
+/*
+ * Pop the first commit from the graph's lookahead buffer.
+ * Returns NULL if the buffer is empty.
+ */
+struct commit *graph_pop_lookahead(struct git_graph *graph);
+
+/*
+ * Returns how many more commits can be added to the lookahead buffer.
+ */
+int graph_get_lookahead_room(struct git_graph *graph);
+
+/*
+ * Push a commit into the lookahead buffer. Must only be called when
+ * graph_get_lookahead_room() returns > 0.
+ */
+void graph_push_lookahead(struct git_graph *graph, struct commit *c);
+
#endif /* GRAPH_H */
diff --git a/revision.c b/revision.c
index 288935943f..258c3cf782 100644
--- a/revision.c
+++ b/revision.c
@@ -4715,10 +4715,24 @@ struct commit *get_revision(struct rev_info *revs)
return c;
}
- c = next_commit_to_show(revs);
+ if (revs->graph) {
+ c = graph_pop_lookahead(revs->graph);
+ if (!c)
+ c = next_commit_to_show(revs);
+ } else {
+ c = next_commit_to_show(revs);
+ }
- if (c && revs->graph)
+ if (c && revs->graph) {
+ while (graph_get_lookahead_room(revs->graph)) {
+ struct commit *next = next_commit_to_show(revs);
+ if (!next)
+ break;
+ graph_push_lookahead(revs->graph, next);
+ }
graph_update(revs->graph, c);
+ }
+
if (!c) {
free_saved_parents(revs);
commit_list_free(revs->previous_parents);
--
2.54.0
^ permalink raw reply related
* [PATCH v11 2/7] revision: add next_commit_to_show()
From: Pablo Sabater @ 2026-07-13 16:43 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@gmail.com>
get_revision() gets its commits from two sources depending on the mode:
1. Normally it gets the commits from get_revision_internal().
2. --max-count-oldest which was introduced at bb4ce23284 (revision.c:
implement --max-count-oldest, 2026-05-19) gets the commits by popping
from a saved list at revs->commits marking SHOWN and CHILD_SHOWN on
each popped commit.
Extract the choice logic into a helper, next_commit_to_show(), which
returns the next commit regardless of the source it comes from.
This has no change in behavior. The helper is needed in a subsequent
commit that pre-fetches two commits into a buffer for lookahead purposes
and needs to pre-fetch from the same source.
The --reverse branch keeps its own pop loop. Using the helper for
--reverse would additionally set SHOWN and CHILD_SHOWN which is not
desired and a behavior change.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
revision.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/revision.c b/revision.c
index 0c95edef59..288935943f 100644
--- a/revision.c
+++ b/revision.c
@@ -4658,12 +4658,34 @@ static void retrieve_oldest_commits(struct rev_info *revs,
commit_list_insert(c, queue);
}
+/*
+ * Returns the next commit that will be shown, regardless of whether it comes
+ * directly from the revision walk or from the list saved by the staged output
+ * of --max-count-oldest.
+ */
+static struct commit *next_commit_to_show(struct rev_info *revs)
+{
+ struct commit *c;
+ struct commit_list *p;
+
+ if (!revs->max_count_stage)
+ return get_revision_internal(revs);
+
+ c = pop_commit(&revs->commits);
+ if (c) {
+ c->object.flags |= SHOWN;
+ if (!(c->object.flags & BOUNDARY))
+ for (p = c->parents; p; p = p->next)
+ p->item->object.flags |= CHILD_SHOWN;
+ }
+ return c;
+}
+
struct commit *get_revision(struct rev_info *revs)
{
struct commit *c;
struct commit_list *reversed;
struct commit_list *queue = NULL;
- struct commit_list *p;
if (revs->max_count_type == 1 && !revs->max_count_stage) {
retrieve_oldest_commits(revs, &queue);
@@ -4693,17 +4715,7 @@ struct commit *get_revision(struct rev_info *revs)
return c;
}
- if (revs->max_count_stage) {
- c = pop_commit(&revs->commits);
- if (c) {
- c->object.flags |= SHOWN;
- if (!(c->object.flags & BOUNDARY))
- for (p = c->parents; p; p = p->next)
- p->item->object.flags |= CHILD_SHOWN;
- }
- } else {
- c = get_revision_internal(revs);
- }
+ c = next_commit_to_show(revs);
if (c && revs->graph)
graph_update(revs->graph, c);
--
2.54.0
^ permalink raw reply related
* [PATCH v11 1/7] lib-log-graph: move check_graph function
From: Pablo Sabater @ 2026-07-13 16:43 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v11-0-dcb65bc4ba99@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 v11 0/7] graph: indent visual roots in graph
From: Pablo Sabater @ 2026-07-13 16:43 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, krka, mroik,
peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260713-ps-pre-commit-indent-v10-0-82ddab26bc96@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
* D
* E
* F
* G
* H
* I
* J1
* J2
The indentation wraps after cascading columns and when wrapping back to
the initial column if the next commit is a non-visual-root commit, force
the indentation one extra level.
Series explanation:
- Cleanup to bring a common function from t4215 and t6016 that will be
used in t4218.
- Logic extraction of the chose of from where the commit source comes
from.
- Add a buffer for lookahead purposes.
- Principal commit. Implement the logic to get the visual roots
indented.
- Make visual root cascading wrap after 4 columns
- Add --[no-]graph-indent and log.graphIndent options.
GitHub CI: https://github.com/pabloosabaterr/git/actions/runs/29266560903
[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
V9 DIFF:
- Changed boolean variables to be bit fields.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Pablo Sabater (7):
lib-log-graph: move check_graph function
revision: add next_commit_to_show()
graph: add a 2 commit buffer for lookahead
graph: indent visual root in graph
graph: wrap cascading commits after 4 columns
graph: move config reading into graph_read_config()
graph: add --[no-]graph-indent and log.graphIndent
Documentation/config/log.adoc | 4 +
Documentation/rev-list-options.adoc | 8 +
graph.c | 332 +++++++++++++++-
graph.h | 17 +
revision.c | 57 ++-
revision.h | 2 +
t/lib-log-graph.sh | 5 +
t/meson.build | 1 +
t/t4215-log-skewed-merges.sh | 33 +-
t/t4218-log-graph-indentation.sh | 595 +++++++++++++++++++++++++++++
t/t6016-rev-list-graph-simplify-history.sh | 25 +-
11 files changed, 1031 insertions(+), 48 deletions(-)
Range-diff versus v10:
1: 9541b410b7 = 1: dd0bb0d215 lib-log-graph: move check_graph function
2: 4f8fb2cc1d = 2: 07e239533d revision: add next_commit_to_show()
3: b50574bbe1 = 3: 4d71f674a1 graph: add a 2 commit buffer for lookahead
4: fc3a8253fd = 4: 48ad2562f0 graph: indent visual root in graph
5: 204aae5061 = 5: 45be69d11b graph: wrap cascading commits after 4 columns
6: 1b42ed86a1 = 6: 8ce53ae21b graph: move config reading into graph_read_config()
7: 737331b68d ! 7: c1fa81022e graph: add --[no-]graph-indent and log.graphIndent
@@ revision.h: struct rev_info {
/* Display history graph */
struct git_graph *graph;
int graph_max_lanes;
-+ int no_graph_indent;
-+ unsigned int graph_indent_set;
++ unsigned int no_graph_indent:1;
++ unsigned int graph_indent_set:1;
/* special limits */
int skip_count;
---
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
^ permalink raw reply
* Re: [PATCH v18 5/7] branch: add --delete-merged <branch>
From: Phillip Wood @ 2026-07-13 16:39 UTC (permalink / raw)
To: Harald Nordgren, phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <5212d968-6121-466c-8225-36b4bad6b211@gmail.com>
Hi Harald
On 13/07/2026 16:39, Phillip Wood wrote:
> On 11/07/2026 20:36, Harald Nordgren wrote:
>>
>> Digging more into this, probably the most elegant solution is to
>> replace mainline with main, but then also do this:
>>
>> git config branch.main.pushRemote origin
>>
>> This exposes something that I don't love about this feature,
>
> by "this feature" do you mean "git branch --delete-merged"?
>
>> which is
>> that when using a pushDefault (like we do in the tests with 'git
>> config remote.pushDefault fork') if not adding a special case for the
>> main/master branch (like 'git config branch.main.pushRemote origin'),
>> then it will get cleaned up as a forked branch.
>
> Oh, so because the default push remote is not "origin" we need to
> override that for the branches that we do push to "origin". That's a
> pain, but even if we did add a special case for the default branch, it
> would not protect other branches like "next" and "seen".
Thinking about this a bit more, rather than protecting branches where
$branch@{push} == $branch@{upstream}, perhaps we should be protecting
branches that are merged into their upstream but
git push branch.$branch.remote $branch
would update $branch@{upstream}. So we'd apply the push refspec to the
branch name, then apply the fetch refspec to that and check the result
did not match the name of the upstream branch.
Does that make sense?
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v11 06/10] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
From: Junio C Hamano @ 2026-07-13 16:39 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, pabloosabaterr, cirnovskyv, szeder.dev, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-7-cat@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
> Subject: Re: [PATCH v11 06/10] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
Are there patches 7..10/10 posted somewhere else? I didn't see them
in the thread (neither did "b4").
>
> -static void git_apply_config(void)
> +static void git_apply_config(struct repository *repo)
> {
> - repo_config_get_string(the_repository, "apply.whitespace", &apply_default_whitespace);
> - repo_config_get_string(the_repository, "apply.ignorewhitespace", &apply_default_ignorewhitespace);
> - repo_config(the_repository, git_xmerge_config, NULL);
> + struct repo_config_values *cfg = repo_config_values(repo);
> +
> + FREE_AND_NULL(cfg->apply_default_whitespace);
> + repo_config_get_string(repo, "apply.whitespace",
> + &cfg->apply_default_whitespace);
> + FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
> + repo_config_get_string(repo, "apply.ignorewhitespace",
> + &cfg->apply_default_ignorewhitespace);
> + repo_config(repo, git_xmerge_config, NULL);
> }
OK.
> static int parse_whitespace_option(struct apply_state *state, const char *option)
> @@ -126,10 +132,15 @@ int init_apply_state(struct apply_state *state,
> strset_init(&state->kept_symlinks);
> strbuf_init(&state->root, 0);
>
> - git_apply_config();
> - if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
> + git_apply_config(repo);
> +
> + struct repo_config_values *cfg = repo_config_values(repo);
Doesn't "-Wdeclaration-after-statement" complain on this, declaring cfg
after calling "git_apply_config(repo)" on the line before?
^ permalink raw reply
* Re: [PATCH v2 12/12] odb: make optimizations pluggable
From: Junio C Hamano @ 2026-07-13 16:34 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260713-b4-pks-odb-optimize-v2-12-9c2c3ee94b38@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c"
> into the "files" source and wire them up via newly introduced vtable
> pointers for the object database sources. This makes the logic pluggable
> and thus allows other backends to have their own, custom implementation.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/gc.c | 490 +----------------------------------------------------
> odb.c | 12 ++
> odb.h | 45 +++++
> odb/source-files.c | 470 ++++++++++++++++++++++++++++++++++++++++++++++++++
Reviewing this commit with "git show --color-moved" is an excellent
way to verify that this is 90% pure code motion. Only minimal
adjustments were required, such as replacing a direct reference to a
struct member in the original with a variable that was previously
assigned the equivalent value.
For example, this original
> - switch (opts->strategy) {
> - case ODB_OPTIMIZE_INCREMENTAL: {
> - int gc_auto_threshold = 6700;
> - int gc_auto_pack_limit = 50;
> -
> - repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
> - repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
> -
> - /*
> - * Setting gc.auto to 0 or negative can disable the
> - * automatic gc.
> - */
turns into
> + switch (opts->strategy) {
> + case ODB_OPTIMIZE_INCREMENTAL: {
> + int gc_auto_threshold = 6700;
> + int gc_auto_pack_limit = 50;
> +
> + repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
> + repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
> +
> + /*
> + * Setting gc.auto to 0 or negative can disable the
> + * automatic gc.
> + */
And the change from odb->repo vs repo is such an adjustment.
Looking good.
^ permalink raw reply
* Re: [PATCH v18 6/7] branch: add branch.<name>.deleteMerged opt-out
From: Harald Nordgren @ 2026-07-13 16:33 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <f53f7140-b9c0-40dd-8bd3-89917a4eb2a2@gmail.com>
> > +test_expect_success '--delete-merged honours branch.<name>.deleteMerged=false' '
> > + test_when_finished "rm -rf repo" &&
> > + setup_repo_for_delete_merged &&
> > + merged_branch deleted origin/next &&
> > + merged_branch kept origin/next &&
> > + git -C repo config branch.kept.deleteMerged false &&
> > + git -C repo checkout --detach &&
> > +
> > + git -C repo branch --delete-merged origin/next 2>err &&
> > +
> > + test_grep "Skipping .kept." err &&
> > + test_must_fail git -C repo rev-parse --verify refs/heads/deleted &&
> > + git -C repo rev-parse --verify refs/heads/kept
>
> As with the previous patches, I think this would be nicer if we checked
> the output for for-each-ref. Everything else looks fine.
Thanks! Yes, I have already created a helper for it, and it makes it a
lot clearer. Thanks again for a good suggestion.
Harald
^ permalink raw reply
* Re: [PATCH v3] sequencer: honor --empty when a fixup!/squash! empties its target
From: Farid Zakaria @ 2026-07-13 16:30 UTC (permalink / raw)
To: Phillip Wood, Junio C Hamano, Farid Zakaria
Cc: git, Phillip Wood, Elijah Newren, Patrick Steinhardt
In-Reply-To: <7a1e5111-185e-4390-afa1-c19908c9bd86@gmail.com>
On Mon Jul 13, 2026 at 6:18 AM PDT, Phillip Wood wrote:
> On 12/07/2026 06:01, Junio C Hamano wrote:
>> Farid Zakaria <farid.m.zakaria@gmail.com> writes:
>>
>>> When "git rebase --autosquash" melds a "fixup!" or "squash!" commit into
>>> its target, the result can be a commit that no longer changes anything
>>> relative to its parent, for example when the melded change reverts the
>>> target. Rather than dropping or keeping this empty commit, the rebase
>>> stops with
>>>
>>> You asked to amend the most recent commit, but doing so would
>>> make it empty. ...
>>>
>>> and the "--empty" option has no effect on it. This makes backing a
>>> change out of a series awkward: reverting a commit as a "fixup!" and
>>> running "git rebase --autosquash --empty=drop" ought to remove both the
>>> commit and its revert, but it halts instead.
>>> ...
>>> Changes in v3:
>>> * Switch the new tests' assertions from grep to test_grep for better
>>> diagnostics (per review).
>>> * Link to v2: https://lore.kernel.org/r/20260710-fz-autosquash-empty-v2-1-fa1e277e05f8@gmail.com
>>
>> I see you are already working well with Phillip, which is great.
>>
>> This topic, when merged to 'seen', seems to have quite a lot of
>> overlaps with his pw/rebase-drop-notes-with-commit topic.
>
> Oh, I should have thought of that
>
>> We are
>> expecting the topic to be rerolled, and I was under the impression
>> that the remaining issues in that topic were all minor (Phillip,
>> correct me if I am wrong) and hopefully we will see it in 'next'
>> not in so distant future.
>
> I've just sent a new version and cc'd Farid, I'll try and take look at
> this patch tomorrow
>
Thanks for cc'd. I'm not familiar with the workflow (I read the docs)
but is there an email reply when it's accepted into 'next' that I will
just look-out for ? I'm not subscribed to the mailing list in general
otherwise.
>> So it might make sense for you to coordinate with Phillip, and wait
>> for his topic to be merged to 'next'. After that happens, you would
>> prepare a merge commit of the other branch into f85a7e6620 (Start
>> Git 2.56 cycle, 2026-07-06) or some other stable point, and rebuild
>> this patch on top of it. That way, it will be much less likely that
>> I'd make stupid and unnecessary mismerges when attempting to
>> integrate this topic into my tree.
>
> That makes sense, assuming no-one has any more comments on
> 'pw/rebase-drop-notes-with-commit' it should in be 'next' fairly soon.
>
> Thanks
>
> Phillip
Phillip,
Let me know if you have any more comments. I suspect not much will
changes logic-wise once I rebase it onto 'next'.
For clarity, is the f85a7e6620 commit the 'next' branch ? I would have
thought to just rebase ontop of 'next' and I'm a bit confused with this
commit hash.
If there is anything else I should be aware of, I would appreciate a CC
if you can remember :)
Thank you!
^ permalink raw reply
* Re: [PATCH 1/2] t1100: modernize test style
From: Junio C Hamano @ 2026-07-13 16:30 UTC (permalink / raw)
To: Shlok Kulshreshtha; +Cc: git
In-Reply-To: <20260713140142.27898-2-diy2903@gmail.com>
Shlok Kulshreshtha <diy2903@gmail.com> writes:
> The tests in this script use the old style in which the test title and
> body are passed as separate backslash-continued arguments, with bodies
> indented using spaces:
>
> test_expect_success \
> 'title' \
> 'body'
>
> Convert them to the modern style in which the body is a single-quoted
> block on its own lines, indented with a tab:
>
> test_expect_success 'title' '
> body
> '
>
> This is a style-only change; no test logic is modified.
Cleanly done. Running "git show -w" on this patch clearly
demonstrates that no code has changed.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 11/12] builtin/gc: fix signedness issues in ODB-related functionality
From: Junio C Hamano @ 2026-07-13 16:28 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260713-b4-pks-odb-optimize-v2-11-9c2c3ee94b38@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> There are a couple of signedness issues in ODB-related functionality.
> These are not a problem because we disable -Wsign-compare in this file,
> but once we move these functions into "odb/source-files.c" they will
> result in warnings.
>
> Fix those issues:
>
> - In `too_many_loose_objects()` we receive a signed limit, but compare
> it with the unsigned actual number of loose objects. This is fixed
> by bailing out immediately when the limit is smaller than or equal
> to zero, which we also do similarly in other places. The warning is
> then squelched via a cast.
>
> - In `find_base_packs()` we compare the signed size of the pack
> against the unsigned limit. As the pack size is always going to be a
> positive file size it's safe to cast it to an unsigned value.
>
> - In `odb_optimize()` we compare the unsigned `keep_pack.nr` value
> against the signed `gc_auto_pack_limit`. We only reach this code
> when `too_many_packs()` returns true-ish, and that can only happen
> when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning
> by casting the limit to an unsigned value.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/gc.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
Yuck. The -Wsign-compare strikes again X-<.
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 3207182488..8cf3781313 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -430,19 +430,21 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
>
> static int too_many_loose_objects(struct odb_source_files *files, int limit)
> {
> - /*
> - * This is weird, but stems from legacy behaviour: the GC auto
> - * threshold was always essentially interpreted as if it was rounded up
> - * to the next multiple 256 of, so we retain this behaviour for now.
> - */
> - int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
> unsigned long loose_count;
>
> + if (limit <= 0)
> + return 0;
> +
> if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
> &loose_count) < 0)
> return 0;
>
> - return loose_count > auto_threshold;
> + /*
> + * This is weird, but stems from legacy behaviour: the GC auto
> + * threshold was always essentially interpreted as if it was rounded up
> + * to the next multiple 256 of, so we retain this behaviour for now.
> + */
> + return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
> }
OK. It is trivially correct (if rounding up is correct, that is).
> @@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
> if (e->pack->is_cruft)
> continue;
> if (limit) {
> - if (e->pack->pack_size >= limit)
> + if ((uintmax_t) e->pack->pack_size >= limit)
Here, just like in too_many_loose_objects(), 'limit' is of type
'unsigned long'. While it makes sense to convert both sides of
the comparison to an unsigned type, casting only the left side
to a type that differs from the right side puzzles me.
Presumably, the other side is of type 'off_t', which is signed,
explaining the desire to cast it to an unsigned type. But I am
not sure what happens if 'off_t' is wider than 'unsigned long'.
Otherwise looking good.
^ permalink raw reply
* Re: [PATCH v18 7/7] branch: add --dry-run for --delete-merged
From: Harald Nordgren @ 2026-07-13 16:23 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <278df7ad-caf1-40fa-b5f9-34d78f435fd0@gmail.com>
> This matches what we do without '--dry-run' but what use is '--dry-run
> --quiet' if it does not print anything?
Yeah, it's a bit weird. What do you suggest we do instead, reject the
combination or turn it into a no-op?
Harald
^ permalink raw reply
* Re: [PATCH v10 7/7] graph: add --[no-]graph-indent and log.graphIndent
From: Pablo Sabater @ 2026-07-13 16:16 UTC (permalink / raw)
To: Mirko Faina, Pablo Sabater
Cc: git, ayu.chandekar, chandrapratap3519, christian.couder, gitster,
jltobler, karthik.188, krka, peff, phillip.wood,
siddharthasthana31
In-Reply-To: <alTuevrGiK3bwh31@exploit>
On Mon Jul 13, 2026 at 4:06 PM CEST, Mirko Faina wrote:
> On Mon, Jul 13, 2026 at 12:44:42PM +0200, Pablo Sabater wrote:
>> Some users may prefer to not have graph indentation.
>>
>> Add "log.graphIndent" config variable to graph_read_config() to read the
>> default preference. By default is graph indentation is true.
>>
>> Add --graph-indent and --no-graph-indent options to overwrite the
>> default preference.
>>
>> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
>> ---
>> Documentation/config/log.adoc | 4 +++
>> Documentation/rev-list-options.adoc | 8 ++++++
>> graph.c | 10 +++++--
>> revision.c | 9 +++++++
>> revision.h | 2 ++
>> t/t4218-log-graph-indentation.sh | 52 +++++++++++++++++++++++++++++++++++++
>> 6 files changed, 83 insertions(+), 2 deletions(-)
>
> [snip]
>
>> diff --git a/revision.h b/revision.h
>> index 569b3fa1cb..49e1380b80 100644
>> --- a/revision.h
>> +++ b/revision.h
>> @@ -314,6 +314,8 @@ struct rev_info {
>> /* Display history graph */
>> struct git_graph *graph;
>> int graph_max_lanes;
>> + int no_graph_indent;
>> + unsigned int graph_indent_set;
>
> These are both boolean values and could be set to be 1 bit wide.
True, I'll change it.
>
> Other than that LGTM.
>
> Thank you for the changes.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH 2/2] t1100: move creation of expected output into setup test
From: Junio C Hamano @ 2026-07-13 16:10 UTC (permalink / raw)
To: Shlok Kulshreshtha; +Cc: git
In-Reply-To: <20260713140142.27898-3-diy2903@gmail.com>
Shlok Kulshreshtha <diy2903@gmail.com> writes:
> The "expected" file was created at the top level of the script, outside
Use the present tense to describe what the current code does. For
example:
The 'expected' file is created at the top-level of the script,
outside ...
> of any test. Code that runs outside of a test is not protected by the
> test harness: a failure there is not reported as a test failure and is
> easy to miss.
>
> Move the here-doc that creates "expected" into the existing setup test
> ("test preparation: write empty tree"), using a "<<-" here-doc so its
> body can be indented along with the rest of the test.
>
> Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>
> ---
> t/t1100-commit-tree-options.sh | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
Trivially correct.
Thanks.
>
> diff --git a/t/t1100-commit-tree-options.sh b/t/t1100-commit-tree-options.sh
> index fabe5a97cb..b434d1848e 100755
> --- a/t/t1100-commit-tree-options.sh
> +++ b/t/t1100-commit-tree-options.sh
> @@ -14,15 +14,14 @@ Also make sure that command line parser understands the normal
>
> . ./test-lib.sh
>
> -cat >expected <<EOF
> -tree $EMPTY_TREE
> -author Author Name <author@email> 1117148400 +0000
> -committer Committer Name <committer@email> 1117150200 +0000
> -
> -comment text
> -EOF
> -
> test_expect_success 'test preparation: write empty tree' '
> + cat >expected <<-EOF &&
> + tree $EMPTY_TREE
> + author Author Name <author@email> 1117148400 +0000
> + committer Committer Name <committer@email> 1117150200 +0000
> +
> + comment text
> + EOF
> git write-tree >treeid
> '
^ permalink raw reply
* Re: [PATCH] fast-export: standardize usage string and SYNOPSIS
From: Junio C Hamano @ 2026-07-13 15:54 UTC (permalink / raw)
To: Christian Couder
Cc: git, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Justin Tobler,
Christian Couder
In-Reply-To: <20260713124153.245268-1-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> The output of `git fast-export -h` currently starts with:
> ...
> This also enables us to remove fast-export from
> "t/t0450/adoc-help-mismatches".
Improving consistency and eliminating special cases are both
positive steps forward. Nice work.
> diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
> index 297b57bb2e..719aeca244 100644
> --- a/Documentation/git-fast-export.adoc
> +++ b/Documentation/git-fast-export.adoc
> @@ -9,7 +9,7 @@ git-fast-export - Git data exporter
> SYNOPSIS
> --------
> [verse]
> -'git fast-export' [<options>] | 'git fast-import'
While this was well-intentioned (demonstrating a typical usage
pattern to show readers that the command's output is meant to be
consumed by 'fast-import'), it is highly unusual for a synopsis.
Examples of this nature are better suited for the body of the manual
pages.
> +'git fast-export' [<options>] [<revision-range>] [[--] <path>...]
This updated version is much more consistent with other manual
pages.
Nice. Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH v18 5/7] branch: add --delete-merged <branch>
From: Phillip Wood @ 2026-07-13 15:39 UTC (permalink / raw)
To: Harald Nordgren, phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <CAHwyqnWspUTSnqmkMyXtWuAnENDSzrRLhhUR=Ljtt1xer3tphA@mail.gmail.com>
On 11/07/2026 20:36, Harald Nordgren wrote:
>>>> + (
>>>> + cd repo &&
>>>> + git checkout -b mainline main &&
>>>> + git checkout -b on-local mainline &&
>>>> + git branch --set-upstream-to=mainline on-local &&
>>>
>>> Why do we need on-local to track mainline rather than main? I'm a bit
>>> confused what the point of mainline is.
>>
>> It's to have an indirection of a branch that is the same as main but
>> will be protected. I tried to delete it now and replace it with just
>> main, but then main was deleted and subsequent tests failed.
>
> Digging more into this, probably the most elegant solution is to
> replace mainline with main, but then also do this:
>
> git config branch.main.pushRemote origin
>
> This exposes something that I don't love about this feature,
by "this feature" do you mean "git branch --delete-merged"?
> which is
> that when using a pushDefault (like we do in the tests with 'git
> config remote.pushDefault fork') if not adding a special case for the
> main/master branch (like 'git config branch.main.pushRemote origin'),
> then it will get cleaned up as a forked branch.
Oh, so because the default push remote is not "origin" we need to
override that for the branches that we do push to "origin". That's a
pain, but even if we did add a special case for the default branch, it
would not protect other branches like "next" and "seen".
We should maybe add a note to the documentation about that to warn users.
> But there was a lot of discussion about this already, so I won't get
> into this again.
Fair enough
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v18 2/7] branch: convert delete_branches() to a flags argument
From: Phillip Wood @ 2026-07-13 15:28 UTC (permalink / raw)
To: Harald Nordgren, phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <CAHwyqnV6kS8ZmfXm+9JttoJ=kWP4kYdLGX2giASVMSRXVKL-Pw@mail.gmail.com>
Hi Harald
On 10/07/2026 18:25, Harald Nordgren wrote:
>> This means we have two sources of truth because we modify "flags" later.
>> The idea of replacing the old function parameters with local variables
>> only works if we're not passing the flags variable on to another
>> function so I think we should replace all instances of "force" and
>> "quiet" with flags & DELETE_BRANCH_FORCE/QUIET. That way we have a
>> single source of truth and should avoid any future regressions like the
>> one we saw in an earlier iteration.
>
> Yes, it makes sense.
>
> I just hope we remember this discussion, so another reviewer doesn't
> push in the other direction later because it seems like low-hanging
> fruit.
If you explain why we're using flags as the source of truth, rather than
replacing the function parameters with local variables in this function
that should not be a problem.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v18 6/7] branch: add branch.<name>.deleteMerged opt-out
From: Phillip Wood @ 2026-07-13 15:27 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <d52d717b70c80b668e6d3a1fdf186ab4846664c7.1782338106.git.gitgitgadget@gmail.com>
Hi Harald
On 24/06/2026 22:55, Harald Nordgren via GitGitGadget wrote:
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 047ba54778..b7595610d9 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -2024,4 +2024,30 @@ test_expect_success '--delete-merged clears the upstream of a kept base whose ow
> test_cmp expect actual
> '
>
> +test_expect_success '--delete-merged honours branch.<name>.deleteMerged=false' '
> + test_when_finished "rm -rf repo" &&
> + setup_repo_for_delete_merged &&
> + merged_branch deleted origin/next &&
> + merged_branch kept origin/next &&
> + git -C repo config branch.kept.deleteMerged false &&
> + git -C repo checkout --detach &&
> +
> + git -C repo branch --delete-merged origin/next 2>err &&
> +
> + test_grep "Skipping .kept." err &&
> + test_must_fail git -C repo rev-parse --verify refs/heads/deleted &&
> + git -C repo rev-parse --verify refs/heads/kept
As with the previous patches, I think this would be nicer if we checked
the output for for-each-ref. Everything else looks fine.
Thanks
Phillip
> +'
> +
> +test_expect_success "branch -d still deletes a deleteMerged=false branch" '
> + test_when_finished "rm -rf repo" &&
> + setup_repo_for_delete_merged &&
> + merged_branch kept origin/next &&
> + git -C repo config branch.kept.deleteMerged false &&
> + git -C repo checkout --detach &&
> +
> + git -C repo branch -d kept &&
> + test_must_fail git -C repo rev-parse --verify refs/heads/kept
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH v18 7/7] branch: add --dry-run for --delete-merged
From: Phillip Wood @ 2026-07-13 15:27 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <8d0323f4b30cdfed134ff2840cc8a9ab32f9db53.1782338106.git.gitgitgadget@gmail.com>
Hi Harald
On 24/06/2026 22:55, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> With --dry-run, --delete-merged prints the local branches it would
> delete, one "Would delete branch <name>" line each, and exits
> without touching any ref. The same filtering applies, so the output
> is exactly the set that the real run would delete.
The same filtering as what? I think something like
"git branch --dry-run --delete-merged ..." prints one line per ref that
would be deleted without modifing any refs.
would be sufficient
> --dry-run is only meaningful together with --delete-merged and is
> rejected otherwise.
Good
> @@ -346,13 +348,20 @@ static int delete_branches(int argc, const char **argv, int kinds,
> free(target);
> }
>
> - if (refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
> + if (!dry_run &&
> + refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
> ret = 1;
>
> for_each_string_list_item(item, &refs_to_delete) {
> char *describe_ref = item->util;
> char *name = item->string;
> - if (!refs_ref_exists(get_main_ref_store(the_repository), name)) {
> + if (dry_run) {
> + if (!quiet)
This matches what we do without '--dry-run' but what use is '--dry-run
--quiet' if it does not print anything?
> + printf(remote_branch
> + ? _("Would delete remote-tracking branch %s (was %s).\n")
> + : _("Would delete branch %s (was %s).\n"),
> + name + branch_name_pos, describe_ref);
> + } else if (!refs_ref_exists(get_main_ref_store(the_repository), name)) {
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index b7595610d9..cddcde341d 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1892,8 +1892,12 @@ test_expect_success '--delete-merged deletes merged branches and spares the rest
> ) &&
> sha=$(git -C repo rev-parse --short merged) &&
>
> - git -C repo branch --delete-merged origin/next >actual 2>&1 &&
> + git -C repo branch --dry-run --delete-merged origin/next >actual 2>&1 &&
> + echo "Would delete branch merged (was $sha)." >expect &&
> + test_cmp expect actual &&
> + git -C repo rev-parse --verify refs/heads/merged &&
>
> + git -C repo branch --delete-merged origin/next >actual 2>&1 &&
I was wondering why the diff shows the line above being deleted and then
added, it is because previously there was a blank line after it. The
test for --dry-run looks good.
> echo "Deleted branch merged (was $sha)." >expect &&
> test_cmp expect actual &&
> git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
> @@ -2050,4 +2054,9 @@ test_expect_success "branch -d still deletes a deleteMerged=false branch" '
> test_must_fail git -C repo rev-parse --verify refs/heads/kept
> '
>
> +test_expect_success '--dry-run without --delete-merged is rejected' '
> + test_must_fail git -C forked branch --dry-run 2>err &&
> + test_grep "requires --delete-merged" err
Nice
Thanks
Phillip
^ permalink raw reply
* [PATCH v3 9/9] builtin/cat-file: filter objects via object database
From: Patrick Steinhardt @ 2026-07-13 14:41 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <20260713-pks-odb-for-each-object-filter-v3-0-b3c65c641073@pks.im>
When batching all objects, git-cat-file(1) reaches into the internals of
the object database and manually manages bitmaps to apply object
filters. This creates coupling between the command and the internals of
the respective backend.
Refactor git-cat-file(1) to use the new object filter option when
batching all objects. This significantly simplifies the logic and
ensures that we don't have to reach into internals of the "files" source
anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/cat-file.c | 76 +++++-------------------------------------------------
1 file changed, 7 insertions(+), 69 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..1458dd76d6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,7 +20,6 @@
#include "userdiff.h"
#include "oid-array.h"
#include "packfile.h"
-#include "pack-bitmap.h"
#include "object-file.h"
#include "object-name.h"
#include "odb.h"
@@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
return payload->callback(oid, NULL, 0, payload->payload);
}
-static int batch_one_object_packed(const struct object_id *oid,
- struct packed_git *pack,
- uint32_t pos,
- void *_payload)
-{
- struct for_each_object_payload *payload = _payload;
- return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
- payload->payload);
-}
-
-static int batch_one_object_bitmapped(const struct object_id *oid,
- enum object_type type UNUSED,
- int flags UNUSED,
- uint32_t hash UNUSED,
- struct packed_git *pack,
- off_t offset,
- void *_payload)
-{
- struct for_each_object_payload *payload = _payload;
- return payload->callback(oid, pack, offset, payload->payload);
-}
-
static void batch_each_object(struct batch_options *opt,
for_each_object_fn callback,
unsigned flags,
@@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
.callback = callback,
.payload = _payload,
};
+ struct odb_source_info source_info;
+ struct object_info oi = {
+ .source_infop = &source_info,
+ };
struct odb_for_each_object_options opts = {
.flags = flags,
+ .filter = &opt->objects_filter,
};
- struct bitmap_index *bitmap = NULL;
- struct odb_source *source;
-
- /*
- * TODO: we still need to tap into implementation details of the object
- * database sources. Ideally, we should extend `odb_for_each_object()`
- * to handle object filters itself so that we can move the filtering
- * logic into the individual sources.
- */
- odb_prepare_alternates(the_repository->objects);
- for (source = the_repository->objects->sources; source; source = source->next) {
- struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
- &payload, &opts);
- if (ret)
- break;
- }
-
- if (opt->objects_filter.choice != LOFC_DISABLED &&
- (bitmap = prepare_bitmap_git(the_repository)) &&
- !for_each_bitmapped_object(bitmap, &opt->objects_filter,
- batch_one_object_bitmapped, &payload)) {
- struct packed_git *pack;
-
- repo_for_each_pack(the_repository, pack) {
- if (bitmap_index_contains_pack(bitmap, pack) ||
- open_pack_index(pack))
- continue;
- for_each_object_in_pack(pack, batch_one_object_packed,
- &payload, flags);
- }
- } else {
- struct odb_source_info source_info;
- struct object_info oi = {
- .source_infop = &source_info,
- };
-
- for (source = the_repository->objects->sources; source; source = source->next) {
- struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = odb_source_for_each_object(&files->packed->base, &oi,
- batch_one_object_oi, &payload, &opts);
- if (ret)
- break;
- }
- }
- free_bitmap_index(bitmap);
+ odb_for_each_object_ext(the_repository->objects, &oi,
+ batch_one_object_oi, &payload, &opts);
}
static int batch_objects(struct batch_options *opt)
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v3 8/9] odb: introduce object filters to `odb_for_each_object()`
From: Patrick Steinhardt @ 2026-07-13 14:41 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <20260713-pks-odb-for-each-object-filter-v3-0-b3c65c641073@pks.im>
The function `for_each_bitmapped_object()` can be used to iterate
through all objects covered by a bitmap. The benefit of this function is
that it allows the caller to efficiently handle some object filters. For
example, this can be used to filter out objects of a specific type with
some simple bitmap operations. But callers are currently required to
manually wire up the use of bitmaps though, and to do so they have to
reach into internals of a given object database source.
Introduce a new `struct odb_for_each_object_options::filter` field so
that the interface becomes generic. When set, then a backend may
optionally use the filter to skip some objects that it would have
otherwise yielded.
Note that the respective backends are free to ignore this field if they
cannot meaningfully optimize for a given filter, and consequently
callers need to verify whether they actually want the returned objects.
While annoying, we cannot easily lift this restriction anyway as the
object filter infrastructure supports some filters that cannot be
answered by the object database alone.
An alternative might be to limit the filters to only those that _can_ be
answered by backends. But ultimately, the filters that can be answered
efficiently by the "packed" backend are completely disjunct from those
that can be answered by the "loose" backend, and consequently the set of
filters supported by all backends would be empty. Furthermore, it would
require us to make assumptions about capabilities of future backends,
which may be able to efficiently handle more filters than current ones.
So in the end, this alternative would only limit us artificially.
Implement the logic for the "packed" source. Note that we use the new
function `prepare_source_bitmap_git()` to open the bitmap: as the
backend operates on a single object source, we must only use bitmaps
that belong to that specific source. Otherwise we might yield objects
that are not part of the source at all, and with multiple sources we
would enumerate the same bitmap once per source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.h | 12 +++++++++++
odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
pack-bitmap.c | 3 +--
pack-bitmap.h | 3 +++
4 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/odb.h b/odb.h
index a1e222f605..67d0b34942 100644
--- a/odb.h
+++ b/odb.h
@@ -8,6 +8,7 @@
#include "thread-utils.h"
struct cached_object_entry;
+struct list_objects_filter_options;
struct odb_source_inmemory;
struct packed_git;
struct repository;
@@ -490,6 +491,17 @@ struct odb_for_each_object_options {
*/
const struct object_id *prefix;
size_t prefix_hex_len;
+
+ /*
+ * Optional object filter that allows backends to skip yielding
+ * objects that are excluded by the filter as an optimization. The
+ * filter is a best-effort hint: backends may use it to skip
+ * excluded objects (e.g. by consulting a reachability bitmap), but
+ * are also free to ignore it entirely and yield every object. As a
+ * consequence, callers must re-apply the filter on yielded objects
+ * if they require strict filtering semantics.
+ */
+ const struct list_objects_filter_options *filter;
};
/*
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 9cfa02b7a2..4777395053 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -3,11 +3,13 @@
#include "chdir-notify.h"
#include "dir.h"
#include "git-zlib.h"
+#include "list-objects-filter-options.h"
#include "mergesort.h"
#include "midx.h"
#include "odb/source-packed.h"
#include "odb/streaming.h"
#include "packfile.h"
+#include "pack-bitmap.h"
static int find_pack_entry(struct odb_source_packed *store,
const struct object_id *oid,
@@ -315,6 +317,37 @@ static int odb_source_packed_for_each_prefixed_object(
return ret;
}
+struct bitmapped_for_each_object_data {
+ struct odb_source_packed *packed;
+ const struct object_info *request;
+ const struct odb_for_each_object_options *opts;
+ odb_for_each_object_cb cb;
+ void *cb_data;
+};
+
+static int bitmapped_for_each_object(const struct object_id *oid,
+ enum object_type type UNUSED,
+ int flags UNUSED,
+ uint32_t hash UNUSED,
+ struct packed_git *pack,
+ off_t offset,
+ void *cb_data)
+{
+ struct bitmapped_for_each_object_data *data = cb_data;
+
+ if (should_exclude_pack(pack, data->opts->flags))
+ return 0;
+
+ if (data->request) {
+ struct object_info oi = *data->request;
+ if (packed_object_info(data->packed, pack, offset, &oi) < 0)
+ return -1;
+ return data->cb(oid, &oi, data->cb_data);
+ }
+
+ return data->cb(oid, NULL, data->cb_data);
+}
+
static int odb_source_packed_for_each_object(struct odb_source *source,
const struct object_info *request,
odb_for_each_object_cb cb,
@@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
.cb = cb,
.cb_data = cb_data,
};
+ struct bitmap_index *bitmap = NULL;
struct packfile_list_entry *e;
int pack_errors = 0, ret;
if (opts->prefix)
return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
+ if (opts->filter &&
+ opts->filter->choice != LOFC_DISABLED &&
+ can_filter_bitmap(opts->filter))
+ bitmap = prepare_bitmap_git_for_source(packed);
+ if (bitmap) {
+ struct bitmapped_for_each_object_data bitmap_data = {
+ .packed = packed,
+ .request = request,
+ .opts = opts,
+ .cb = cb,
+ .cb_data = cb_data,
+ };
+
+ ret = for_each_bitmapped_object(bitmap, opts->filter,
+ bitmapped_for_each_object,
+ &bitmap_data);
+ if (ret)
+ goto out;
+ }
+
packed->skip_mru_updates = true;
for (e = packfile_store_get_packs(packed); e; e = e->next) {
@@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
if (should_exclude_pack(p, opts->flags))
continue;
+ /*
+ * Objects covered by the bitmap have already been yielded
+ * above; skip them here to avoid duplicates.
+ */
+ if (bitmap && bitmap_index_contains_pack(bitmap, p))
+ continue;
+
if (open_pack_index(p)) {
pack_errors = 1;
continue;
@@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
out:
packed->skip_mru_updates = false;
+ free_bitmap_index(bitmap);
if (!ret && pack_errors)
ret = -1;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 09ba15d26b..f55a0859ea 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return -1;
}
-static int can_filter_bitmap(const struct list_objects_filter_options *filter)
+bool can_filter_bitmap(const struct list_objects_filter_options *filter)
{
return !filter_bitmap(NULL, NULL, NULL, filter);
}
-
static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
struct bitmap *result)
{
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 9f20fb6e56..1385027c1f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);
struct list_objects_filter_options;
+/* Check whether the filter can be computed via the bitmap. */
+bool can_filter_bitmap(const struct list_objects_filter_options *filter);
+
/*
* Filter bitmapped objects and iterate through all resulting objects,
* executing `show_reach` for each of them. Returns `-1` in case the filter is
--
2.55.0.313.g8d093f411d.dirty
^ 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