* [PATCH v2 07/10] sequencer: remove unnecessary condition in pick_one_commit()
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
item->commit holds the commit to be picked and so it must be non-NULL
otherwise pick_one_commit() would not know which commit to pick.
It is also unconditionally dereferenced in do_pick_commit() which is
called at the top of this function. Therefore the check to see if it
is non-NULL is superfluous.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index a70889a107e..5f5ff3783e6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4988,7 +4988,7 @@ static int pick_one_commit(struct repository *r,
if (res && is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
- } else if (res && is_rebase_i(opts) && item->commit) {
+ } else if (res && is_rebase_i(opts)) {
int to_amend = 0;
struct object_id oid;
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 06/10] sequencer: simplify handing of fixup with conflicts
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Commit e032abd5a0 (rebase: fix rewritten list for failed pick,
2023-09-06) introduced an early return when res == -1, so if we enter
this conditional block then res is positive. After the last couple
of commits the only possible positive value is 1 so we can simplify
the code by removing the conditional call to intend_to_amend() and
call it error_with_patch() instead.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index d0d2cc228c8..a70889a107e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3874,7 +3874,7 @@ static int error_failed_squash(struct repository *r,
return error(_("could not copy '%s' to '%s'"),
rebase_path_message(),
git_path_merge_msg(r));
- return error_with_patch(r, commit, subject, subject_len, opts, 1, 0);
+ return error_with_patch(r, commit, subject, subject_len, opts, 1, 1);
}
static int do_exec(struct repository *r, const char *command_line, int quiet)
@@ -4986,8 +4986,6 @@ static int pick_one_commit(struct repository *r,
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
if (res && is_fixup(item->command)) {
- if (res == 1)
- intend_to_amend();
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
} else if (res && is_rebase_i(opts) && item->commit) {
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 05/10] sequencer: remove unnecessary "or" in pick_one_commit()
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If error_with_patch(..., res, ...) succeeds then it returns "res", if
it fails then it returns -1. This means that or-ing the return value
with "res" is pointless as the result is the same as the return value.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index c97b996bebc..d0d2cc228c8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5007,9 +5007,8 @@ static int pick_one_commit(struct repository *r,
oideq(&opts->squash_onto, &oid))))
to_amend = 1;
- return res | error_with_patch(r, item->commit,
- arg, item->arg_len, opts,
- res, to_amend);
+ return error_with_patch(r, item->commit, arg, item->arg_len,
+ opts, res, to_amend);
}
return res;
}
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 04/10] sequencer: never reschedule on failed commit
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If "git commit" fails to run then run_git_commit() returns -1 which
causes the current command to be rescheduled. This is incorrect as
we have successfully picked the commit and have written all the state
files we need to successfully commit when the user continues. Fix this
by converting -1 to 1 which matches what do_merge() does.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 21dd5ec9799..c97b996bebc 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2542,6 +2542,12 @@ static int do_pick_commit(struct repository *r,
res = run_git_commit(NULL, reflog_action, opts, flags);
*check_todo = 1;
}
+ /*
+ * If "git commit" failed to run then res == -1, but we don't
+ * want reschedule the last command because the picking the
+ * commit was successful.
+ */
+ res = !!res;
}
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 03/10] sequencer: be more careful with external merge
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If an external merge strategy cannot merge (for example because it
would overwrite an untracked file) it exits with a non-zero exit
code other than 1. This should be treated differently to a merge
with conflicts which is signalled by an exit code of 1 because as
the merge failed we need to reschedule the last pick. The caller
expects us to return -1 in this case. Also reschedule without trying
to merge if the commit message cannot be written as that prevents us
from successfully picking the commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 19 +++++++++++++++----
t/t3404-rebase-interactive.sh | 11 +++++++++++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 32a09b6e87d..21dd5ec9799 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2453,14 +2453,25 @@ static int do_pick_commit(struct repository *r,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
- res = write_message(ctx->message.buf, ctx->message.len,
- git_path_merge_msg(r), 0);
+ if (write_message(ctx->message.buf, ctx->message.len,
+ git_path_merge_msg(r), 0)) {
+ res = -1;
+ goto leave;
+ }
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(r, opts->strategy,
- opts->xopts.nr, opts->xopts.v,
+ res = try_merge_command(r, opts->strategy,
+ opts->xopts.nr, opts->xopts.v,
common, oid_to_hex(&head), remotes);
+ /*
+ * If there were conflicts, try_merge_command() returns 1,
+ * any other no-zero return code means that either the merge
+ * command could not be run, or it failed to merge.
+ */
+ if (res && res != 1)
+ res = -1;
+
commit_list_free(common);
commit_list_free(remotes);
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c271..297b84e60d5 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1249,6 +1249,17 @@ test_expect_success 'interrupted rebase -i with --strategy and -X' '
git rebase --continue &&
test $(git show conflict-branch:conflict) = $(cat conflict) &&
test $(cat file1) = Z
+'
+
+test_expect_success 'failing pick with --strategy is rescheduled' '
+ test_when_finished "rm -rf bin; test_might_fail git rebase --abort" &&
+ mkdir bin &&
+ echo exit 2 | write_script bin/git-merge-fail &&
+ git log -1 --format="pick %H # %s" HEAD >expect &&
+ test_must_fail env PATH="$PWD/bin:$PATH" \
+ git rebase --no-ff --strategy fail HEAD^ &&
+ test_cmp expect .git/rebase-merge/git-rebase-todo &&
+ test_cmp expect .git/rebase-merge/done
'
test_expect_success 'rebase -i error on commits with \ in message' '
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 02/10] sequencer: move definition of is_final_fixup()
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Move this function earlier in the file in preparation for adding a
new caller in a later commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 57855b0066a..32a09b6e87d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4627,21 +4627,6 @@ static int do_update_refs(struct repository *r, int quiet)
strbuf_release(&update_msg);
strbuf_release(&error_msg);
return res;
-}
-
-static int is_final_fixup(struct todo_list *todo_list)
-{
- int i = todo_list->current;
-
- if (!is_fixup(todo_list->items[i].command))
- return 0;
-
- while (++i < todo_list->nr)
- if (is_fixup(todo_list->items[i].command))
- return 0;
- else if (!is_noop(todo_list->items[i].command))
- break;
- return 1;
}
static enum todo_command peek_command(struct todo_list *todo_list, int offset)
@@ -4925,6 +4910,21 @@ static int reread_todo_if_changed(struct repository *r,
strbuf_release(&buf);
return 0;
+}
+
+static int is_final_fixup(struct todo_list *todo_list)
+{
+ int i = todo_list->current;
+
+ if (!is_fixup(todo_list->items[i].command))
+ return 0;
+
+ while (++i < todo_list->nr)
+ if (is_fixup(todo_list->items[i].command))
+ return 0;
+ else if (!is_noop(todo_list->items[i].command))
+ break;
+ return 1;
}
static const char rescheduled_advice[] =
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 01/10] t3400: restore coverage for note copying with apply backend
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1783948637.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Now that the merge backend is the default we have lost coverage for
"git rebase --apply" copying notes. Fix this by replacing "-m" with
"--apply" as the previous test which uses the default backend now
checks the merge backend.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t/t3400-rebase.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index c0c00fbb7b1..f0e7fcf649a 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -270,9 +270,9 @@ test_expect_success 'rebase can copy notes' '
test "a note" = "$(git notes show HEAD)"
'
-test_expect_success 'rebase -m can copy notes' '
+test_expect_success 'rebase --apply can copy notes' '
git reset --hard n3 &&
- git rebase -m --onto n1 n2 &&
+ git rebase --apply --onto n1 n2 &&
test "a note" = "$(git notes show HEAD)"
'
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v2 00/10] sequencer: do not record dropped commits as rewritten
From: Phillip Wood @ 2026-07-13 13:17 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
Thanks to everyone who commented on v1. I've squashed the fixups that
Junio had in "seen", squashed patches 8 & 9 together as suggested by
Oswald and expanded the commit message, and added Uwe's Tested-by:
trailer to the final patch. Oswald suggested extended the use of the
enum which I think is a good idea in the long-term but I punted on
that for now because I think it would be fairly invasive and this
series has enough refactoring in it already.
If a commit gets dropped because its changes are already upstream
then we should not record it as rewritten. As well as confusing any
post-rewrite hooks this means we end up copying the notes from the
dropped commit to the commit that was picked immediately before the
one that was dropped.
This series is structured as follows:
Patch 1 restores some test coverage that was lost when the default
rebase backend was changed.
Patch 2 moves a function so it can be called without a forward
declaration in Patch 11.
Patches 3 & 4 fix the return value of do_pick_commit() when an external
command fails (this is in preparation for patch 9).
Patches 5-8 try and simplify the control flow in pick_one_commit()
in preparation for patch 9.
Patch 9 changes the return type of do_pick_commit() to an enum.
Patch 10 adds a new member to the enum from patch 9 for commits that
are dropped when they become empty and uses that to stop them from
being recorded as rewritten.
base-commit: 6c3d7b73556db708feb3b16232fab1efc4353428
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Frebase-drop-notes-with-commit%2Fv2
View-Changes-At: https://github.com/phillipwood/git/compare/6c3d7b735...c89234dd9
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/rebase-drop-notes-with-commit/v2
Phillip Wood (10):
t3400: restore coverage for note copying with apply backend
sequencer: move definition of is_final_fixup()
sequencer: be more careful with external merge
sequencer: never reschedule on failed commit
sequencer: remove unnecessary "or" in pick_one_commit()
sequencer: simplify handing of fixup with conflicts
sequencer: remove unnecessary condition in pick_one_commit()
sequencer: simplify pick_one_commit()
sequencer: use an enum to represent result of picking a commit
sequencer: do not record dropped commits as rewritten
sequencer.c | 154 +++++++++++++++++++++++-----------
t/t3400-rebase.sh | 16 +++-
t/t3404-rebase-interactive.sh | 11 +++
t/t5407-post-rewrite-hook.sh | 23 +++++
4 files changed, 155 insertions(+), 49 deletions(-)
Range-diff against v1:
1: 65af2ac07a2 = 1: 65af2ac07a2 t3400: restore coverage for note copying with apply backend
2: 02670f57e7d = 2: 02670f57e7d sequencer: move definition of is_final_fixup()
3: 16fba1e823b ! 3: 3d79362332c sequencer: be more careful with external merge
@@ sequencer.c: static int do_pick_commit(struct repository *r,
+ opts->xopts.nr, opts->xopts.v,
common, oid_to_hex(&head), remotes);
+ /*
-+ * If the there were conflicts, try_merge_command() returns 1,
++ * If there were conflicts, try_merge_command() returns 1,
+ * any other no-zero return code means that either the merge
+ * command could not be run, or it failed to merge.
+ */
4: 3ffd06d6509 ! 4: fc89e77c6e8 sequencer: never reschedule on failed commit
@@ sequencer.c: static int do_pick_commit(struct repository *r,
*check_todo = 1;
}
+ /*
-+ * If "git commit" failed to run than res == -1 but we dont
++ * If "git commit" failed to run then res == -1, but we don't
+ * want reschedule the last command because the picking the
+ * commit was successful.
+ */
5: cb286ac70d7 ! 5: 26eef6c0958 sequencer: remove unnecessary "or" in pick_one_commit()
@@ Commit message
If error_with_patch(..., res, ...) succeeds then it returns "res", if
it fails then it returns -1. This means that or-ing the return value
- with "res" is pointless the result is the same as the return value.
+ with "res" is pointless as the result is the same as the return value.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
6: 1585d47e2ea = 6: 26dc48951ce sequencer: simplify handing of fixup with conflicts
7: 4386ca67d10 = 7: 71ed717d322 sequencer: remove unnecessary condition in pick_one_commit()
8: f51751fa3ec ! 8: e8b7fa4c59e sequencer: simplify pick_one_commit()
@@ Commit message
sequencer: simplify pick_one_commit()
Unless we're rebasing all we do in pick_one_commit() is call
- do_pick_commit() and return its result. Simplify the code by returing
+ do_pick_commit() and return its result. Simplify the code by returning
early if we're not rebasing so that we don't have to continually call
is_rebase_i() in the rest of the function. Note that there are a couple
of conditions that do not call is_rebase_i() but they check for either
an "edit" or a "fixup" command, both of which imply we're rebasing.
+
+ The only block that does not return early is the one guarded by
+ "!res". Move the return into that block to make it clear that after
+ recording the commit as rewritten all we do is return from the function.
As the conditional blocks are all mutually exclusive (either the
conditions are mutually exclusive, or an earlier conditional block
that would match a later one contains a "return" statement) chain
them together with "else if" to make that clear.
+
+ While we could remove "res" from the conditions below "if (!res)"
+ they are left alone because, when we start using an enum in the next
+ commit, it makes it clear that these clauses are handling cases where
+ there are conflicts.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
@@ sequencer.c: static int pick_one_commit(struct repository *r,
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
- if (res && is_fixup(item->command)) {
++ return 0;
+ } else if (res && is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
@@ sequencer.c: static int pick_one_commit(struct repository *r,
int to_amend = 0;
struct object_id oid;
+@@ sequencer.c: static int pick_one_commit(struct repository *r,
+ return error_with_patch(r, item->commit, arg, item->arg_len,
+ opts, res, to_amend);
+ }
+- return res;
++
++ BUG("Unhandled return value from do_pick_commit()");
+ }
+
+ static int pick_commits(struct repository *r,
9: 2541a4d6e3d < -: ----------- sequencer: return early from pick_one_commit() on success
10: e4050ead27f = 9: 4fb641afb3c sequencer: use an enum to represent result of picking a commit
11: 26551f2687b ! 10: c89234dd949 sequencer: do not record dropped commits as rewritten
@@ Commit message
when rewording a fast-forwarded commit.
Reported-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
+ Tested-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
## sequencer.c ##
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply
* [PATCH] fast-export: standardize usage string and SYNOPSIS
From: Christian Couder @ 2026-07-13 12:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Justin Tobler,
Christian Couder, Christian Couder
The output of `git fast-export -h` currently starts with:
usage: git fast-export [<rev-list-opts>]
while the SYNOPSIS section in this command's documentation shows:
'git fast-export' [<options>] | 'git fast-import'
Let's make both of these consistent with each other and with other Git
commands by describing the arguments with:
[<options>] [<revision-range>] [[--] <path>...]
This takes into account the following:
- `git fast-export` accepts both rev-list arguments and a number of
genuine options of its own (--[no-]progress, --[no-]signed-tags,
--[no-]signed-commits, etc).
- `git fast-export` was the only command using `[<rev-list-opts>]`
while many other commands describe their revision arguments as
`[<revision-range>] [[--] <path>...]`.
- In the DESCRIPTION section of the documentation, it's already
mentioned several times that the output should eventually be fed to
`git fast-import`.
This also enables us to remove fast-export from
"t/t0450/adoc-help-mismatches".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
CI tests passed:
https://github.com/chriscool/git/actions/runs/29245342050
Documentation/git-fast-export.adoc | 2 +-
builtin/fast-export.c | 2 +-
t/t0450/adoc-help-mismatches | 1 -
3 files changed, 2 insertions(+), 3 deletions(-)
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'
+'git fast-export' [<options>] [<revision-range>] [[--] <path>...]
DESCRIPTION
-----------
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0be43104dc..629d7c591a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -33,7 +33,7 @@
#include "gpg-interface.h"
static const char *const fast_export_usage[] = {
- N_("git fast-export [<rev-list-opts>]"),
+ N_("git fast-export [<options>] [<revision-range>] [[--] <path>...]"),
NULL
};
diff --git a/t/t0450/adoc-help-mismatches b/t/t0450/adoc-help-mismatches
index e8d6c13ccd..c4a55ff4e3 100644
--- a/t/t0450/adoc-help-mismatches
+++ b/t/t0450/adoc-help-mismatches
@@ -12,7 +12,6 @@ column
credential
credential-cache
credential-store
-fast-export
fast-import
fetch-pack
fmt-merge-msg
--
2.55.0.124.gac3b7a3d2d
^ permalink raw reply related
* Re: [PATCH v8 2/2] config: add "worktree" and "worktree/i" includeIf conditions
From: Patrick Steinhardt @ 2026-07-13 11:16 UTC (permalink / raw)
To: me; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano, Phillip Wood
In-Reply-To: <20260710-includeif-worktree-v8-2-04686d8a616c@black-desk.cn>
On Fri, Jul 10, 2026 at 02:43:30PM +0800, Chen Linxuan via B4 Relay wrote:
> diff --git a/Documentation/config.adoc b/Documentation/config.adoc
> index 15b1a4d59347..1ef72de62f2b 100644
> --- a/Documentation/config.adoc
> +++ b/Documentation/config.adoc
> @@ -146,6 +146,51 @@ refer to linkgit:gitignore[5] for details. For convenience:
> This is the same as `gitdir` except that matching is done
> case-insensitively (e.g. on case-insensitive file systems)
>
> +`worktree`::
> + The data that follows the keyword `worktree` and a colon is used as a
> + glob pattern. If the working directory of the current worktree matches
> + the pattern, the include condition is met.
> ++
> +The worktree location is the path where files are checked out (as returned
> +by `git rev-parse --show-toplevel`). This is different from `gitdir`, which
> +matches the `.git` directory path. In a linked worktree, the worktree path
> +is the directory where that worktree's files are located, not the main
> +repository's `.git` directory.
> ++
> +The pattern uses the same glob syntax as `gitdir` (including `~/`, `./`,
> +`**/`, and trailing-`/` prefix matching). This condition will never match
> +in a bare repository (which has no worktree).
> ++
> +Unlike `gitdir`, the `worktree` condition currently matches only the
> +realpath-resolved worktree location. If the working tree was entered via a
> +symbolic link, a pattern that uses the symbolic-link spelling may not match;
> +use the real path instead.
It might be worth noticing that this is essentially a limitation and
that this limitation may be fixed at a later point in time. But that
alone is not worth a reroll.
> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index f3892578e4ff..4e840dfdb35b 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh
> @@ -396,4 +396,132 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di
> test_must_fail nongit git --git-dir=nonexistent config get foo.bar
> '
>
> +# worktree: conditional include tests
> +
> +test_expect_success 'conditional include, worktree bare repo' '
> + git init --bare wt-bare &&
> + (
> + cd wt-bare &&
> + echo "[includeIf \"worktree:/\"]path=bar-bare" >>config &&
> + echo "[test]wtbare=1" >bar-bare &&
> + test_must_fail git config test.wtbare
> + )
> +'
> +
> +test_expect_success 'conditional include, worktree multiple worktrees' '
> + git init wt-multi &&
> + (
> + cd wt-multi &&
> + test_commit initial &&
> + git worktree add -b linked-branch ../wt-linked HEAD &&
> + git worktree add -b prefix-branch ../wt-prefix/linked HEAD
> + ) &&
> + wt_main="$(cd wt-multi && pwd)" &&
> + wt_linked="$(cd wt-linked && pwd)" &&
> + wt_prefix_parent="$(cd wt-prefix && pwd)" &&
> + cat >>wt-multi/.git/config <<-EOF &&
> + [includeIf "worktree:$wt_main"]
> + path = main-config
> + [includeIf "worktree:$wt_linked"]
> + path = linked-config
> + [includeIf "worktree:$wt_prefix_parent/"]
> + path = prefix-config
> + EOF
> + echo "[test]mainvar=main" >wt-multi/.git/main-config &&
> + echo "[test]linkedvar=linked" >wt-multi/.git/linked-config &&
> + echo "[test]prefixvar=prefix" >wt-multi/.git/prefix-config &&
> + echo main >expect &&
> + git -C wt-multi config test.mainvar >actual &&
> + test_cmp expect actual &&
> + test_must_fail git -C wt-multi config test.linkedvar &&
> + test_must_fail git -C wt-multi config test.prefixvar &&
> + echo linked >expect &&
> + git -C wt-linked config test.linkedvar >actual &&
> + test_cmp expect actual &&
> + test_must_fail git -C wt-linked config test.mainvar &&
> + test_must_fail git -C wt-linked config test.prefixvar &&
> + echo prefix >expect &&
> + git -C wt-prefix/linked config test.prefixvar >actual &&
> + test_cmp expect actual &&
> + test_must_fail git -C wt-prefix/linked config test.mainvar &&
> + test_must_fail git -C wt-prefix/linked config test.linkedvar
> +'
> +
> +test_expect_success SYMLINKS 'conditional include, worktree resolves symlinks' '
> + mkdir real-wt &&
> + ln -s real-wt link-wt &&
> + git init link-wt/repo &&
> + (
> + cd link-wt/repo &&
> + # repo->worktree resolves symlinks, so use real path in pattern
> + echo "[includeIf \"worktree:**/real-wt/repo\"]path=bar-link" >>.git/config &&
> + echo "[test]wtlink=2" >.git/bar-link &&
> + echo 2 >expect &&
> + git config test.wtlink >actual &&
> + test_cmp expect actual
> + )
> +'
There should arguably be a test with `test_expect_failure` that shows
that we in theory _want_ to use both the realpath, but also the
symlinked path to resolve this.
In any case, I think this version is good enough. It's a bit sad that we
cannot easily handle symlinked paths right now, but I'd say that this is
acceptable for now. We might fix this eventually, once we have cleaned
up "setup.c" to not modify global state left and right.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 0/2] t9811: reformat and modernize tests
From: Patrick Steinhardt @ 2026-07-13 11:10 UTC (permalink / raw)
To: Marcelo Machado Lage; +Cc: git
In-Reply-To: <20260711160447.99708-1-marcelomlage@usp.br>
On Sat, Jul 11, 2026 at 01:04:45PM -0300, Marcelo Machado Lage wrote:
> This patch series reformats and modernizes the t9811 tests.
> Changes since v1:
> - Break long && chains into multiple lines according to how git tests are
> written nowadays. This was suggested by Patrick Steinhardt.
> - Replace 'test -f' calls by more useful 'test_path_*' helpers as the
> second commit in the series.
Thanks, I'm happy with this version!
Patrick
^ permalink raw reply
* Re: cygwin v2.55.0 test failures
From: Patrick Steinhardt @ 2026-07-13 11:06 UTC (permalink / raw)
To: Ramsay Jones
Cc: GIT Mailing-list, Johannes Schindelin, Junio C Hamano,
Johannes Sixt, Adam Dinwoodie, Torsten Bögershausen
In-Reply-To: <f65466c9-bede-472e-ad57-e72a5289be27@ramsayjones.plus.com>
On Fri, Jul 10, 2026 at 07:32:23PM +0100, Ramsay Jones wrote:
[snip]
> Note that Patrick wanted to have a clean test-suite run on cygwin, so in
> commit 5f8af25ff9 ("t5500, t5601: skip tests which exercise paths with '[::1]'
> on Cygwin", 2024-10-16), he suppressed the test failures in t5500 and t5601.
> (that was about the time of the v2.48.0 release).
>
> The changes to tests t5500 and t5601, in the patch given below, essentially
> reverts Patrick's commit 5f8af25ff9. This fixes all of the tests in t5601 and
> ten of the twelve failures in t5500. (I don't recall what happened to t5580,
> the single failure - the push test - was fixed somewhere between v2.43.0 and
> v2.44.0-rc0).
Yeah, this was merely papering over issues. I'd very much welcome a
revert and proper fix for this.
> As luck would have it, I left a note to myself about the remaining two
> failure cases. This leads to the remaining hunk, to connect.c, in the patch
> below; ie. the removal of a conditional (which should only fire for GfW and
> cygwin). The '#ifdef DUMMY/#endif' should probably be replaced with an
> '#ifdef GIT_WINDOWS_NATIVE/#endif' so that GfW is not affected. (Having said
> that, I suspect that even GfW should drop it ['somebody was smoking something
> exotic'], but I have no way to test it, so ...).
>
> With this final hunk, this patch results in a clean test-suite run. :)
Nice :)
By the way: I was pondering multiple times over whether or not we should
add Cygwin to our CI matrix. It seems to be sufficiently different from
both MSYS2 and native Win32 to have its own set of compatibility issues,
so that could be worth it?
> connect.c | 2 ++
> git-compat-util.h | 39 +++++++++++++++++++++++++++++++++++++++
> t/t5500-fetch-pack.sh | 14 ++++----------
> t/t5601-clone.sh | 11 ++---------
> 4 files changed, 47 insertions(+), 19 deletions(-)
For the record: I don't really have much of an opinion on this given
that I tend to not use Windows, except when I (once again) break some
tests there. Especially the path handling si something that tends to
cause lots of confusion on my side.
Patrick
^ permalink raw reply
* [PATCH v10 7/7] graph: add --[no-]graph-indent and log.graphIndent
From: Pablo Sabater @ 2026-07-13 10: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-v10-0-82ddab26bc96@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..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;
/* 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 v10 6/7] graph: move config reading into graph_read_config()
From: Pablo Sabater @ 2026-07-13 10: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-v10-0-82ddab26bc96@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 v10 5/7] graph: wrap cascading commits after 4 columns
From: Pablo Sabater @ 2026-07-13 10: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-v10-0-82ddab26bc96@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 v10 4/7] graph: indent visual root in graph
From: Pablo Sabater @ 2026-07-13 10: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-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 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 v10 3/7] graph: add a 2 commit buffer for lookahead
From: Pablo Sabater @ 2026-07-13 10: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-v10-0-82ddab26bc96@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 v10 2/7] revision: add next_commit_to_show()
From: Pablo Sabater @ 2026-07-13 10: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-v10-0-82ddab26bc96@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 v10 1/7] lib-log-graph: move check_graph function
From: Pablo Sabater @ 2026-07-13 10: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-v10-0-82ddab26bc96@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 v10 0/7] graph: indent visual roots in graph
From: Pablo Sabater @ 2026-07-13 10: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: <20260711-ps-pre-commit-indent-v9-0-eab6676e82f7@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/29241054418
[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
V9 DIFF:
- visual roots cascading now wrap after 4 columns. This was introduced
into a new commit to make reviewing easier because the Main one is
already big and has gone through multiple rounds already.
- Made a new graph_read_config() function where the calls to
repo_config_get_*() live to leave graph_init() simpler.
- Added --[no-]graph-indent and log.graphIndent options so a user can
set his preferences about the graph indentation.
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 v9:
1: 22ab444372 = 1: 9541b410b7 lib-log-graph: move check_graph function
2: ebb88c8b29 = 2: 4f8fb2cc1d revision: add next_commit_to_show()
3: 0705ee321e = 3: b50574bbe1 graph: add a 2 commit buffer for lookahead
4: fa2e60fb3f = 4: fc3a8253fd graph: indent visual root in graph
-: ---------- > 5: 204aae5061 graph: wrap cascading commits after 4 columns
-: ---------- > 6: 1b42ed86a1 graph: move config reading into graph_read_config()
-: ---------- > 7: 737331b68d graph: add --[no-]graph-indent and log.graphIndent
---
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
^ permalink raw reply
* Re: [PATCH v2 1/8] odb/source-packed: improve lookup when enumerating objects
From: Patrick Steinhardt @ 2026-07-13 9:54 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Justin Tobler, Junio C Hamano
In-Reply-To: <alFxRvkfNgJRCQTB@com-79390>
On Fri, Jul 10, 2026 at 03:25:10PM -0700, Taylor Blau wrote:
> On Fri, Jul 10, 2026 at 10:48:53AM +0200, Patrick Steinhardt wrote:
> > Fix the issue by using `packed_object_info()` directly.
>
> What you wrote here makes sense to me insofar as I understand the
> pluggable ODB code.
>
> However, I am confused by the way this function is written in general.
> We use `bsearch_one_midx()` to locate the first possible MIDX position
> in which an object matching the given prefix may exist, which is
> sensible. However, we go from that position up to "num", where "num" is
> the total number of objects in the MIDX!
>
> Functionally this is not incorrect as we will happily discard objects
> that do not match the prefix. But it causes us to waste CPU cycles
> repeatedly calling `match_hash()` (at least for the first byte of the
> prefix) for objects that we know will match.
That's not quite true though, as we abort iteration as soon as
`match_hash()` tells us that the prefix doesn't match anymore.
Or do you mean that `num` should only be `m->num_objects` instead of
also iterating through `num_objects_in_base`? I have to admit that I'm
alwas struggling with the chained MIDX. It's never quite clear to me
whether a given function cares about the complete chain or whether it
really only cares about a single MIDX.
In any case, this code ultimately derives from 3f5f1cff92 (midx:
introduce `bsearch_one_midx()`, 2024-08-06). If one squints a bit you
can see that it's still roughly in the same shape.
> How often do we call this function with a prefix longer than a
> single byte? I have no idea, but I would suspect that it makes up the
> majority of calls. If we read the OID fanout chunk, we could narrow the
> range that we enumerate through, and only compare the second byte
> onwards of the given prefix, if one exists. In the single-byte prefix
> case, this means that we shouldn't have to do any memory comparisons at
> all.
The function is currently used to find unique prefixes and to
disambiguate object names. So whenever we either want to abbreviate a
object ID or in case we cant to figure out whether a given object ID
prefix is unique we'll end up calling it.
If this logic is currently wrong (or at least wasteful) though I'd
propose to fix this in a separate series, as it's been this way for
quite a while.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects
From: Patrick Steinhardt @ 2026-07-13 9:53 UTC (permalink / raw)
To: Jeff King; +Cc: Taylor Blau, git, Justin Tobler, Junio C Hamano
In-Reply-To: <20260711080114.GA1470749@coredump.intra.peff.net>
On Sat, Jul 11, 2026 at 04:01:14AM -0400, Jeff King wrote:
> On Fri, Jul 10, 2026 at 03:34:53PM -0700, Taylor Blau wrote:
>
> > However, the remaining `show_objects_for_type()` callers from within
> > `traverse_bitmap_commit_list()` do *not* bother to inspect the return
> > value, despite taking in an arbitrary 'show_reachable_fn', which itself
> > may return a non-zero value.
> >
> > I guess this must be effectively OK in practice with respect to the
> > existing code for the same reason you indicate in the commit message
> > above, but we should change this function to *also* propagate non-zero
> > return values to eliminate the foot-gun completely.
>
> The matching non-bitmap traverse_commit_list() does not allow aborting
> based on callback returns, either. In fact, its callbacks return void!
>
> Whichever direction we go, those two should probably stay in sync (so
> either both should allow aborting early with a non-zero return, or both
> should return void).
That's fair. But adapting `traverse_commit_list()` requires tons of
changes all over the tree, so I'm inclined to rather leave both
`traverse_bitmap_commit_list()` and `traverse_commit_list()` as-is.
Does that work for both of you?
Patrick
^ permalink raw reply
* Re: What's cooking in git.git (Jul 2026, #04)
From: Harald Nordgren @ 2026-07-13 8:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqcxwvk6q1.fsf@gitster.g>
> * hn/checkout-track-fetch (2026-06-24) 2 commits
> - checkout: extend --track with a "fetch" mode to refresh start-point
> - branch: expose helpers for finding the remote owning a tracking ref
>
> The 'git checkout --track=...' command has been taught to optionally
> fetch the branch from the remote the new branch will work with.
>
> Waiting for response(s) to review comment(s).
> cf. <xmqq5x37h6fj.fsf@gitster.g>
> source: <pull.2281.v15.git.git.1782338098.gitgitgadget@gmail.com>
I don't know if this is accurate. I think I answered every review
comment, maybe better to say it's stalled.
Harald
^ permalink raw reply
* Re: [PATCH v9 0/4] graph: indent visual roots in graph
From: Chandra Pratap @ 2026-07-13 7:41 UTC (permalink / raw)
To: Pablo Sabater
Cc: Mirko Faina, git, ayu.chandekar, christian.couder, gitster,
jltobler, karthik.188, krka, peff, phillip.wood,
siddharthasthana31
In-Reply-To: <DJWR4GEV14P4.3G9N0ZL1R8VDL@gmail.com>
[snip]
> I agree that having an infinite stair is not a good solution. the 3
> column wrap looks reasonable.
>
> I see two cases with this wrap:
>
> 1. No conflict case:
>
> A
> B
> C
> D
> E
> F
>
> No ambiguity, this would be the ideal case.
>
> 2. Ambiguity:
>
> If it happens that the visual number on visual roots meet the condition
> (number_of_visual_roots % 3 == 0) and the next commit is NOT a visual
> root this would happen:
>
> A
> B
> C
> D
> E
> E
>
> Which would be ambiguous. The solution is to check with the lookahead
> buffer that we have since patch 3 if the next is a visual root, if it's
> not we indent D anyway:
>
> A
> B
> C
> D
> E
> E
>
> Which I find the pyramid effect uncomfortable.
> What about capping at 4 columns?
>
> 1.
>
> A
> B
> C
> D
> E
> F
> G
> H
>
> 2.
>
> A
> B
> C
> D
> E
> F
> F
>
> I prefer the 4 column wrap because it looks more abrupt and IMO shows
> better that the commits are unrelated.
>
> What do you think?
I agree with Mirko, the 4-column wrap looks like a reasonable compromise.
> Also, about the no-opt option "--no-graph-indent" is still wanted
> regardless of the final design that we choose?
I feel indifferent about this personally, but there are clearly people who have
a use-case for such a flag.
Let us add an explicit opt-out flag: --no-graph-indent alongside a configuration
variable: graph.indent, log.graphIndent, or something similar.
A heads up: I think it would best to add these changes as two new commits to
the series.
Thanks,
Chandra.
^ permalink raw reply
* Re: What's cooking in git.git (Jul 2026, #05)
From: Michael Montalbo @ 2026-07-13 6:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqik6j1m7u.fsf@gitster.g>
On Sun, Jul 12, 2026 at 10:40 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> * mm/diff-process-hunks (2026-06-14) 6 commits
> - blame: consult diff process for no-hunk detection
> - diff: bypass diff process with --no-ext-diff and in format-patch
> - diff: add long-running diff process via diff.<driver>.process
> - sub-process: separate process lifecycle from hashmap management
> - userdiff: add diff.<driver>.process config
> - xdiff: support external hunks via xpparam_t
>
> A new 'diff.<driver>.process' configuration has been introduced to
> allow a long-running external process to act as a hunk provider,
> allowing external tools to control which lines Git considers changed
> while leaving all output formatting (word diff, color, blame, etc.) to
> Git's standard pipeline.
>
> Expecting a reroll for too long, stalled.
> cf. <CAC2Qwm+P=fZOtpfMPeMiSXf3Afk6OLYpTP8Br78_PRA8WNL1Wg@mail.gmail.com>
> source: <pull.2120.v4.git.1781463564.gitgitgadget@gmail.com>
>
Apologies for the delayed update. I will have a reroll ready for this topic this
week.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox