* [PATCH v4 1/6] t4219: test more --follow cases across merges
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
@ 2026-10-30 20:07 ` Miklos Vajna
2026-10-30 20:07 ` [PATCH v4 2/6] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
` (6 subsequent siblings)
7 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:07 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
A later commit in this series will speed up 'git log --follow' for
merge commits. Add tests for cases which already work, but had no
test coverage, so it's easy to break them while optimizing:
- --follow through a fork with renames on both sides, with -c, --cc
and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit
orders, since the result could depend on the walk order
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their
first parent, but still walks all parents
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
t/t4219-log-follow-merge.sh | 124 ++++++++++++++++++++++++++++++++++++
1 file changed, 124 insertions(+)
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index e370f82955..c109c29f70 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -126,4 +126,128 @@ test_expect_success '--follow follows renames through a fork in a single history
test_cmp expect actual.sorted
'
+for opt in -c --cc --remerge-diff
+do
+ test_expect_success "--follow follows renames through a fork with $opt" '
+ git -C diamond log --follow $opt --pretty=tformat:%s path >log &&
+ grep "^[A-Z]: " log | sort >actual &&
+ cat >expect <<-\EOF &&
+ A: add path0
+ B: modify path0 on upper
+ C: modify path0 on lower
+ M: merge with rename to path
+ X: rename path0 to path1
+ Y: rename path0 to path2
+ Z: modify path
+ EOF
+ test_cmp expect actual
+ '
+done
+
+# Build a merge that takes the renamed path from the second parent, while the
+# first parent only has the old path. $2 decides which side is committed first,
+# which decides the walk order.
+setup_rename_on_second_parent () {
+ git init $1 &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old &&
+ git -C $1 add old &&
+ test_tick &&
+ git -C $1 commit -m "A: add old" &&
+ printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old &&
+ test_tick &&
+ git -C $1 commit -am "B: modify old" &&
+ git -C $1 branch topic &&
+
+ if test "$2" = topic-first
+ then
+ git -C $1 checkout topic &&
+ git -C $1 mv old new &&
+ test_tick &&
+ git -C $1 commit -m "R: rename old to new" &&
+ git -C $1 checkout master
+ fi &&
+
+ echo other >$1/other &&
+ git -C $1 add other &&
+ test_tick &&
+ git -C $1 commit -m "M: unrelated change on master" &&
+
+ if test "$2" = master-first
+ then
+ git -C $1 checkout topic &&
+ git -C $1 mv old new &&
+ test_tick &&
+ git -C $1 commit -m "R: rename old to new" &&
+ git -C $1 checkout master
+ fi &&
+
+ test_tick &&
+ git -C $1 merge --no-ff -m "merge topic" topic
+}
+
+test_expect_success 'setup merge taking a rename from the second parent' '
+ setup_rename_on_second_parent rename-master-first master-first &&
+ setup_rename_on_second_parent rename-topic-first topic-first
+'
+
+for order in master-first topic-first
+do
+ test_expect_success "--follow follows a rename from the second parent ($order)" '
+ git -C rename-$order log --follow --pretty=tformat:%s new >actual &&
+ cat >expect <<-\EOF &&
+ R: rename old to new
+ B: modify old
+ A: add old
+ EOF
+ test_cmp expect actual
+ '
+done
+
+test_expect_success 'setup merge keeping the path, other side renamed it' '
+ git init keep &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F &&
+ git -C keep add F &&
+ test_tick &&
+ git -C keep commit -m "O: add F" &&
+ git -C keep branch B &&
+ printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F &&
+ test_tick &&
+ git -C keep commit -am "A1: update F in place" &&
+
+ git -C keep checkout B &&
+ git -C keep mv F G &&
+ test_tick &&
+ git -C keep commit -m "B1: rename F to G" &&
+ printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G &&
+ test_tick &&
+ git -C keep commit -am "B2: update G" &&
+
+ git -C keep checkout master &&
+ test_tick &&
+ git -C keep merge -s ours -m "M: merge B, keep F, drop G" B
+'
+
+test_expect_success '--follow follows a rename on the side the merge did not take' '
+ git -C keep log --follow --pretty=tformat:%s F >actual &&
+ cat >expect <<-\EOF &&
+ B2: update G
+ B1: rename F to G
+ A1: update F in place
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--follow with --diff-merges=first-parent follows all parents' '
+ git -C keep log --follow --diff-merges=first-parent --no-patch \
+ --pretty=tformat:%s F >actual &&
+ cat >expect <<-\EOF &&
+ B2: update G
+ B1: rename F to G
+ A1: update F in place
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v4 2/6] log: extract showing the remerge/combined diff of a merge commit
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
2026-10-30 20:07 ` [PATCH v4 1/6] t4219: test more --follow cases across merges Miklos Vajna
@ 2026-10-30 20:07 ` Miklos Vajna
2026-10-30 20:08 ` [PATCH v4 3/6] log: fix -m --follow losing the history of later parents Miklos Vajna
` (5 subsequent siblings)
7 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:07 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
No functional changes intended. A later commit in this series will also
call log_tree_merge_diff() after the parent loop of log_tree_diff(), so
extract that code to a function to avoid duplicating it.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..b4c97b86f5 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1180,6 +1180,26 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
diff_free(&diff_opts);
}
+/* Show the remerge or combined diff of a merge commit */
+static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
+ struct commit_list *parents,
+ struct object_id *oid)
+{
+ int octopus = (parents->next->next != NULL);
+
+ if (opt->remerge_diff) {
+ if (octopus) {
+ show_log(opt);
+ fprintf(opt->diffopt.file,
+ "diff: warning: Skipping remerge-diff "
+ "for octopus merges.\n");
+ return 1;
+ }
+ return do_remerge_diff(opt, parents, oid);
+ }
+ return do_diff_combined(opt, commit);
+}
+
/*
* Show the diff of a commit.
*
@@ -1220,20 +1240,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
}
if (is_merge) {
- int octopus = (parents->next->next != NULL);
-
- if (opt->remerge_diff) {
- if (octopus) {
- show_log(opt);
- fprintf(opt->diffopt.file,
- "diff: warning: Skipping remerge-diff "
- "for octopus merges.\n");
- return 1;
- }
- return do_remerge_diff(opt, parents, oid);
- }
- if (opt->combine_merges)
- return do_diff_combined(opt, commit);
+ if (opt->remerge_diff || opt->combine_merges)
+ return log_tree_merge_diff(opt, commit, parents, oid);
if (opt->separate_merges) {
if (!opt->first_parent_merges) {
/* Show parent info for multiple diffs */
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v4 3/6] log: fix -m --follow losing the history of later parents
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
2026-10-30 20:07 ` [PATCH v4 1/6] t4219: test more --follow cases across merges Miklos Vajna
2026-10-30 20:07 ` [PATCH v4 2/6] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
@ 2026-10-30 20:08 ` Miklos Vajna
2026-10-30 20:08 ` [PATCH v4 4/6] log: simplify --follow following renames for non-linear history Miklos Vajna
` (4 subsequent siblings)
7 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:08 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
With 'git log -m --follow', a merge is diffed against each of its
parents, with rename following. Take the diamond case of t4219: the two
sides of a fork rename path0 to path1 and to path2, and the merge
renames the file to path. Here 'git log -m --follow path' doesn't show
"A: add path0" and "C: modify path0 on lower".
The problem is that the parent loop in log_tree_diff() doesn't restore
the followed path between parents. When the diff against the first
parent finds a rename, the followed path is updated, and this has two
effects. First, the second parent is diffed with the already renamed
path, not with the path of the merge. Second, the path that
propagate_follow_pathspec_to_parent() records for each parent also
starts from that updated path, so history is lost on the side of the
second parent. Without -m, merges are not diffed, so the followed path
is not updated there.
Fix the problem by saving the path of the merge before the parent loop,
and restoring it before diffing each parent and after the loop. Do this
only for merges: for a commit with a single parent, the renamed path is
what --follow needs to continue with. Add a test for this, which shows
the full history of the diamond case with -m.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 13 +++++++++++++
t/t4219-log-follow-merge.sh | 16 ++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/log-tree.c b/log-tree.c
index b4c97b86f5..befc5c8b22 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1212,6 +1212,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
struct object_id *oid;
int is_merge;
int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
+ char *path = NULL;
if (!all_need_diff && !opt->merges_need_diff)
return 0;
@@ -1251,10 +1252,18 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
return 0;
}
+ if (is_merge && opt->diffopt.flags.follow_renames)
+ path = xstrdup_or_null(
+ pathspec_single_path(&opt->diffopt.pathspec));
+
showed_log = 0;
for (;;) {
struct commit *parent = parents->item;
+ /* Diff each parent with the path of the merge */
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
parse_commit_or_die(parent);
diff_tree_oid(get_commit_tree_oid(parent),
oid, "", &opt->diffopt);
@@ -1269,6 +1278,10 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
log->parent = parents->item;
opt->loginfo = log;
}
+ /* Propagate to parents starting from the path of the merge, too */
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec, path);
+ free(path);
return showed_log;
}
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index c109c29f70..36204ae2bf 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -144,6 +144,22 @@ do
'
done
+test_expect_success '--follow follows renames through a fork with -m' '
+ git -C diamond log --follow -m --pretty=tformat:%s path >log &&
+ grep "^[A-Z]: " log | sort >actual &&
+ cat >expect <<-\EOF &&
+ A: add path0
+ B: modify path0 on upper
+ C: modify path0 on lower
+ M: merge with rename to path
+ M: merge with rename to path
+ X: rename path0 to path1
+ Y: rename path0 to path2
+ Z: modify path
+ EOF
+ test_cmp expect actual
+'
+
# Build a merge that takes the renamed path from the second parent, while the
# first parent only has the old path. $2 decides which side is committed first,
# which decides the walk order.
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v4 4/6] log: simplify --follow following renames for non-linear history
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
` (2 preceding siblings ...)
2026-10-30 20:08 ` [PATCH v4 3/6] log: fix -m --follow losing the history of later parents Miklos Vajna
@ 2026-10-30 20:08 ` Miklos Vajna
2026-10-30 20:09 ` [PATCH v4 5/6] log: speed up " Miklos Vajna
` (3 subsequent siblings)
7 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:08 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Code added in 304812ed33 (log: improve --follow following renames for
non-linear history, 2026-06-22) implemented the feature by introducing a
new propagate_follow_pathspec_to_parent() function which adds a new call
to diff_tree_oid(). Also it introduces a new loop to record what path
each parent of a merge commit should use when following a path.
This was working, but it caused a performance regression: e.g. 'git log
--stat --follow -- include/linux/rsc_table.h' in linux.git went from 28s
to 6m43s. The new code diffs each merge against each of its parents with
rename detection, which is expensive when a parent doesn't have the
followed path yet. Fixing this would require adding even more custom
code, instead of just extending the existing code in log_tree_diff(). So
first simplify the code here, and fix the performance regression in a
follow-up commit.
Remove propagate_follow_pathspec_to_parent() and the loop calling it
for each parent. Instead, record the path to follow for each parent in
the parent loop of log_tree_diff(), right after diff_tree_oid() updated
the pathspec in case of a rename. Merges are not diffed by default, so
with --follow, run the same loop for them as well, but without showing
anything. With -c, --cc or --remerge-diff, show the merge diff after the
loop. With --diff-merges=first-parent, keep diffing the other parents
silently, unless the walk only follows first parents. This gets rid of
the additional call to diff_tree_oid(). The rework has no positive
performance impact yet, it just simplifies the code.
The previous commit restores the path of the merge before diffing each
parent, and also after the loop, for
propagate_follow_pathspec_to_parent(). Keep restoring it between
parents, but not after the loop, since there is no later propagation
anymore.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 96 +++++++++++++++++++++++-------------------------------
1 file changed, 41 insertions(+), 55 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index befc5c8b22..0aa27fb210 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
-/* Compute a path to follow in parent, if there is one */
-static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
- struct commit *commit,
- struct commit *parent)
-{
- struct diff_options diff_opts;
- const char *path;
-
- parse_commit_or_die(parent);
- repo_diff_setup(opt->diffopt.repo, &diff_opts);
- copy_pathspec(&diff_opts.pathspec, &opt->diffopt.pathspec);
- diff_opts.flags.recursive = 1;
- diff_opts.flags.follow_renames = 1;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
- diff_setup_done(&diff_opts);
- diff_tree_oid(get_commit_tree_oid(parent),
- get_commit_tree_oid(commit),
- "", &diff_opts);
-
- path = pathspec_single_path(&diff_opts.pathspec);
- if (path)
- remember_follow_pathspec(opt, parent, path);
-
- diff_queue_clear(&diff_queued_diff);
- diff_free(&diff_opts);
-}
-
/* Show the remerge or combined diff of a merge commit */
static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
struct commit_list *parents,
@@ -1208,10 +1181,11 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
{
int showed_log;
- struct commit_list *parents;
+ struct commit_list *parents, *all_parents;
struct object_id *oid;
int is_merge;
int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
+ int follow_only = 0;
char *path = NULL;
if (!all_need_diff && !opt->merges_need_diff)
@@ -1241,46 +1215,73 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
}
if (is_merge) {
- if (opt->remerge_diff || opt->combine_merges)
+ int merge_diff = opt->remerge_diff || opt->combine_merges;
+
+ if (merge_diff && !opt->diffopt.flags.follow_renames)
return log_tree_merge_diff(opt, commit, parents, oid);
- if (opt->separate_merges) {
+ if (opt->separate_merges && !merge_diff) {
if (!opt->first_parent_merges) {
/* Show parent info for multiple diffs */
log->parent = parents->item;
}
+ } else if (opt->diffopt.flags.follow_renames) {
+ /* Diff only to find the path to follow in parents */
+ follow_only = 1;
} else
return 0;
}
- if (is_merge && opt->diffopt.flags.follow_renames)
+ if (opt->diffopt.flags.follow_renames)
path = xstrdup_or_null(
pathspec_single_path(&opt->diffopt.pathspec));
showed_log = 0;
+ all_parents = parents;
for (;;) {
struct commit *parent = parents->item;
- /* Diff each parent with the path of the merge */
- if (path)
- set_pathspec_to_single_path(&opt->diffopt.pathspec,
- path);
parse_commit_or_die(parent);
diff_tree_oid(get_commit_tree_oid(parent),
oid, "", &opt->diffopt);
- log_tree_diff_flush(opt);
+ if (follow_only) {
+ diff_queue_clear(&diff_queued_diff);
+ opt->diffopt.found_follow = 0;
+ } else {
+ log_tree_diff_flush(opt);
+ }
showed_log |= !opt->loginfo;
+ /* The diff updated the pathspec in case of a rename */
+ if (path)
+ remember_follow_pathspec(opt, parent,
+ pathspec_single_path(&opt->diffopt.pathspec));
+
/* Set up the log info for the next parent, if any.. */
parents = parents->next;
- if (!parents || opt->first_parent_merges)
+ if (!parents)
break;
+ if (opt->first_parent_merges) {
+ if (!path || opt->first_parent_only)
+ break;
+ /* Only find the path to follow in the other parents */
+ follow_only = 1;
+ }
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ if (follow_only)
+ continue;
log->parent = parents->item;
opt->loginfo = log;
}
- /* Propagate to parents starting from the path of the merge, too */
- if (path)
- set_pathspec_to_single_path(&opt->diffopt.pathspec, path);
+ if (follow_only && (opt->remerge_diff || opt->combine_merges)) {
+ /* Show the merge diff with the path of this commit */
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ showed_log = log_tree_merge_diff(opt, commit, all_parents, oid);
+ }
free(path);
return showed_log;
}
@@ -1319,21 +1320,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
if (shown)
show_diff_of_diff(opt);
-
- /* Record what path each parent of this commit should use */
- if (opt->diffopt.flags.follow_renames) {
- struct commit_list *parents = get_saved_parents(opt, commit);
- if (parents && parents->next) {
- struct commit_list *p;
- for (p = parents; p; p = p->next)
- propagate_follow_pathspec_to_parent(opt, commit,
- p->item);
- } else if (parents) {
- remember_follow_pathspec(opt, parents->item,
- pathspec_single_path(&opt->diffopt.pathspec));
- }
- }
-
opt->loginfo = NULL;
maybe_flush_or_die(opt->diffopt.file, "stdout");
opt->diffopt.no_free = no_free;
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v4 5/6] log: speed up --follow following renames for non-linear history
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
` (3 preceding siblings ...)
2026-10-30 20:08 ` [PATCH v4 4/6] log: simplify --follow following renames for non-linear history Miklos Vajna
@ 2026-10-30 20:09 ` Miklos Vajna
2026-11-06 14:14 ` Patrick Steinhardt
2026-10-30 20:10 ` [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take Miklos Vajna
` (2 subsequent siblings)
7 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:09 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Test the performance of 'git log --stat --follow --
include/linux/rsc_table.h' on linux.git, it used to take about 28
seconds, and since commit 304812ed33 (log: improve --follow following
renames for non-linear history, 2026-06-22), it takes 6m43s.
What happens is that log_tree_diff() diffs each merge against all of its
parents with rename following. diff_tree_oid() only does the expensive
rename detection when the path is added relative to a parent, but that
is common for merges, e.g. when a branch forked before the path was
created merges mainline, and this slows down 'git log --follow'.
Fix the problem by diffing the merge against its parents in two passes:
- First pass: diff against the parents without rename following. If the
path is unchanged against a parent, then just follow that history in
all parents.
- Except when an other parent doesn't have the path, the merge has it
and a merge base of the two parents also has it, i.e. the path was
renamed or removed on that side: still do the rename following
against such a parent, otherwise the history on that side would be
lost. The merge base is only computed in this rare case, and
back-merges where the path was created after the other side forked
still skip the expensive rename detection.
- Second pass: only do the rename following against each parent if the
path changed relative to all parents.
Given that this way a parent may be asked to follow a path that it
doesn't have, also adjust remember_follow_pathspec(): if children
disagree on what should be the path for a commit, then prefer the path
that exists in that commit.
Now the above use case takes 36s, somewhat close to the original cost.
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 91 insertions(+), 5 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 0aa27fb210..0dc71be61e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -31,6 +31,7 @@
#include "range-diff.h"
#include "strmap.h"
#include "tree.h"
+#include "tree-walk.h"
#include "wildmatch.h"
#include "write-or-die.h"
#include "pager.h"
@@ -1110,6 +1111,17 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path)
PATHSPEC_LITERAL_PATH, "", paths);
}
+static int commit_has_path(struct rev_info *opt, struct commit *c,
+ const char *path)
+{
+ struct object_id oid;
+ unsigned short mode;
+
+ parse_commit_or_die(c);
+ return !get_tree_entry(opt->diffopt.repo, get_commit_tree_oid(c), path,
+ &oid, &mode);
+}
+
static void remember_follow_pathspec(struct rev_info *opt,
struct commit *c, const char *path)
{
@@ -1122,8 +1134,12 @@ static void remember_follow_pathspec(struct rev_info *opt,
init_follow_pathspec_slab(opt->follow_pathspec_slab);
}
slot = follow_pathspec_slab_at(opt->follow_pathspec_slab, c);
- if (*slot && !strcmp(*slot, path))
- return;
+ /* If children of 'c' disagree on the path, prefer the one 'c' has. */
+ if (*slot) {
+ if (!strcmp(*slot, path) || commit_has_path(opt, c, *slot) ||
+ !commit_has_path(opt, c, path))
+ return;
+ }
free(*slot);
*slot = xstrdup(path);
}
@@ -1153,6 +1169,29 @@ void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
+/*
+ * When the path in a merge is the same as in 'same', does the path still need
+ * renames to be followed in 'parent'? Only if the merge has the path, 'parent'
+ * doesn't have it, but its fork point with 'same' had it, e.g. 'parent'
+ * renamed the path.
+ */
+static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
+ struct commit *same, const char *path)
+{
+ struct commit_list *bases = NULL, *base;
+ int ret = 0;
+
+ if (commit_has_path(opt, parent, path) ||
+ !commit_has_path(opt, same, path))
+ return 0;
+ if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
+ exit(128);
+ for (base = bases; base && !ret; base = base->next)
+ ret = commit_has_path(opt, base->item, path);
+ commit_list_free(bases);
+ return ret;
+}
+
/* Show the remerge or combined diff of a merge commit */
static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
struct commit_list *parents,
@@ -1181,7 +1220,7 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
{
int showed_log;
- struct commit_list *parents, *all_parents;
+ struct commit_list *parents, *all_parents, *rename_parents = NULL;
struct object_id *oid;
int is_merge;
int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
@@ -1235,15 +1274,25 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
path = xstrdup_or_null(
pathspec_single_path(&opt->diffopt.pathspec));
+ if (follow_only) {
+ /*
+ * First look for a parent where the path is unchanged, without
+ * the expensive rename detection.
+ */
+ opt->diffopt.flags.follow_renames = 0;
+ }
+
showed_log = 0;
all_parents = parents;
for (;;) {
struct commit *parent = parents->item;
+ int changed = 1;
parse_commit_or_die(parent);
diff_tree_oid(get_commit_tree_oid(parent),
oid, "", &opt->diffopt);
if (follow_only) {
+ changed = diff_queued_diff.nr;
diff_queue_clear(&diff_queued_diff);
opt->diffopt.found_follow = 0;
} else {
@@ -1252,20 +1301,54 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
showed_log |= !opt->loginfo;
+ if (!changed) {
+ /*
+ * Same as in this parent, follow that history, but
+ * still look for renames in parents that had the path
+ * at the fork point.
+ */
+ struct commit_list **tail = &rename_parents;
+
+ for (parents = all_parents; parents;
+ parents = parents->next) {
+ if (parents->item != parent &&
+ follow_needs_renames(opt, parents->item,
+ parent, path))
+ tail = &commit_list_insert(parents->item,
+ tail)->next;
+ else
+ remember_follow_pathspec(opt,
+ parents->item,
+ path);
+ }
+ if (!rename_parents)
+ break;
+ opt->diffopt.flags.follow_renames = 1;
+ parents = rename_parents;
+ continue;
+ }
+
/* The diff updated the pathspec in case of a rename */
- if (path)
+ if (path && opt->diffopt.flags.follow_renames)
remember_follow_pathspec(opt, parent,
pathspec_single_path(&opt->diffopt.pathspec));
/* Set up the log info for the next parent, if any.. */
parents = parents->next;
+ if (!parents && follow_only &&
+ !opt->diffopt.flags.follow_renames) {
+ /* The path changed in all parents, look for renames. */
+ opt->diffopt.flags.follow_renames = 1;
+ parents = all_parents;
+ }
if (!parents)
break;
- if (opt->first_parent_merges) {
+ if (opt->first_parent_merges && !follow_only) {
if (!path || opt->first_parent_only)
break;
/* Only find the path to follow in the other parents */
follow_only = 1;
+ opt->diffopt.flags.follow_renames = 0;
}
if (path)
set_pathspec_to_single_path(&opt->diffopt.pathspec,
@@ -1275,6 +1358,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
log->parent = parents->item;
opt->loginfo = log;
}
+ if (follow_only)
+ opt->diffopt.flags.follow_renames = 1;
if (follow_only && (opt->remerge_diff || opt->combine_merges)) {
/* Show the merge diff with the path of this commit */
if (path)
@@ -1282,6 +1367,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
path);
showed_log = log_tree_merge_diff(opt, commit, all_parents, oid);
}
+ commit_list_free(rename_parents);
free(path);
return showed_log;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v4 5/6] log: speed up --follow following renames for non-linear history
2026-10-30 20:09 ` [PATCH v4 5/6] log: speed up " Miklos Vajna
@ 2026-11-06 14:14 ` Patrick Steinhardt
2026-11-09 8:02 ` Miklos Vajna
0 siblings, 1 reply; 43+ messages in thread
From: Patrick Steinhardt @ 2026-11-06 14:14 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, git
On Fri, Oct 30, 2026 at 09:09:11PM +0100, Miklos Vajna wrote:
> diff --git a/log-tree.c b/log-tree.c
> index 0aa27fb210..0dc71be61e 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -1153,6 +1169,29 @@ void release_follow_pathspec_slab(struct rev_info *opt)
> FREE_AND_NULL(opt->follow_pathspec_slab);
> }
>
> +/*
> + * When the path in a merge is the same as in 'same', does the path still need
> + * renames to be followed in 'parent'? Only if the merge has the path, 'parent'
> + * doesn't have it, but its fork point with 'same' had it, e.g. 'parent'
> + * renamed the path.
> + */
> +static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
> + struct commit *same, const char *path)
> +{
> + struct commit_list *bases = NULL, *base;
> + int ret = 0;
> +
> + if (commit_has_path(opt, parent, path) ||
> + !commit_has_path(opt, same, path))
> + return 0;
> + if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
> + exit(128);
> + for (base = bases; base && !ret; base = base->next)
> + ret = commit_has_path(opt, base->item, path);
> + commit_list_free(bases);
> + return ret;
> +}
With unrelated histories this would return no bases at all, and hence
we'd always return 0 in such a case. But 304812ed33d (log: improve
--follow following renames for non-linear history, 2026-06-22)
explicitly mentioned the use case of sub-tree merges of unrelated
histories, so isn't this a regression?
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v4 5/6] log: speed up --follow following renames for non-linear history
2026-11-06 14:14 ` Patrick Steinhardt
@ 2026-11-09 8:02 ` Miklos Vajna
0 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:02 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: gitster, szeder.dev, git
Hi Patrick,
On Fri, Nov 06, 2026 at 03:14:11PM +0100, Patrick Steinhardt <ps@pks.im> wrote:
> With unrelated histories this would return no bases at all, and hence
> we'd always return 0 in such a case. But 304812ed33d (log: improve
> --follow following renames for non-linear history, 2026-06-22)
> explicitly mentioned the use case of sub-tree merges of unrelated
> histories, so isn't this a regression?
You're right. The existing '--follow finds the pre-merge commit through
a subtree merge' test didn't catch this, because there no parent has the
path unchanged, so follow_needs_renames() is not called.
When there is no merge base, follow_needs_renames() now returns 1, so
renames are followed as before.
I've added a new '--follow finds the history of a copied file through a
subtree merge' test to 1/5 in v5 for such a case, and 5/5 now passes it.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
` (4 preceding siblings ...)
2026-10-30 20:09 ` [PATCH v4 5/6] log: speed up " Miklos Vajna
@ 2026-10-30 20:10 ` Miklos Vajna
2026-11-06 14:14 ` Patrick Steinhardt
2026-11-06 14:14 ` [PATCH v4 0/6] log: speed up --follow following renames for non-linear history Patrick Steinhardt
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
7 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:10 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Take a history where F is renamed to G on master, F is edited on a side
branch, and the side branch is merged with '-s ours', so the merge keeps
G and drops F. Here 'git log --follow G' doesn't show the edit of F on
the side branch, while it did before the previous commit.
The problem is that follow_needs_renames() only searches for renames in
a parent without the path if the merge base still has the path, which
covers the case when the parent renamed the path. Here the other side
of the merge did the rename, so the merge base only has the old name.
The parent is then told to follow G, which it doesn't have, and the
history of F on its side is lost. Searching for renames in each parent
that lacks the path would fix this, but that is the expensive search
that the previous commit avoids, e.g. 148 merges hit this case for
include/linux/rsc_table.h in linux.git.
Fix the problem by also searching for renames when the merge base
doesn't have the path, but the parent changed a file since the merge
base that the merge no longer has: only such a file can be the old name
of the path that still needs to be followed in the parent. This check
is cheap, since it only diffs the changes of the side branch, and it
rules out 146 of the 148 merges in linux.git. Then do a plain rename
detection from the parent to the merge, which only looks at the files
deleted by the merge, not at all files of the parent like --follow does.
Reported-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 96 +++++++++++++++++++++++++++++++++----
t/t4219-log-follow-merge.sh | 31 ++++++++++++
2 files changed, 117 insertions(+), 10 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 0dc71be61e..c2c3473f25 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1111,15 +1111,20 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path)
PATHSPEC_LITERAL_PATH, "", paths);
}
-static int commit_has_path(struct rev_info *opt, struct commit *c,
- const char *path)
+static int tree_has_path(struct rev_info *opt, struct object_id *tree,
+ const char *path)
{
struct object_id oid;
unsigned short mode;
+ return !get_tree_entry(opt->diffopt.repo, tree, path, &oid, &mode);
+}
+
+static int commit_has_path(struct rev_info *opt, struct commit *c,
+ const char *path)
+{
parse_commit_or_die(c);
- return !get_tree_entry(opt->diffopt.repo, get_commit_tree_oid(c), path,
- &oid, &mode);
+ return tree_has_path(opt, get_commit_tree_oid(c), path);
}
static void remember_follow_pathspec(struct rev_info *opt,
@@ -1169,26 +1174,97 @@ void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
+/*
+ * Did 'parent' change a file since 'base' that the merge (with tree 'oid') no
+ * longer has? Only such a file can be the source of a rename on the other side
+ * of the merge that still needs to be followed in 'parent'.
+ */
+static int follow_parent_changed_dropped_file(struct rev_info *opt,
+ struct commit *base,
+ struct commit *parent,
+ struct object_id *oid)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
+
+ repo_diff_setup(opt->diffopt.repo, &diff_opts);
+ diff_opts.flags.recursive = 1;
+ diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&diff_opts);
+ diff_tree_oid(get_commit_tree_oid(base), get_commit_tree_oid(parent),
+ "", &diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+
+ ret = DIFF_FILE_VALID(p->two) &&
+ !tree_has_path(opt, oid, p->two->path);
+ }
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
+/*
+ * Is 'path' in the merge (with tree 'oid') renamed from a file of 'parent'?
+ * Only look at the files that the merge deleted, not at all files of 'parent'
+ * like --follow does.
+ */
+static int follow_has_rename_source(struct rev_info *opt,
+ struct commit *parent,
+ struct object_id *oid, const char *path)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
+
+ repo_diff_setup(opt->diffopt.repo, &diff_opts);
+ diff_opts.flags.recursive = 1;
+ diff_opts.detect_rename = DIFF_DETECT_RENAME;
+ diff_opts.rename_score = opt->diffopt.rename_score;
+ diff_opts.single_follow = path;
+ diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&diff_opts);
+ diff_tree_oid(get_commit_tree_oid(parent), oid, "", &diff_opts);
+ diffcore_std(&diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+
+ ret = p->status == DIFF_STATUS_RENAMED &&
+ !strcmp(p->two->path, path);
+ }
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
/*
* When the path in a merge is the same as in 'same', does the path still need
* renames to be followed in 'parent'? Only if the merge has the path, 'parent'
- * doesn't have it, but its fork point with 'same' had it, e.g. 'parent'
- * renamed the path.
+ * doesn't have it, and either:
+ *
+ * - its fork point with 'same' had it, e.g. 'parent' renamed the path, or
+ * - 'same' renamed the path from a file that 'parent' changed.
*/
static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
- struct commit *same, const char *path)
+ struct commit *same, struct object_id *oid,
+ const char *path)
{
struct commit_list *bases = NULL, *base;
- int ret = 0;
+ int ret = 0, changed_dropped = 0;
if (commit_has_path(opt, parent, path) ||
!commit_has_path(opt, same, path))
return 0;
if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
exit(128);
- for (base = bases; base && !ret; base = base->next)
+ for (base = bases; base && !ret; base = base->next) {
ret = commit_has_path(opt, base->item, path);
+ if (!ret && !changed_dropped)
+ changed_dropped = follow_parent_changed_dropped_file(
+ opt, base->item, parent, oid);
+ }
commit_list_free(bases);
+ if (!ret && changed_dropped)
+ ret = follow_has_rename_source(opt, parent, oid, path);
return ret;
}
@@ -1313,7 +1389,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
parents = parents->next) {
if (parents->item != parent &&
follow_needs_renames(opt, parents->item,
- parent, path))
+ parent, oid, path))
tail = &commit_list_insert(parents->item,
tail)->next;
else
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index 36204ae2bf..a85fb78ca2 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -266,4 +266,35 @@ test_expect_success '--follow with --diff-merges=first-parent follows all parent
test_cmp expect actual
'
+test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
+ git init keep-renamed &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
+ git -C keep-renamed add F &&
+ test_tick &&
+ git -C keep-renamed commit -m "O: add F" &&
+ git -C keep-renamed branch B &&
+ git -C keep-renamed mv F G &&
+ test_tick &&
+ git -C keep-renamed commit -m "A1: rename F to G on master" &&
+
+ git -C keep-renamed checkout B &&
+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
+ test_tick &&
+ git -C keep-renamed commit -am "B2: update F on B" &&
+
+ git -C keep-renamed checkout master &&
+ test_tick &&
+ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
+'
+
+test_expect_success '--follow follows an edit on the side the merge did not take' '
+ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
+ cat >expect <<-\EOF &&
+ B2: update F on B
+ A1: rename F to G on master
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take
2026-10-30 20:10 ` [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take Miklos Vajna
@ 2026-11-06 14:14 ` Patrick Steinhardt
2026-11-09 8:03 ` Miklos Vajna
0 siblings, 1 reply; 43+ messages in thread
From: Patrick Steinhardt @ 2026-11-06 14:14 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, git
On Fri, Oct 30, 2026 at 09:10:14PM +0100, Miklos Vajna wrote:
> Take a history where F is renamed to G on master, F is edited on a side
> branch, and the side branch is merged with '-s ours', so the merge keeps
> G and drops F. Here 'git log --follow G' doesn't show the edit of F on
> the side branch, while it did before the previous commit.
>
> The problem is that follow_needs_renames() only searches for renames in
> a parent without the path if the merge base still has the path, which
> covers the case when the parent renamed the path. Here the other side
> of the merge did the rename, so the merge base only has the old name.
> The parent is then told to follow G, which it doesn't have, and the
> history of F on its side is lost. Searching for renames in each parent
> that lacks the path would fix this, but that is the expensive search
> that the previous commit avoids, e.g. 148 merges hit this case for
> include/linux/rsc_table.h in linux.git.
>
> Fix the problem by also searching for renames when the merge base
> doesn't have the path, but the parent changed a file since the merge
> base that the merge no longer has: only such a file can be the old name
> of the path that still needs to be followed in the parent.
This isn't true though in the case where you copy a file though, is it?
E.g. the following test case is exactly the same as the one you add in
this commit, but instead of using a rename we use a copy:
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index a85fb78ca28..c67d9481ab5 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -297,4 +297,36 @@ test_expect_success '--follow follows an edit on the side the merge did not take
test_cmp expect actual
'
+test_expect_success 'setup merge keeping the copied path, other side edited the original' '
+ git init keep-copied &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F &&
+ git -C keep-copied add F &&
+ test_tick &&
+ git -C keep-copied commit -m "O: add F" &&
+ git -C keep-copied branch B &&
+ cp keep-copied/F keep-copied/G &&
+ git -C keep-copied add G &&
+ test_tick &&
+ git -C keep-copied commit -m "A1: copy F to G on master" &&
+
+ git -C keep-copied checkout B &&
+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F &&
+ test_tick &&
+ git -C keep-copied commit -am "B2: update F on B" &&
+
+ git -C keep-copied checkout master &&
+ test_tick &&
+ git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B
+'
+
+test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' '
+ git -C keep-copied log --follow --pretty=tformat:%s G >actual &&
+ cat >expect <<-\EOF &&
+ B2: update F on B
+ A1: copy F to G on master
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
test_done
This test passes on master, but fails with your patch series.
Patrick
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take
2026-11-06 14:14 ` Patrick Steinhardt
@ 2026-11-09 8:03 ` Miklos Vajna
0 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:03 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: gitster, szeder.dev, git
Hi Patrick,
On Fri, Nov 06, 2026 at 03:14:17PM +0100, Patrick Steinhardt <ps@pks.im> wrote:
> > Fix the problem by also searching for renames when the merge base
> > doesn't have the path, but the parent changed a file since the merge
> > base that the merge no longer has: only such a file can be the old name
> > of the path that still needs to be followed in the parent.
>
> This isn't true though in the case where you copy a file though, is it?
> E.g. the following test case is exactly the same as the one you add in
> this commit, but instead of using a rename we use a copy:
You're right, I didn't consider copying. I've added your test to 1/5 and
fixed 5/5 in v5 to handle this correctly.
The pre-filter now also takes a file into account that the merge still
has, but with the version of the other side, and the rename check now
also detects copies. This makes the linux.git case take about 42s,
instead of 37s here.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v4 0/6] log: speed up --follow following renames for non-linear history
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
` (5 preceding siblings ...)
2026-10-30 20:10 ` [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take Miklos Vajna
@ 2026-11-06 14:14 ` Patrick Steinhardt
2026-11-09 8:01 ` Miklos Vajna
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
7 siblings, 1 reply; 43+ messages in thread
From: Patrick Steinhardt @ 2026-11-06 14:14 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, git
Hi Miklos,
On Fri, Oct 30, 2026 at 09:07:00PM +0100, Miklos Vajna wrote:
> Hi Patrick,
>
> Thanks for the reviews of v3. Changes in v4:
please note that the expectation is that you answer to the review
comments directly instead of only answering reviews via the cover letter
of the next version. We want there to be discussions, and reviewers
should not have to figure out by themselves whether you did or did not
address specific concerns.
> - Patch 6/6 fixes the case you found, where the speedup lost history:
> one side of the merge renamed the path, and the other side, whose
> version the merge dropped, edited the old name. It uses your test.
> I kept it separate from the speedup patch, which is complicated
> enough already. With the extra check, the linux.git case still takes
> about 37s.
Hmm, okay. But isn't it the case that your speedup patch introduces a
regression where we now don't follow history in some specific cases
anymore, and that regression was only introduced in the preceding
commit? So it's a bit curious to have this be an extra commit.
> - I also noticed that the simplify patch lost history with
> --diff-merges=first-parent, which diffs merges only against their
> first parent, but still walks all parents. Fixed that and added a
> test for it to 1/6.
> Interdiff against v3:
Instead of an interdiff, could you please send a range-diff next time?
That makes it was easier to see which specific commits I need to have a
look at again, and it also shows diffs to commit messages.
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v4 0/6] log: speed up --follow following renames for non-linear history
2026-11-06 14:14 ` [PATCH v4 0/6] log: speed up --follow following renames for non-linear history Patrick Steinhardt
@ 2026-11-09 8:01 ` Miklos Vajna
0 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:01 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: gitster, szeder.dev, git
Hi Patrick,
On Fri, Nov 06, 2026 at 03:14:03PM +0100, Patrick Steinhardt <ps@pks.im> wrote:
> please note that the expectation is that you answer to the review
> comments directly instead of only answering reviews via the cover letter
> of the next version. We want there to be discussions, and reviewers
> should not have to figure out by themselves whether you did or did not
> address specific concerns.
I see, I'll stick to that way from now on.
> Hmm, okay. But isn't it the case that your speedup patch introduces a
> regression where we now don't follow history in some specific cases
> anymore, and that regression was only introduced in the preceding
> commit? So it's a bit curious to have this be an extra commit.
I've squashed patch 6 into patch 5 in v5. This way 5/5 in v5 is a bit
larger, but no patch in the series fixes a regression from the same
series.
> > Interdiff against v3:
>
> Instead of an interdiff, could you please send a range-diff next time?
> That makes it was easier to see which specific commits I need to have a
> look at again, and it also shows diffs to commit messages.
I did it that way in v5.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
` (6 preceding siblings ...)
2026-11-06 14:14 ` [PATCH v4 0/6] log: speed up --follow following renames for non-linear history Patrick Steinhardt
@ 2026-11-09 8:03 ` Miklos Vajna
2026-11-09 8:04 ` [PATCH v5 1/5] t4219: test more --follow cases across merges Miklos Vajna
` (5 more replies)
7 siblings, 6 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:03 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Hi Patrick,
Thanks for the reviews of v4. Changes in v5:
- The fix of the old 6/6 is now squashed into the speedup patch (5/5),
so no commit in the series loses history, and its test is moved to
1/5. While at it, I simplified 5/5: instead of two passes of diffs
against the parents, it first finds a parent where the path is
unchanged, and then only diffs against the other parents if they
still need renames to be followed. Because of the squash, the
range-diff pairs the new 5/5 with the old 6/6.
- 5/5 now also follows renames when a parent has no merge base with
the parent where the path is unchanged, e.g. a subtree merge of
unrelated histories. Added a test for this to 1/5.
- 5/5 now also follows copies, not only renames, when the merge dropped
the edit of the copy source. Added your test for this to 1/5. With
this, the linux.git case takes about 42s, instead of 37s.
Thanks,
Miklos
Miklos Vajna (5):
t4219: test more --follow cases across merges
log: extract showing the remerge/combined diff of a merge commit
log: fix -m --follow losing the history of later parents
log: simplify --follow following renames for non-linear history
log: speed up --follow following renames for non-linear history
log-tree.c | 258 +++++++++++++++++++++++++++++-------
t/t4219-log-follow-merge.sh | 230 ++++++++++++++++++++++++++++++++
2 files changed, 438 insertions(+), 50 deletions(-)
Range-diff against v4:
1: aabf5652c9 ! 1: 5512aa682f t4219: test more --follow cases across merges
@@ Commit message
t4219: test more --follow cases across merges
A later commit in this series will speed up 'git log --follow' for
- merge commits. Add tests for cases which already work, but had no
+ merge commits. Add tests for cases which already work, but have no
test coverage, so it's easy to break them while optimizing:
+ - a subtree merge adding the history of a file that was copied before
- --follow through a fork with renames on both sides, with -c, --cc
and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit
@@ Commit message
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their
first parent, but still walks all parents
+ - a merge keeping the renamed path, while the other side edited the old
+ one
+ - a merge keeping the copied path, while the other side edited the
+ original
+ Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
## t/t4219-log-follow-merge.sh ##
+@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow finds the pre-merge commit through a subtree merge
+ test_cmp expect actual
+ '
+
++test_expect_success 'setup subtree merge adding the history of a copied file' '
++ git init graft &&
++ echo outer >graft/outer.txt &&
++ git -C graft add outer.txt &&
++ test_tick &&
++ git -C graft commit -m "outer init" &&
++ mkdir graft/inner &&
++ cp inner/inner.txt graft/inner/ &&
++ git -C graft add inner &&
++ test_tick &&
++ git -C graft commit -m "copy inner.txt to inner/" &&
++
++ git -C graft fetch ../inner master &&
++ test_tick &&
++ git -C graft merge -s ours --allow-unrelated-histories \
++ -m "Merge the history of inner/" FETCH_HEAD
++'
++
++test_expect_success '--follow finds the history of a copied file through a subtree merge' '
++ git -C graft log --follow --pretty=tformat:%s inner/inner.txt >actual &&
++ cat >expect <<-\EOF &&
++ copy inner.txt to inner/
++ inner init
++ EOF
++ test_cmp expect actual
++'
++
+ test_expect_success 'setup merge of two branches that both renamed a file to README' '
+ git init foo &&
+ mkdir foo/foo &&
@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow follows renames through a fork in a single history
test_cmp expect actual.sorted
'
@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow follows renames throu
+ EOF
+ test_cmp expect actual
+'
++
++test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
++ git init keep-renamed &&
++ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
++ git -C keep-renamed add F &&
++ test_tick &&
++ git -C keep-renamed commit -m "O: add F" &&
++ git -C keep-renamed branch B &&
++ git -C keep-renamed mv F G &&
++ test_tick &&
++ git -C keep-renamed commit -m "A1: rename F to G on master" &&
++
++ git -C keep-renamed checkout B &&
++ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
++ test_tick &&
++ git -C keep-renamed commit -am "B2: update F on B" &&
++
++ git -C keep-renamed checkout master &&
++ test_tick &&
++ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
++'
++
++test_expect_success '--follow follows an edit on the side the merge did not take' '
++ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
++ cat >expect <<-\EOF &&
++ B2: update F on B
++ A1: rename F to G on master
++ O: add F
++ EOF
++ test_cmp expect actual
++'
++
++test_expect_success 'setup merge keeping the copied path, other side edited the original' '
++ git init keep-copied &&
++ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F &&
++ git -C keep-copied add F &&
++ test_tick &&
++ git -C keep-copied commit -m "O: add F" &&
++ git -C keep-copied branch B &&
++ cp keep-copied/F keep-copied/G &&
++ git -C keep-copied add G &&
++ test_tick &&
++ git -C keep-copied commit -m "A1: copy F to G on master" &&
++
++ git -C keep-copied checkout B &&
++ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F &&
++ test_tick &&
++ git -C keep-copied commit -am "B2: update F on B" &&
++
++ git -C keep-copied checkout master &&
++ test_tick &&
++ git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B
++'
++
++test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' '
++ git -C keep-copied log --follow --pretty=tformat:%s G >actual &&
++ cat >expect <<-\EOF &&
++ B2: update F on B
++ A1: copy F to G on master
++ O: add F
++ EOF
++ test_cmp expect actual
++'
+
test_done
2: b172890bf4 = 2: f36129714b log: extract showing the remerge/combined diff of a merge commit
3: 6586e8f004 = 3: 85b6067fbd log: fix -m --follow losing the history of later parents
4: b903152bc4 ! 4: 5ba93e49cf log: simplify --follow following renames for non-linear history
@@ Commit message
log: simplify --follow following renames for non-linear history
Code added in 304812ed33 (log: improve --follow following renames for
- non-linear history, 2026-06-22) implemented the feature by introducing a
+ non-linear history, 2026-06-22) implements the feature by introducing a
new propagate_follow_pathspec_to_parent() function which adds a new call
to diff_tree_oid(). Also it introduces a new loop to record what path
each parent of a merge commit should use when following a path.
- This was working, but it caused a performance regression: e.g. 'git log
- --stat --follow -- include/linux/rsc_table.h' in linux.git went from 28s
- to 6m43s. The new code diffs each merge against each of its parents with
- rename detection, which is expensive when a parent doesn't have the
- followed path yet. Fixing this would require adding even more custom
- code, instead of just extending the existing code in log_tree_diff(). So
- first simplify the code here, and fix the performance regression in a
- follow-up commit.
+ This works, but it causes a performance regression: e.g. 'git log
+ --stat --follow -- include/linux/rsc_table.h' in linux.git takes 6m43s,
+ while it took 28s before that commit. The new code diffs each merge
+ against each of its parents with rename detection, which is expensive
+ when a parent doesn't have the followed path yet. Fixing this would
+ require adding even more custom code, instead of just extending the
+ existing code in log_tree_diff(). So first simplify the code here, and
+ fix the performance regression in a follow-up commit.
Remove propagate_follow_pathspec_to_parent() and the loop calling it
for each parent. Instead, record the path to follow for each parent in
- the parent loop of log_tree_diff(), right after diff_tree_oid() updated
+ the parent loop of log_tree_diff(), right after diff_tree_oid() updates
the pathspec in case of a rename. Merges are not diffed by default, so
with --follow, run the same loop for them as well, but without showing
anything. With -c, --cc or --remerge-diff, show the merge diff after the
5: 072d90b865 < -: ---------- log: speed up --follow following renames for non-linear history
6: 7be8cfb3ba ! 5: ddcac2115e log: fix --follow following an edit on a side the merge did not take
@@ Metadata
Author: Miklos Vajna <vmiklos@collabora.com>
## Commit message ##
- log: fix --follow following an edit on a side the merge did not take
+ log: speed up --follow following renames for non-linear history
- Take a history where F is renamed to G on master, F is edited on a side
- branch, and the side branch is merged with '-s ours', so the merge keeps
- G and drops F. Here 'git log --follow G' doesn't show the edit of F on
- the side branch, while it did before the previous commit.
+ Test the performance of 'git log --stat --follow --
+ include/linux/rsc_table.h' on linux.git: since commit 304812ed33 (log:
+ improve --follow following renames for non-linear history, 2026-06-22),
+ it takes 6m43s, while it took about 28 seconds before.
- The problem is that follow_needs_renames() only searches for renames in
- a parent without the path if the merge base still has the path, which
- covers the case when the parent renamed the path. Here the other side
- of the merge did the rename, so the merge base only has the old name.
- The parent is then told to follow G, which it doesn't have, and the
- history of F on its side is lost. Searching for renames in each parent
- that lacks the path would fix this, but that is the expensive search
- that the previous commit avoids, e.g. 148 merges hit this case for
- include/linux/rsc_table.h in linux.git.
+ What happens is that log_tree_diff() diffs each merge against all of its
+ parents with rename following. diff_tree_oid() only does the expensive
+ rename detection when the path is added relative to a parent, but that
+ is common for merges, e.g. when a branch forked before the path was
+ created merges mainline, and this slows down 'git log --follow'.
- Fix the problem by also searching for renames when the merge base
- doesn't have the path, but the parent changed a file since the merge
- base that the merge no longer has: only such a file can be the old name
- of the path that still needs to be followed in the parent. This check
- is cheap, since it only diffs the changes of the side branch, and it
- rules out 146 of the 148 merges in linux.git. Then do a plain rename
- detection from the parent to the merge, which only looks at the files
- deleted by the merge, not at all files of the parent like --follow does.
+ Fix the problem by first diffing the merge against its parents without
+ rename following, to find a parent where the path is unchanged. If there
+ is such a parent, then skip the diff against the other parents and just
+ follow the same path there. The exception is a parent that doesn't have
+ the path, but still needs renames to be followed:
+ - The merge base has the path, i.e. that side renamed or removed it, or
+ there is no merge base, e.g. a subtree merge of unrelated histories.
+ - The merge base doesn't have the path, but the parent changed a file
+ since then, and the merge dropped that change. A copy detection also
+ finds that file as the source of the path, i.e. the other side renamed
+ or copied the path from that file.
+ If the path changed relative to all parents, then diff against them with
+ rename following, as before. Given that this way a parent may be asked
+ to follow a path that it doesn't have, also adjust
+ remember_follow_pathspec(): if children disagree on what should be the
+ path for a commit, then prefer the path that exists in that commit.
- Reported-by: Patrick Steinhardt <ps@pks.im>
+ Now the above use case takes 42s, somewhat close to the original cost.
+
+ Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
+ Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
## log-tree.c ##
+@@
+ #include "range-diff.h"
+ #include "strmap.h"
+ #include "tree.h"
++#include "tree-walk.h"
+ #include "wildmatch.h"
+ #include "write-or-die.h"
+ #include "pager.h"
@@ log-tree.c: static void set_pathspec_to_single_path(struct pathspec *ps, const char *path)
PATHSPEC_LITERAL_PATH, "", paths);
}
--static int commit_has_path(struct rev_info *opt, struct commit *c,
-- const char *path)
+static int tree_has_path(struct rev_info *opt, struct object_id *tree,
+ const char *path)
- {
- struct object_id oid;
- unsigned short mode;
-
++{
++ struct object_id oid;
++ unsigned short mode;
++
+ return !get_tree_entry(opt->diffopt.repo, tree, path, &oid, &mode);
+}
+
+static int commit_has_path(struct rev_info *opt, struct commit *c,
+ const char *path)
+{
- parse_commit_or_die(c);
-- return !get_tree_entry(opt->diffopt.repo, get_commit_tree_oid(c), path,
-- &oid, &mode);
++ parse_commit_or_die(c);
+ return tree_has_path(opt, get_commit_tree_oid(c), path);
- }
-
++}
++
static void remember_follow_pathspec(struct rev_info *opt,
+ struct commit *c, const char *path)
+ {
+@@ log-tree.c: static void remember_follow_pathspec(struct rev_info *opt,
+ init_follow_pathspec_slab(opt->follow_pathspec_slab);
+ }
+ slot = follow_pathspec_slab_at(opt->follow_pathspec_slab, c);
+- if (*slot && !strcmp(*slot, path))
+- return;
++ /* If children of 'c' disagree on the path, prefer the one 'c' has. */
++ if (*slot) {
++ if (!strcmp(*slot, path) || commit_has_path(opt, c, *slot) ||
++ !commit_has_path(opt, c, path))
++ return;
++ }
+ free(*slot);
+ *slot = xstrdup(path);
+ }
@@ log-tree.c: void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
++/* Find a parent where 'path' is the same as in the merge. */
++static struct commit *follow_same_parent(struct rev_info *opt,
++ struct commit_list *parents,
++ struct object_id *merge_tree)
++{
++ struct commit *same = NULL;
++
++ opt->diffopt.flags.follow_renames = 0;
++ for (; parents && !same; parents = parents->next) {
++ parse_commit_or_die(parents->item);
++ diff_tree_oid(get_commit_tree_oid(parents->item), merge_tree,
++ "", &opt->diffopt);
++ if (!diff_queued_diff.nr)
++ same = parents->item;
++ diff_queue_clear(&diff_queued_diff);
++ }
++ opt->diffopt.flags.follow_renames = 1;
++ return same;
++}
++
+/*
-+ * Did 'parent' change a file since 'base' that the merge (with tree 'oid') no
-+ * longer has? Only such a file can be the source of a rename on the other side
-+ * of the merge that still needs to be followed in 'parent'.
++ * Did 'parent' change a file since 'base', but the merge tree dropped that
++ * change? I.e. the merge doesn't have the file or it's the same as in 'same'.
+ */
+static int follow_parent_changed_dropped_file(struct rev_info *opt,
+ struct commit *base,
+ struct commit *parent,
-+ struct object_id *oid)
++ struct commit *same,
++ struct object_id *merge_tree)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
@@ log-tree.c: void release_follow_pathspec_slab(struct rev_info *opt)
+ "", &diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
++ struct object_id merge_oid, same_oid;
++ unsigned short mode;
+
-+ ret = DIFF_FILE_VALID(p->two) &&
-+ !tree_has_path(opt, oid, p->two->path);
++ if (!DIFF_FILE_VALID(p->two))
++ continue;
++ if (get_tree_entry(opt->diffopt.repo, merge_tree, p->two->path,
++ &merge_oid, &mode))
++ ret = 1;
++ else
++ ret = !oideq(&merge_oid, &p->two->oid) &&
++ !get_tree_entry(opt->diffopt.repo,
++ get_commit_tree_oid(same),
++ p->two->path, &same_oid, &mode) &&
++ oideq(&merge_oid, &same_oid);
+ }
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
-+/*
-+ * Is 'path' in the merge (with tree 'oid') renamed from a file of 'parent'?
-+ * Only look at the files that the merge deleted, not at all files of 'parent'
-+ * like --follow does.
-+ */
++/* Is 'path' in the merge tree renamed or copied from a file of 'parent'? */
+static int follow_has_rename_source(struct rev_info *opt,
+ struct commit *parent,
-+ struct object_id *oid, const char *path)
++ struct object_id *merge_tree,
++ const char *path)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
+
+ repo_diff_setup(opt->diffopt.repo, &diff_opts);
+ diff_opts.flags.recursive = 1;
-+ diff_opts.detect_rename = DIFF_DETECT_RENAME;
++ diff_opts.detect_rename = DIFF_DETECT_COPY;
+ diff_opts.rename_score = opt->diffopt.rename_score;
+ diff_opts.single_follow = path;
+ diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&diff_opts);
-+ diff_tree_oid(get_commit_tree_oid(parent), oid, "", &diff_opts);
++ diff_tree_oid(get_commit_tree_oid(parent), merge_tree, "", &diff_opts);
+ diffcore_std(&diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+
-+ ret = p->status == DIFF_STATUS_RENAMED &&
++ ret = (p->status == DIFF_STATUS_RENAMED ||
++ p->status == DIFF_STATUS_COPIED) &&
+ !strcmp(p->two->path, path);
+ }
+ diff_queue_clear(&diff_queued_diff);
@@ log-tree.c: void release_follow_pathspec_slab(struct rev_info *opt)
+ return ret;
+}
+
- /*
- * When the path in a merge is the same as in 'same', does the path still need
- * renames to be followed in 'parent'? Only if the merge has the path, 'parent'
-- * doesn't have it, but its fork point with 'same' had it, e.g. 'parent'
-- * renamed the path.
-+ * doesn't have it, and either:
-+ *
-+ * - its fork point with 'same' had it, e.g. 'parent' renamed the path, or
-+ * - 'same' renamed the path from a file that 'parent' changed.
- */
- static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
-- struct commit *same, const char *path)
-+ struct commit *same, struct object_id *oid,
-+ const char *path)
- {
- struct commit_list *bases = NULL, *base;
-- int ret = 0;
++/*
++ * When the path in a merge tree is the same as in 'same', does the path still
++ * need renames to be followed in 'parent'?
++ */
++static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
++ struct commit *same,
++ struct object_id *merge_tree, const char *path)
++{
++ struct commit_list *bases = NULL, *base;
+ int ret = 0, changed_dropped = 0;
-
- if (commit_has_path(opt, parent, path) ||
- !commit_has_path(opt, same, path))
- return 0;
- if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
- exit(128);
-- for (base = bases; base && !ret; base = base->next)
++
++ if (parent == same || commit_has_path(opt, parent, path) ||
++ !commit_has_path(opt, same, path))
++ return 0;
++ if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
++ exit(128);
++ /* Unrelated histories, e.g. a subtree merge */
++ ret = !bases;
+ for (base = bases; base && !ret; base = base->next) {
- ret = commit_has_path(opt, base->item, path);
++ ret = commit_has_path(opt, base->item, path);
+ if (!ret && !changed_dropped)
+ changed_dropped = follow_parent_changed_dropped_file(
-+ opt, base->item, parent, oid);
++ opt, base->item, parent, same, merge_tree);
+ }
- commit_list_free(bases);
++ commit_list_free(bases);
+ if (!ret && changed_dropped)
-+ ret = follow_has_rename_source(opt, parent, oid, path);
- return ret;
- }
++ ret = follow_has_rename_source(opt, parent, merge_tree, path);
++ return ret;
++}
++
+ /* Show the remerge or combined diff of a merge commit */
+ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
+ struct commit_list *parents,
+@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
+ int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
+ int follow_only = 0;
+ char *path = NULL;
++ struct commit *same = NULL;
+ if (!all_need_diff && !opt->merges_need_diff)
+ return 0;
@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
- parents = parents->next) {
- if (parents->item != parent &&
- follow_needs_renames(opt, parents->item,
-- parent, path))
-+ parent, oid, path))
- tail = &commit_list_insert(parents->item,
- tail)->next;
- else
-
- ## t/t4219-log-follow-merge.sh ##
-@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow with --diff-merges=first-parent follows all parent
- test_cmp expect actual
- '
+ if (opt->diffopt.flags.follow_renames)
+ path = xstrdup_or_null(
+ pathspec_single_path(&opt->diffopt.pathspec));
++ if (path && is_merge && (follow_only || opt->first_parent_merges))
++ same = follow_same_parent(opt, parents, oid);
-+test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
-+ git init keep-renamed &&
-+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
-+ git -C keep-renamed add F &&
-+ test_tick &&
-+ git -C keep-renamed commit -m "O: add F" &&
-+ git -C keep-renamed branch B &&
-+ git -C keep-renamed mv F G &&
-+ test_tick &&
-+ git -C keep-renamed commit -m "A1: rename F to G on master" &&
-+
-+ git -C keep-renamed checkout B &&
-+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
-+ test_tick &&
-+ git -C keep-renamed commit -am "B2: update F on B" &&
-+
-+ git -C keep-renamed checkout master &&
-+ test_tick &&
-+ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
-+'
-+
-+test_expect_success '--follow follows an edit on the side the merge did not take' '
-+ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
-+ cat >expect <<-\EOF &&
-+ B2: update F on B
-+ A1: rename F to G on master
-+ O: add F
-+ EOF
-+ test_cmp expect actual
-+'
-+
- test_done
+ showed_log = 0;
+ all_parents = parents;
+@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
+ struct commit *parent = parents->item;
+
+ parse_commit_or_die(parent);
+- diff_tree_oid(get_commit_tree_oid(parent),
+- oid, "", &opt->diffopt);
++ /*
++ * Same as in a parent: follow the path as-is, unless renames
++ * still need to be followed in this parent.
++ */
++ if (!follow_only || !same ||
++ follow_needs_renames(opt, parent, same, oid, path))
++ diff_tree_oid(get_commit_tree_oid(parent),
++ oid, "", &opt->diffopt);
+ if (follow_only) {
+ diff_queue_clear(&diff_queued_diff);
+ opt->diffopt.found_follow = 0;
--
2.51.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v5 1/5] t4219: test more --follow cases across merges
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
@ 2026-11-09 8:04 ` Miklos Vajna
2026-11-10 23:06 ` Justin Tobler
2026-11-09 8:05 ` [PATCH v5 2/5] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
` (4 subsequent siblings)
5 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:04 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
A later commit in this series will speed up 'git log --follow' for
merge commits. Add tests for cases which already work, but have no
test coverage, so it's easy to break them while optimizing:
- a subtree merge adding the history of a file that was copied before
- --follow through a fork with renames on both sides, with -c, --cc
and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit
orders, since the result could depend on the walk order
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their
first parent, but still walks all parents
- a merge keeping the renamed path, while the other side edited the old
one
- a merge keeping the copied path, while the other side edited the
original
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
t/t4219-log-follow-merge.sh | 214 ++++++++++++++++++++++++++++++++++++
1 file changed, 214 insertions(+)
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index e370f82955..806ccc24bd 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -31,6 +31,33 @@ test_expect_success '--follow finds the pre-merge commit through a subtree merge
test_cmp expect actual
'
+test_expect_success 'setup subtree merge adding the history of a copied file' '
+ git init graft &&
+ echo outer >graft/outer.txt &&
+ git -C graft add outer.txt &&
+ test_tick &&
+ git -C graft commit -m "outer init" &&
+ mkdir graft/inner &&
+ cp inner/inner.txt graft/inner/ &&
+ git -C graft add inner &&
+ test_tick &&
+ git -C graft commit -m "copy inner.txt to inner/" &&
+
+ git -C graft fetch ../inner master &&
+ test_tick &&
+ git -C graft merge -s ours --allow-unrelated-histories \
+ -m "Merge the history of inner/" FETCH_HEAD
+'
+
+test_expect_success '--follow finds the history of a copied file through a subtree merge' '
+ git -C graft log --follow --pretty=tformat:%s inner/inner.txt >actual &&
+ cat >expect <<-\EOF &&
+ copy inner.txt to inner/
+ inner init
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'setup merge of two branches that both renamed a file to README' '
git init foo &&
mkdir foo/foo &&
@@ -126,4 +153,191 @@ test_expect_success '--follow follows renames through a fork in a single history
test_cmp expect actual.sorted
'
+for opt in -c --cc --remerge-diff
+do
+ test_expect_success "--follow follows renames through a fork with $opt" '
+ git -C diamond log --follow $opt --pretty=tformat:%s path >log &&
+ grep "^[A-Z]: " log | sort >actual &&
+ cat >expect <<-\EOF &&
+ A: add path0
+ B: modify path0 on upper
+ C: modify path0 on lower
+ M: merge with rename to path
+ X: rename path0 to path1
+ Y: rename path0 to path2
+ Z: modify path
+ EOF
+ test_cmp expect actual
+ '
+done
+
+# Build a merge that takes the renamed path from the second parent, while the
+# first parent only has the old path. $2 decides which side is committed first,
+# which decides the walk order.
+setup_rename_on_second_parent () {
+ git init $1 &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old &&
+ git -C $1 add old &&
+ test_tick &&
+ git -C $1 commit -m "A: add old" &&
+ printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old &&
+ test_tick &&
+ git -C $1 commit -am "B: modify old" &&
+ git -C $1 branch topic &&
+
+ if test "$2" = topic-first
+ then
+ git -C $1 checkout topic &&
+ git -C $1 mv old new &&
+ test_tick &&
+ git -C $1 commit -m "R: rename old to new" &&
+ git -C $1 checkout master
+ fi &&
+
+ echo other >$1/other &&
+ git -C $1 add other &&
+ test_tick &&
+ git -C $1 commit -m "M: unrelated change on master" &&
+
+ if test "$2" = master-first
+ then
+ git -C $1 checkout topic &&
+ git -C $1 mv old new &&
+ test_tick &&
+ git -C $1 commit -m "R: rename old to new" &&
+ git -C $1 checkout master
+ fi &&
+
+ test_tick &&
+ git -C $1 merge --no-ff -m "merge topic" topic
+}
+
+test_expect_success 'setup merge taking a rename from the second parent' '
+ setup_rename_on_second_parent rename-master-first master-first &&
+ setup_rename_on_second_parent rename-topic-first topic-first
+'
+
+for order in master-first topic-first
+do
+ test_expect_success "--follow follows a rename from the second parent ($order)" '
+ git -C rename-$order log --follow --pretty=tformat:%s new >actual &&
+ cat >expect <<-\EOF &&
+ R: rename old to new
+ B: modify old
+ A: add old
+ EOF
+ test_cmp expect actual
+ '
+done
+
+test_expect_success 'setup merge keeping the path, other side renamed it' '
+ git init keep &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F &&
+ git -C keep add F &&
+ test_tick &&
+ git -C keep commit -m "O: add F" &&
+ git -C keep branch B &&
+ printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F &&
+ test_tick &&
+ git -C keep commit -am "A1: update F in place" &&
+
+ git -C keep checkout B &&
+ git -C keep mv F G &&
+ test_tick &&
+ git -C keep commit -m "B1: rename F to G" &&
+ printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G &&
+ test_tick &&
+ git -C keep commit -am "B2: update G" &&
+
+ git -C keep checkout master &&
+ test_tick &&
+ git -C keep merge -s ours -m "M: merge B, keep F, drop G" B
+'
+
+test_expect_success '--follow follows a rename on the side the merge did not take' '
+ git -C keep log --follow --pretty=tformat:%s F >actual &&
+ cat >expect <<-\EOF &&
+ B2: update G
+ B1: rename F to G
+ A1: update F in place
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--follow with --diff-merges=first-parent follows all parents' '
+ git -C keep log --follow --diff-merges=first-parent --no-patch \
+ --pretty=tformat:%s F >actual &&
+ cat >expect <<-\EOF &&
+ B2: update G
+ B1: rename F to G
+ A1: update F in place
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
+ git init keep-renamed &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
+ git -C keep-renamed add F &&
+ test_tick &&
+ git -C keep-renamed commit -m "O: add F" &&
+ git -C keep-renamed branch B &&
+ git -C keep-renamed mv F G &&
+ test_tick &&
+ git -C keep-renamed commit -m "A1: rename F to G on master" &&
+
+ git -C keep-renamed checkout B &&
+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
+ test_tick &&
+ git -C keep-renamed commit -am "B2: update F on B" &&
+
+ git -C keep-renamed checkout master &&
+ test_tick &&
+ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
+'
+
+test_expect_success '--follow follows an edit on the side the merge did not take' '
+ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
+ cat >expect <<-\EOF &&
+ B2: update F on B
+ A1: rename F to G on master
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'setup merge keeping the copied path, other side edited the original' '
+ git init keep-copied &&
+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F &&
+ git -C keep-copied add F &&
+ test_tick &&
+ git -C keep-copied commit -m "O: add F" &&
+ git -C keep-copied branch B &&
+ cp keep-copied/F keep-copied/G &&
+ git -C keep-copied add G &&
+ test_tick &&
+ git -C keep-copied commit -m "A1: copy F to G on master" &&
+
+ git -C keep-copied checkout B &&
+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F &&
+ test_tick &&
+ git -C keep-copied commit -am "B2: update F on B" &&
+
+ git -C keep-copied checkout master &&
+ test_tick &&
+ git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B
+'
+
+test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' '
+ git -C keep-copied log --follow --pretty=tformat:%s G >actual &&
+ cat >expect <<-\EOF &&
+ B2: update F on B
+ A1: copy F to G on master
+ O: add F
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.51.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v5 1/5] t4219: test more --follow cases across merges
2026-11-09 8:04 ` [PATCH v5 1/5] t4219: test more --follow cases across merges Miklos Vajna
@ 2026-11-10 23:06 ` Justin Tobler
0 siblings, 0 replies; 43+ messages in thread
From: Justin Tobler @ 2026-11-10 23:06 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, ps, git
On 26/11/09 09:04AM, Miklos Vajna wrote:
>A later commit in this series will speed up 'git log --follow' for
>merge commits. Add tests for cases which already work, but have no
>test coverage, so it's easy to break them while optimizing:
>
>- a subtree merge adding the history of a file that was copied before
>- --follow through a fork with renames on both sides, with -c, --cc
> and --remerge-diff
>- a --no-ff merge taking a rename from the second parent, in both commit
> orders, since the result could depend on the walk order
>- a merge keeping the path, while the other side renamed it
>- --diff-merges=first-parent, which diffs merges only against their
> first parent, but still walks all parents
>- a merge keeping the renamed path, while the other side edited the old
> one
>- a merge keeping the copied path, while the other side edited the
> original
This explains what tests we are adding, but it doesn't really provide
readers with much context as do why they are the expected behaviors.
>
>Helped-by: Patrick Steinhardt <ps@pks.im>
>Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
>---
> t/t4219-log-follow-merge.sh | 214 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 214 insertions(+)
>
>diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
>index e370f82955..806ccc24bd 100755
>--- a/t/t4219-log-follow-merge.sh
>+++ b/t/t4219-log-follow-merge.sh
>@@ -31,6 +31,33 @@ test_expect_success '--follow finds the pre-merge commit through a subtree merge
> test_cmp expect actual
> '
>
>+test_expect_success 'setup subtree merge adding the history of a copied file' '
>+ git init graft &&
>+ echo outer >graft/outer.txt &&
>+ git -C graft add outer.txt &&
>+ test_tick &&
>+ git -C graft commit -m "outer init" &&
>+ mkdir graft/inner &&
>+ cp inner/inner.txt graft/inner/ &&
>+ git -C graft add inner &&
>+ test_tick &&
>+ git -C graft commit -m "copy inner.txt to inner/" &&
>+
>+ git -C graft fetch ../inner master &&
>+ test_tick &&
>+ git -C graft merge -s ours --allow-unrelated-histories \
>+ -m "Merge the history of inner/" FETCH_HEAD
>+'
Ok IIUC, this test is setting up a repository has manually copied and
committed from another repository, but then later merges the unrelated
history. It took me a little bit to parse what exactly was going on
here. Maybe some comments would be a bit helpful here?
>+
>+test_expect_success '--follow finds the history of a copied file through a subtree merge' '
>+ git -C graft log --follow --pretty=tformat:%s inner/inner.txt >actual &&
>+ cat >expect <<-\EOF &&
>+ copy inner.txt to inner/
>+ inner init
>+ EOF
>+ test_cmp expect actual
>+'
Ok and here we validate that we follow the file into the now merged
unrelated history. I assume that prior to 304812ed33de (log: improve
--follow following renames for non-linear history, 2026-06-22), this
would not have passed? If so, it might be helpful to more directly
mention this in the commit message?
>+
> test_expect_success 'setup merge of two branches that both renamed a file to README' '
> git init foo &&
> mkdir foo/foo &&
>@@ -126,4 +153,191 @@ test_expect_success '--follow follows renames through a fork in a single history
> test_cmp expect actual.sorted
> '
>
>+for opt in -c --cc --remerge-diff
>+do
>+ test_expect_success "--follow follows renames through a fork with $opt" '
>+ git -C diamond log --follow $opt --pretty=tformat:%s path >log &&
>+ grep "^[A-Z]: " log | sort >actual &&
>+ cat >expect <<-\EOF &&
>+ A: add path0
>+ B: modify path0 on upper
>+ C: modify path0 on lower
>+ M: merge with rename to path
>+ X: rename path0 to path1
>+ Y: rename path0 to path2
>+ Z: modify path
>+ EOF
>+ test_cmp expect actual
>+ '
>+done
I assume here we are validating that the merge is printed?
>+
>+# Build a merge that takes the renamed path from the second parent, while the
>+# first parent only has the old path. $2 decides which side is committed first,
>+# which decides the walk order.
>+setup_rename_on_second_parent () {
>+ git init $1 &&
>+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old &&
>+ git -C $1 add old &&
>+ test_tick &&
>+ git -C $1 commit -m "A: add old" &&
>+ printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old &&
>+ test_tick &&
>+ git -C $1 commit -am "B: modify old" &&
>+ git -C $1 branch topic &&
>+
>+ if test "$2" = topic-first
>+ then
>+ git -C $1 checkout topic &&
>+ git -C $1 mv old new &&
>+ test_tick &&
>+ git -C $1 commit -m "R: rename old to new" &&
>+ git -C $1 checkout master
>+ fi &&
>+
>+ echo other >$1/other &&
>+ git -C $1 add other &&
>+ test_tick &&
>+ git -C $1 commit -m "M: unrelated change on master" &&
>+
>+ if test "$2" = master-first
>+ then
>+ git -C $1 checkout topic &&
>+ git -C $1 mv old new &&
>+ test_tick &&
>+ git -C $1 commit -m "R: rename old to new" &&
>+ git -C $1 checkout master
>+ fi &&
>+
>+ test_tick &&
>+ git -C $1 merge --no-ff -m "merge topic" topic
>+}
Ok, because the walk order is based on commiter date, we have a helper
function here that can create various test setups to validate we can
follow a file through the history.
>+
>+test_expect_success 'setup merge taking a rename from the second parent' '
>+ setup_rename_on_second_parent rename-master-first master-first &&
>+ setup_rename_on_second_parent rename-topic-first topic-first
>+'
>+
>+for order in master-first topic-first
>+do
>+ test_expect_success "--follow follows a rename from the second parent ($order)" '
>+ git -C rename-$order log --follow --pretty=tformat:%s new >actual &&
>+ cat >expect <<-\EOF &&
>+ R: rename old to new
>+ B: modify old
>+ A: add old
>+ EOF
>+ test_cmp expect actual
>+ '
>+done
Ok, so we expect the results to be same the same and the file to be
found on both sides here. I realize we are crafting very specific
repository histories to validate here, but I must say the test setup
here is rather complex IMO.
>+
>+test_expect_success 'setup merge keeping the path, other side renamed it' '
>+ git init keep &&
>+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F &&
>+ git -C keep add F &&
>+ test_tick &&
>+ git -C keep commit -m "O: add F" &&
>+ git -C keep branch B &&
>+ printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F &&
>+ test_tick &&
>+ git -C keep commit -am "A1: update F in place" &&
>+
>+ git -C keep checkout B &&
>+ git -C keep mv F G &&
>+ test_tick &&
>+ git -C keep commit -m "B1: rename F to G" &&
>+ printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G &&
>+ test_tick &&
>+ git -C keep commit -am "B2: update G" &&
>+
>+ git -C keep checkout master &&
>+ test_tick &&
>+ git -C keep merge -s ours -m "M: merge B, keep F, drop G" B
>+'
Ok, here we are creating a history where a topic modifies and renames a
file, but the merge drops these changes. Some comments here might also
be helpful for readers.
>+
>+test_expect_success '--follow follows a rename on the side the merge did not take' '
>+ git -C keep log --follow --pretty=tformat:%s F >actual &&
>+ cat >expect <<-\EOF &&
>+ B2: update G
>+ B1: rename F to G
>+ A1: update F in place
>+ O: add F
>+ EOF
>+ test_cmp expect actual
>+'
We expect that the commits from branch B are still inspected even though
the merge dropped them. Makes sense.
>+
>+test_expect_success '--follow with --diff-merges=first-parent follows all parents' '
>+ git -C keep log --follow --diff-merges=first-parent --no-patch \
>+ --pretty=tformat:%s F >actual &&
>+ cat >expect <<-\EOF &&
>+ B2: update G
>+ B1: rename F to G
>+ A1: update F in place
>+ O: add F
>+ EOF
>+ test_cmp expect actual
>+'
hmmmm, is the expectation here that `--diff-merges=first-parent` doesn't
prevent us from visiting commits from the second parent?
>+
>+test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
>+ git init keep-renamed &&
>+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
>+ git -C keep-renamed add F &&
>+ test_tick &&
>+ git -C keep-renamed commit -m "O: add F" &&
>+ git -C keep-renamed branch B &&
>+ git -C keep-renamed mv F G &&
>+ test_tick &&
>+ git -C keep-renamed commit -m "A1: rename F to G on master" &&
>+
>+ git -C keep-renamed checkout B &&
>+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
>+ test_tick &&
>+ git -C keep-renamed commit -am "B2: update F on B" &&
>+
>+ git -C keep-renamed checkout master &&
>+ test_tick &&
>+ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
>+'
Here the merge keeps master's renamed G and discards branch B's edit to
F.
>+
>+test_expect_success '--follow follows an edit on the side the merge did not take' '
>+ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
>+ cat >expect <<-\EOF &&
>+ B2: update F on B
>+ A1: rename F to G on master
>+ O: add F
>+ EOF
>+ test_cmp expect actual
>+'
Commits from both sides along with the original commit are expected.
Makes sense.
>+
>+test_expect_success 'setup merge keeping the copied path, other side edited the original' '
>+ git init keep-copied &&
>+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F &&
>+ git -C keep-copied add F &&
>+ test_tick &&
>+ git -C keep-copied commit -m "O: add F" &&
>+ git -C keep-copied branch B &&
>+ cp keep-copied/F keep-copied/G &&
>+ git -C keep-copied add G &&
>+ test_tick &&
>+ git -C keep-copied commit -m "A1: copy F to G on master" &&
>+
>+ git -C keep-copied checkout B &&
>+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F &&
>+ test_tick &&
>+ git -C keep-copied commit -am "B2: update F on B" &&
>+
>+ git -C keep-copied checkout master &&
>+ test_tick &&
>+ git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B
>+'
>+
>+test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' '
>+ git -C keep-copied log --follow --pretty=tformat:%s G >actual &&
>+ cat >expect <<-\EOF &&
>+ B2: update F on B
>+ A1: copy F to G on master
>+ O: add F
>+ EOF
>+ test_cmp expect actual
>+'
>+
> test_done
Ok, these tests are similar to last set, but copied instead of renamed.
-Justin
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v5 2/5] log: extract showing the remerge/combined diff of a merge commit
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
2026-11-09 8:04 ` [PATCH v5 1/5] t4219: test more --follow cases across merges Miklos Vajna
@ 2026-11-09 8:05 ` Miklos Vajna
2026-11-09 8:05 ` [PATCH v5 3/5] log: fix -m --follow losing the history of later parents Miklos Vajna
` (3 subsequent siblings)
5 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:05 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
No functional changes intended. A later commit in this series will also
call log_tree_merge_diff() after the parent loop of log_tree_diff(), so
extract that code to a function to avoid duplicating it.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..b4c97b86f5 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1180,6 +1180,26 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
diff_free(&diff_opts);
}
+/* Show the remerge or combined diff of a merge commit */
+static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
+ struct commit_list *parents,
+ struct object_id *oid)
+{
+ int octopus = (parents->next->next != NULL);
+
+ if (opt->remerge_diff) {
+ if (octopus) {
+ show_log(opt);
+ fprintf(opt->diffopt.file,
+ "diff: warning: Skipping remerge-diff "
+ "for octopus merges.\n");
+ return 1;
+ }
+ return do_remerge_diff(opt, parents, oid);
+ }
+ return do_diff_combined(opt, commit);
+}
+
/*
* Show the diff of a commit.
*
@@ -1220,20 +1240,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
}
if (is_merge) {
- int octopus = (parents->next->next != NULL);
-
- if (opt->remerge_diff) {
- if (octopus) {
- show_log(opt);
- fprintf(opt->diffopt.file,
- "diff: warning: Skipping remerge-diff "
- "for octopus merges.\n");
- return 1;
- }
- return do_remerge_diff(opt, parents, oid);
- }
- if (opt->combine_merges)
- return do_diff_combined(opt, commit);
+ if (opt->remerge_diff || opt->combine_merges)
+ return log_tree_merge_diff(opt, commit, parents, oid);
if (opt->separate_merges) {
if (!opt->first_parent_merges) {
/* Show parent info for multiple diffs */
--
2.51.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v5 3/5] log: fix -m --follow losing the history of later parents
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
2026-11-09 8:04 ` [PATCH v5 1/5] t4219: test more --follow cases across merges Miklos Vajna
2026-11-09 8:05 ` [PATCH v5 2/5] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
@ 2026-11-09 8:05 ` Miklos Vajna
2026-11-09 8:05 ` [PATCH v5 4/5] log: simplify --follow following renames for non-linear history Miklos Vajna
` (2 subsequent siblings)
5 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:05 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
With 'git log -m --follow', a merge is diffed against each of its
parents, with rename following. Take the diamond case of t4219: the two
sides of a fork rename path0 to path1 and to path2, and the merge
renames the file to path. Here 'git log -m --follow path' doesn't show
"A: add path0" and "C: modify path0 on lower".
The problem is that the parent loop in log_tree_diff() doesn't restore
the followed path between parents. When the diff against the first
parent finds a rename, the followed path is updated, and this has two
effects. First, the second parent is diffed with the already renamed
path, not with the path of the merge. Second, the path that
propagate_follow_pathspec_to_parent() records for each parent also
starts from that updated path, so history is lost on the side of the
second parent. Without -m, merges are not diffed, so the followed path
is not updated there.
Fix the problem by saving the path of the merge before the parent loop,
and restoring it before diffing each parent and after the loop. Do this
only for merges: for a commit with a single parent, the renamed path is
what --follow needs to continue with. Add a test for this, which shows
the full history of the diamond case with -m.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 13 +++++++++++++
t/t4219-log-follow-merge.sh | 16 ++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/log-tree.c b/log-tree.c
index b4c97b86f5..befc5c8b22 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1212,6 +1212,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
struct object_id *oid;
int is_merge;
int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
+ char *path = NULL;
if (!all_need_diff && !opt->merges_need_diff)
return 0;
@@ -1251,10 +1252,18 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
return 0;
}
+ if (is_merge && opt->diffopt.flags.follow_renames)
+ path = xstrdup_or_null(
+ pathspec_single_path(&opt->diffopt.pathspec));
+
showed_log = 0;
for (;;) {
struct commit *parent = parents->item;
+ /* Diff each parent with the path of the merge */
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
parse_commit_or_die(parent);
diff_tree_oid(get_commit_tree_oid(parent),
oid, "", &opt->diffopt);
@@ -1269,6 +1278,10 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
log->parent = parents->item;
opt->loginfo = log;
}
+ /* Propagate to parents starting from the path of the merge, too */
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec, path);
+ free(path);
return showed_log;
}
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index 806ccc24bd..ac9ab85888 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -171,6 +171,22 @@ do
'
done
+test_expect_success '--follow follows renames through a fork with -m' '
+ git -C diamond log --follow -m --pretty=tformat:%s path >log &&
+ grep "^[A-Z]: " log | sort >actual &&
+ cat >expect <<-\EOF &&
+ A: add path0
+ B: modify path0 on upper
+ C: modify path0 on lower
+ M: merge with rename to path
+ M: merge with rename to path
+ X: rename path0 to path1
+ Y: rename path0 to path2
+ Z: modify path
+ EOF
+ test_cmp expect actual
+'
+
# Build a merge that takes the renamed path from the second parent, while the
# first parent only has the old path. $2 decides which side is committed first,
# which decides the walk order.
--
2.51.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v5 4/5] log: simplify --follow following renames for non-linear history
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
` (2 preceding siblings ...)
2026-11-09 8:05 ` [PATCH v5 3/5] log: fix -m --follow losing the history of later parents Miklos Vajna
@ 2026-11-09 8:05 ` Miklos Vajna
2026-11-10 16:45 ` Toon Claes
2026-11-09 8:06 ` [PATCH v5 5/5] log: speed up " Miklos Vajna
2026-11-11 0:40 ` [PATCH v5 0/5] " Justin Tobler
5 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:05 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Code added in 304812ed33 (log: improve --follow following renames for
non-linear history, 2026-06-22) implements the feature by introducing a
new propagate_follow_pathspec_to_parent() function which adds a new call
to diff_tree_oid(). Also it introduces a new loop to record what path
each parent of a merge commit should use when following a path.
This works, but it causes a performance regression: e.g. 'git log
--stat --follow -- include/linux/rsc_table.h' in linux.git takes 6m43s,
while it took 28s before that commit. The new code diffs each merge
against each of its parents with rename detection, which is expensive
when a parent doesn't have the followed path yet. Fixing this would
require adding even more custom code, instead of just extending the
existing code in log_tree_diff(). So first simplify the code here, and
fix the performance regression in a follow-up commit.
Remove propagate_follow_pathspec_to_parent() and the loop calling it
for each parent. Instead, record the path to follow for each parent in
the parent loop of log_tree_diff(), right after diff_tree_oid() updates
the pathspec in case of a rename. Merges are not diffed by default, so
with --follow, run the same loop for them as well, but without showing
anything. With -c, --cc or --remerge-diff, show the merge diff after the
loop. With --diff-merges=first-parent, keep diffing the other parents
silently, unless the walk only follows first parents. This gets rid of
the additional call to diff_tree_oid(). The rework has no positive
performance impact yet, it just simplifies the code.
The previous commit restores the path of the merge before diffing each
parent, and also after the loop, for
propagate_follow_pathspec_to_parent(). Keep restoring it between
parents, but not after the loop, since there is no later propagation
anymore.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 96 +++++++++++++++++++++++-------------------------------
1 file changed, 41 insertions(+), 55 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index befc5c8b22..0aa27fb210 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
-/* Compute a path to follow in parent, if there is one */
-static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
- struct commit *commit,
- struct commit *parent)
-{
- struct diff_options diff_opts;
- const char *path;
-
- parse_commit_or_die(parent);
- repo_diff_setup(opt->diffopt.repo, &diff_opts);
- copy_pathspec(&diff_opts.pathspec, &opt->diffopt.pathspec);
- diff_opts.flags.recursive = 1;
- diff_opts.flags.follow_renames = 1;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
- diff_setup_done(&diff_opts);
- diff_tree_oid(get_commit_tree_oid(parent),
- get_commit_tree_oid(commit),
- "", &diff_opts);
-
- path = pathspec_single_path(&diff_opts.pathspec);
- if (path)
- remember_follow_pathspec(opt, parent, path);
-
- diff_queue_clear(&diff_queued_diff);
- diff_free(&diff_opts);
-}
-
/* Show the remerge or combined diff of a merge commit */
static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
struct commit_list *parents,
@@ -1208,10 +1181,11 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
{
int showed_log;
- struct commit_list *parents;
+ struct commit_list *parents, *all_parents;
struct object_id *oid;
int is_merge;
int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
+ int follow_only = 0;
char *path = NULL;
if (!all_need_diff && !opt->merges_need_diff)
@@ -1241,46 +1215,73 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
}
if (is_merge) {
- if (opt->remerge_diff || opt->combine_merges)
+ int merge_diff = opt->remerge_diff || opt->combine_merges;
+
+ if (merge_diff && !opt->diffopt.flags.follow_renames)
return log_tree_merge_diff(opt, commit, parents, oid);
- if (opt->separate_merges) {
+ if (opt->separate_merges && !merge_diff) {
if (!opt->first_parent_merges) {
/* Show parent info for multiple diffs */
log->parent = parents->item;
}
+ } else if (opt->diffopt.flags.follow_renames) {
+ /* Diff only to find the path to follow in parents */
+ follow_only = 1;
} else
return 0;
}
- if (is_merge && opt->diffopt.flags.follow_renames)
+ if (opt->diffopt.flags.follow_renames)
path = xstrdup_or_null(
pathspec_single_path(&opt->diffopt.pathspec));
showed_log = 0;
+ all_parents = parents;
for (;;) {
struct commit *parent = parents->item;
- /* Diff each parent with the path of the merge */
- if (path)
- set_pathspec_to_single_path(&opt->diffopt.pathspec,
- path);
parse_commit_or_die(parent);
diff_tree_oid(get_commit_tree_oid(parent),
oid, "", &opt->diffopt);
- log_tree_diff_flush(opt);
+ if (follow_only) {
+ diff_queue_clear(&diff_queued_diff);
+ opt->diffopt.found_follow = 0;
+ } else {
+ log_tree_diff_flush(opt);
+ }
showed_log |= !opt->loginfo;
+ /* The diff updated the pathspec in case of a rename */
+ if (path)
+ remember_follow_pathspec(opt, parent,
+ pathspec_single_path(&opt->diffopt.pathspec));
+
/* Set up the log info for the next parent, if any.. */
parents = parents->next;
- if (!parents || opt->first_parent_merges)
+ if (!parents)
break;
+ if (opt->first_parent_merges) {
+ if (!path || opt->first_parent_only)
+ break;
+ /* Only find the path to follow in the other parents */
+ follow_only = 1;
+ }
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ if (follow_only)
+ continue;
log->parent = parents->item;
opt->loginfo = log;
}
- /* Propagate to parents starting from the path of the merge, too */
- if (path)
- set_pathspec_to_single_path(&opt->diffopt.pathspec, path);
+ if (follow_only && (opt->remerge_diff || opt->combine_merges)) {
+ /* Show the merge diff with the path of this commit */
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ showed_log = log_tree_merge_diff(opt, commit, all_parents, oid);
+ }
free(path);
return showed_log;
}
@@ -1319,21 +1320,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
if (shown)
show_diff_of_diff(opt);
-
- /* Record what path each parent of this commit should use */
- if (opt->diffopt.flags.follow_renames) {
- struct commit_list *parents = get_saved_parents(opt, commit);
- if (parents && parents->next) {
- struct commit_list *p;
- for (p = parents; p; p = p->next)
- propagate_follow_pathspec_to_parent(opt, commit,
- p->item);
- } else if (parents) {
- remember_follow_pathspec(opt, parents->item,
- pathspec_single_path(&opt->diffopt.pathspec));
- }
- }
-
opt->loginfo = NULL;
maybe_flush_or_die(opt->diffopt.file, "stdout");
opt->diffopt.no_free = no_free;
--
2.51.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v5 4/5] log: simplify --follow following renames for non-linear history
2026-11-09 8:05 ` [PATCH v5 4/5] log: simplify --follow following renames for non-linear history Miklos Vajna
@ 2026-11-10 16:45 ` Toon Claes
0 siblings, 0 replies; 43+ messages in thread
From: Toon Claes @ 2026-11-10 16:45 UTC (permalink / raw)
To: Miklos Vajna, gitster; +Cc: szeder.dev, ps, git
Miklos Vajna <vmiklos@collabora.com> writes:
> Code added in 304812ed33 (log: improve --follow following renames for
> non-linear history, 2026-06-22) implements the feature by introducing a
> new propagate_follow_pathspec_to_parent() function which adds a new call
> to diff_tree_oid(). Also it introduces a new loop to record what path
> each parent of a merge commit should use when following a path.
>
> This works, but it causes a performance regression: e.g. 'git log
> --stat --follow -- include/linux/rsc_table.h' in linux.git takes 6m43s,
> while it took 28s before that commit. The new code diffs each merge
> against each of its parents with rename detection, which is expensive
> when a parent doesn't have the followed path yet. Fixing this would
> require adding even more custom code, instead of just extending the
> existing code in log_tree_diff(). So first simplify the code here, and
> fix the performance regression in a follow-up commit.
>
> Remove propagate_follow_pathspec_to_parent() and the loop calling it
> for each parent. Instead, record the path to follow for each parent in
> the parent loop of log_tree_diff(), right after diff_tree_oid() updates
> the pathspec in case of a rename. Merges are not diffed by default, so
> with --follow, run the same loop for them as well, but without showing
> anything. With -c, --cc or --remerge-diff, show the merge diff after the
> loop. With --diff-merges=first-parent, keep diffing the other parents
> silently, unless the walk only follows first parents. This gets rid of
> the additional call to diff_tree_oid(). The rework has no positive
> performance impact yet, it just simplifies the code.
>
> The previous commit restores the path of the merge before diffing each
> parent, and also after the loop, for
> propagate_follow_pathspec_to_parent(). Keep restoring it between
> parents, but not after the loop, since there is no later propagation
> anymore.
>
> Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
> ---
> log-tree.c | 96 +++++++++++++++++++++++-------------------------------
> 1 file changed, 41 insertions(+), 55 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index befc5c8b22..0aa27fb210 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)
> FREE_AND_NULL(opt->follow_pathspec_slab);
> }
>
> -/* Compute a path to follow in parent, if there is one */
> -static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
> - struct commit *commit,
> - struct commit *parent)
> -{
> - struct diff_options diff_opts;
> - const char *path;
> -
> - parse_commit_or_die(parent);
> - repo_diff_setup(opt->diffopt.repo, &diff_opts);
> - copy_pathspec(&diff_opts.pathspec, &opt->diffopt.pathspec);
> - diff_opts.flags.recursive = 1;
> - diff_opts.flags.follow_renames = 1;
> - diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
> - diff_setup_done(&diff_opts);
> - diff_tree_oid(get_commit_tree_oid(parent),
> - get_commit_tree_oid(commit),
> - "", &diff_opts);
> -
> - path = pathspec_single_path(&diff_opts.pathspec);
> - if (path)
> - remember_follow_pathspec(opt, parent, path);
> -
> - diff_queue_clear(&diff_queued_diff);
> - diff_free(&diff_opts);
> -}
> -
> /* Show the remerge or combined diff of a merge commit */
> static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
> struct commit_list *parents,
> @@ -1208,10 +1181,11 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
> static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
> {
> int showed_log;
> - struct commit_list *parents;
> + struct commit_list *parents, *all_parents;
> struct object_id *oid;
> int is_merge;
> int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
> + int follow_only = 0;
> char *path = NULL;
>
> if (!all_need_diff && !opt->merges_need_diff)
> @@ -1241,46 +1215,73 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
> }
>
> if (is_merge) {
> - if (opt->remerge_diff || opt->combine_merges)
> + int merge_diff = opt->remerge_diff || opt->combine_merges;
> +
> + if (merge_diff && !opt->diffopt.flags.follow_renames)
> return log_tree_merge_diff(opt, commit, parents, oid);
> - if (opt->separate_merges) {
> + if (opt->separate_merges && !merge_diff) {
> if (!opt->first_parent_merges) {
> /* Show parent info for multiple diffs */
> log->parent = parents->item;
> }
> + } else if (opt->diffopt.flags.follow_renames) {
> + /* Diff only to find the path to follow in parents */
> + follow_only = 1;
> } else
> return 0;
This part of code has become pretty tough to read if you ask me. I've
tried to rewrite it:
if (is_merge) {
/* -c, --cc or --remerge-diff: show one diff for the whole merge. */
int merge_diff = opt->remerge_diff || opt->combine_merges;
/* Without --follow the parent loop is only needed for -m. */
if (!opt->diffopt.flags.follow_renames) {
/* -c, --cc or --remerge-diff: show it, no parent loop. */
if (merge_diff)
return log_tree_merge_diff(opt, commit, parents, oid);
/* No -m either, i.e. merge diffs are off: show nothing. */
if (!opt->separate_merges)
return 0;
}
/* Run the loop silently, unless -m shows a diff per parent. */
follow_only = !opt->separate_merges || merge_diff;
/* -m showing all parents: label each diff "(from <parent>)". */
if (!follow_only && !opt->first_parent_merges)
/* Label the first diff, others are labeled in parent loop. */
log->parent = parents->item;
}
The comments I've added are possibly too verbose for the Git project,
but they helped me understand what happens at each line. I hope it's
functionally the same?
(This is as far as I have gotten in the review today, but I wanted to
send this out already)
> }
>
> - if (is_merge && opt->diffopt.flags.follow_renames)
> + if (opt->diffopt.flags.follow_renames)
> path = xstrdup_or_null(
> pathspec_single_path(&opt->diffopt.pathspec));
>
> showed_log = 0;
> + all_parents = parents;
> for (;;) {
> struct commit *parent = parents->item;
>
> - /* Diff each parent with the path of the merge */
> - if (path)
> - set_pathspec_to_single_path(&opt->diffopt.pathspec,
> - path);
> parse_commit_or_die(parent);
> diff_tree_oid(get_commit_tree_oid(parent),
> oid, "", &opt->diffopt);
> - log_tree_diff_flush(opt);
> + if (follow_only) {
> + diff_queue_clear(&diff_queued_diff);
> + opt->diffopt.found_follow = 0;
> + } else {
> + log_tree_diff_flush(opt);
> + }
>
> showed_log |= !opt->loginfo;
>
> + /* The diff updated the pathspec in case of a rename */
> + if (path)
> + remember_follow_pathspec(opt, parent,
> + pathspec_single_path(&opt->diffopt.pathspec));
> +
> /* Set up the log info for the next parent, if any.. */
> parents = parents->next;
> - if (!parents || opt->first_parent_merges)
> + if (!parents)
> break;
> + if (opt->first_parent_merges) {
> + if (!path || opt->first_parent_only)
> + break;
> + /* Only find the path to follow in the other parents */
> + follow_only = 1;
> + }
> + if (path)
> + set_pathspec_to_single_path(&opt->diffopt.pathspec,
> + path);
> + if (follow_only)
> + continue;
> log->parent = parents->item;
> opt->loginfo = log;
> }
> - /* Propagate to parents starting from the path of the merge, too */
> - if (path)
> - set_pathspec_to_single_path(&opt->diffopt.pathspec, path);
> + if (follow_only && (opt->remerge_diff || opt->combine_merges)) {
> + /* Show the merge diff with the path of this commit */
> + if (path)
> + set_pathspec_to_single_path(&opt->diffopt.pathspec,
> + path);
> + showed_log = log_tree_merge_diff(opt, commit, all_parents, oid);
> + }
> free(path);
> return showed_log;
> }
> @@ -1319,21 +1320,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
> fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
> if (shown)
> show_diff_of_diff(opt);
> -
> - /* Record what path each parent of this commit should use */
> - if (opt->diffopt.flags.follow_renames) {
> - struct commit_list *parents = get_saved_parents(opt, commit);
> - if (parents && parents->next) {
> - struct commit_list *p;
> - for (p = parents; p; p = p->next)
> - propagate_follow_pathspec_to_parent(opt, commit,
> - p->item);
> - } else if (parents) {
> - remember_follow_pathspec(opt, parents->item,
> - pathspec_single_path(&opt->diffopt.pathspec));
> - }
> - }
> -
> opt->loginfo = NULL;
> maybe_flush_or_die(opt->diffopt.file, "stdout");
> opt->diffopt.no_free = no_free;
> --
> 2.51.0
>
--
Laters,
Toon
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v5 5/5] log: speed up --follow following renames for non-linear history
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
` (3 preceding siblings ...)
2026-11-09 8:05 ` [PATCH v5 4/5] log: simplify --follow following renames for non-linear history Miklos Vajna
@ 2026-11-09 8:06 ` Miklos Vajna
2026-11-11 0:40 ` [PATCH v5 0/5] " Justin Tobler
5 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-11-09 8:06 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Test the performance of 'git log --stat --follow --
include/linux/rsc_table.h' on linux.git: since commit 304812ed33 (log:
improve --follow following renames for non-linear history, 2026-06-22),
it takes 6m43s, while it took about 28 seconds before.
What happens is that log_tree_diff() diffs each merge against all of its
parents with rename following. diff_tree_oid() only does the expensive
rename detection when the path is added relative to a parent, but that
is common for merges, e.g. when a branch forked before the path was
created merges mainline, and this slows down 'git log --follow'.
Fix the problem by first diffing the merge against its parents without
rename following, to find a parent where the path is unchanged. If there
is such a parent, then skip the diff against the other parents and just
follow the same path there. The exception is a parent that doesn't have
the path, but still needs renames to be followed:
- The merge base has the path, i.e. that side renamed or removed it, or
there is no merge base, e.g. a subtree merge of unrelated histories.
- The merge base doesn't have the path, but the parent changed a file
since then, and the merge dropped that change. A copy detection also
finds that file as the source of the path, i.e. the other side renamed
or copied the path from that file.
If the path changed relative to all parents, then diff against them with
rename following, as before. Given that this way a parent may be asked
to follow a path that it doesn't have, also adjust
remember_follow_pathspec(): if children disagree on what should be the
path for a commit, then prefer the path that exists in that commit.
Now the above use case takes 42s, somewhat close to the original cost.
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
log-tree.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 155 insertions(+), 4 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 0aa27fb210..1dc02822f1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -31,6 +31,7 @@
#include "range-diff.h"
#include "strmap.h"
#include "tree.h"
+#include "tree-walk.h"
#include "wildmatch.h"
#include "write-or-die.h"
#include "pager.h"
@@ -1110,6 +1111,22 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path)
PATHSPEC_LITERAL_PATH, "", paths);
}
+static int tree_has_path(struct rev_info *opt, struct object_id *tree,
+ const char *path)
+{
+ struct object_id oid;
+ unsigned short mode;
+
+ return !get_tree_entry(opt->diffopt.repo, tree, path, &oid, &mode);
+}
+
+static int commit_has_path(struct rev_info *opt, struct commit *c,
+ const char *path)
+{
+ parse_commit_or_die(c);
+ return tree_has_path(opt, get_commit_tree_oid(c), path);
+}
+
static void remember_follow_pathspec(struct rev_info *opt,
struct commit *c, const char *path)
{
@@ -1122,8 +1139,12 @@ static void remember_follow_pathspec(struct rev_info *opt,
init_follow_pathspec_slab(opt->follow_pathspec_slab);
}
slot = follow_pathspec_slab_at(opt->follow_pathspec_slab, c);
- if (*slot && !strcmp(*slot, path))
- return;
+ /* If children of 'c' disagree on the path, prefer the one 'c' has. */
+ if (*slot) {
+ if (!strcmp(*slot, path) || commit_has_path(opt, c, *slot) ||
+ !commit_has_path(opt, c, path))
+ return;
+ }
free(*slot);
*slot = xstrdup(path);
}
@@ -1153,6 +1174,127 @@ void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
+/* Find a parent where 'path' is the same as in the merge. */
+static struct commit *follow_same_parent(struct rev_info *opt,
+ struct commit_list *parents,
+ struct object_id *merge_tree)
+{
+ struct commit *same = NULL;
+
+ opt->diffopt.flags.follow_renames = 0;
+ for (; parents && !same; parents = parents->next) {
+ parse_commit_or_die(parents->item);
+ diff_tree_oid(get_commit_tree_oid(parents->item), merge_tree,
+ "", &opt->diffopt);
+ if (!diff_queued_diff.nr)
+ same = parents->item;
+ diff_queue_clear(&diff_queued_diff);
+ }
+ opt->diffopt.flags.follow_renames = 1;
+ return same;
+}
+
+/*
+ * Did 'parent' change a file since 'base', but the merge tree dropped that
+ * change? I.e. the merge doesn't have the file or it's the same as in 'same'.
+ */
+static int follow_parent_changed_dropped_file(struct rev_info *opt,
+ struct commit *base,
+ struct commit *parent,
+ struct commit *same,
+ struct object_id *merge_tree)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
+
+ repo_diff_setup(opt->diffopt.repo, &diff_opts);
+ diff_opts.flags.recursive = 1;
+ diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&diff_opts);
+ diff_tree_oid(get_commit_tree_oid(base), get_commit_tree_oid(parent),
+ "", &diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+ struct object_id merge_oid, same_oid;
+ unsigned short mode;
+
+ if (!DIFF_FILE_VALID(p->two))
+ continue;
+ if (get_tree_entry(opt->diffopt.repo, merge_tree, p->two->path,
+ &merge_oid, &mode))
+ ret = 1;
+ else
+ ret = !oideq(&merge_oid, &p->two->oid) &&
+ !get_tree_entry(opt->diffopt.repo,
+ get_commit_tree_oid(same),
+ p->two->path, &same_oid, &mode) &&
+ oideq(&merge_oid, &same_oid);
+ }
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
+/* Is 'path' in the merge tree renamed or copied from a file of 'parent'? */
+static int follow_has_rename_source(struct rev_info *opt,
+ struct commit *parent,
+ struct object_id *merge_tree,
+ const char *path)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
+
+ repo_diff_setup(opt->diffopt.repo, &diff_opts);
+ diff_opts.flags.recursive = 1;
+ diff_opts.detect_rename = DIFF_DETECT_COPY;
+ diff_opts.rename_score = opt->diffopt.rename_score;
+ diff_opts.single_follow = path;
+ diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&diff_opts);
+ diff_tree_oid(get_commit_tree_oid(parent), merge_tree, "", &diff_opts);
+ diffcore_std(&diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+
+ ret = (p->status == DIFF_STATUS_RENAMED ||
+ p->status == DIFF_STATUS_COPIED) &&
+ !strcmp(p->two->path, path);
+ }
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
+/*
+ * When the path in a merge tree is the same as in 'same', does the path still
+ * need renames to be followed in 'parent'?
+ */
+static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
+ struct commit *same,
+ struct object_id *merge_tree, const char *path)
+{
+ struct commit_list *bases = NULL, *base;
+ int ret = 0, changed_dropped = 0;
+
+ if (parent == same || commit_has_path(opt, parent, path) ||
+ !commit_has_path(opt, same, path))
+ return 0;
+ if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
+ exit(128);
+ /* Unrelated histories, e.g. a subtree merge */
+ ret = !bases;
+ for (base = bases; base && !ret; base = base->next) {
+ ret = commit_has_path(opt, base->item, path);
+ if (!ret && !changed_dropped)
+ changed_dropped = follow_parent_changed_dropped_file(
+ opt, base->item, parent, same, merge_tree);
+ }
+ commit_list_free(bases);
+ if (!ret && changed_dropped)
+ ret = follow_has_rename_source(opt, parent, merge_tree, path);
+ return ret;
+}
+
/* Show the remerge or combined diff of a merge commit */
static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
struct commit_list *parents,
@@ -1187,6 +1329,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
int follow_only = 0;
char *path = NULL;
+ struct commit *same = NULL;
if (!all_need_diff && !opt->merges_need_diff)
return 0;
@@ -1234,6 +1377,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
if (opt->diffopt.flags.follow_renames)
path = xstrdup_or_null(
pathspec_single_path(&opt->diffopt.pathspec));
+ if (path && is_merge && (follow_only || opt->first_parent_merges))
+ same = follow_same_parent(opt, parents, oid);
showed_log = 0;
all_parents = parents;
@@ -1241,8 +1386,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
struct commit *parent = parents->item;
parse_commit_or_die(parent);
- diff_tree_oid(get_commit_tree_oid(parent),
- oid, "", &opt->diffopt);
+ /*
+ * Same as in a parent: follow the path as-is, unless renames
+ * still need to be followed in this parent.
+ */
+ if (!follow_only || !same ||
+ follow_needs_renames(opt, parent, same, oid, path))
+ diff_tree_oid(get_commit_tree_oid(parent),
+ oid, "", &opt->diffopt);
if (follow_only) {
diff_queue_clear(&diff_queued_diff);
opt->diffopt.found_follow = 0;
--
2.51.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
2026-11-09 8:03 ` [PATCH v5 0/5] " Miklos Vajna
` (4 preceding siblings ...)
2026-11-09 8:06 ` [PATCH v5 5/5] log: speed up " Miklos Vajna
@ 2026-11-11 0:40 ` Justin Tobler
2026-11-11 10:43 ` Patrick Steinhardt
2026-11-11 16:44 ` [PATCH v5 0/5] log: speed up --follow following renames for non-linear history Junio C Hamano
5 siblings, 2 replies; 43+ messages in thread
From: Justin Tobler @ 2026-11-11 0:40 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, ps, git
On 26/11/09 09:03AM, Miklos Vajna wrote:
>Hi Patrick,
>
>Thanks for the reviews of v4. Changes in v5:
>
>- The fix of the old 6/6 is now squashed into the speedup patch (5/5),
> so no commit in the series loses history, and its test is moved to
> 1/5. While at it, I simplified 5/5: instead of two passes of diffs
> against the parents, it first finds a parent where the path is
> unchanged, and then only diffs against the other parents if they
> still need renames to be followed. Because of the squash, the
> range-diff pairs the new 5/5 with the old 6/6.
>
>- 5/5 now also follows renames when a parent has no merge base with
> the parent where the path is unchanged, e.g. a subtree merge of
> unrelated histories. Added a test for this to 1/5.
>
>- 5/5 now also follows copies, not only renames, when the merge dropped
> the edit of the copy source. Added your test for this to 1/5. With
> this, the linux.git case takes about 42s, instead of 37s.
I'm a bit late to the party, but I decided to run some benchmarks
against linux.git to showcase performance with 304812ed33de (log:
improve --follow following renames for non-linear history, 2026-06-22)
reverted, at the regression, and with v5 of this series applied:
Benchmark 1: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted)
Time (mean ± σ): 23.493 s ± 0.645 s [User: 23.001 s, System: 0.389 s]
Range (min … max): 22.748 s … 23.883 s 3 runs
Benchmark 2: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
Time (mean ± σ): 335.883 s ± 4.866 s [User: 333.773 s, System: 0.577 s]
Range (min … max): 332.296 s … 341.422 s 3 runs
Benchmark 3: git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
Time (mean ± σ): 38.688 s ± 0.379 s [User: 38.081 s, System: 0.415 s]
Range (min … max): 38.250 s … 38.923 s 3 runs
Summary
git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) ran
14.30 ± 0.44 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
1.65 ± 0.05 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
While the fix in this series does improve the state of things, it is
still overall a significant performance regression that I expect will
result in increased file history timeouts on the server-side for some
repositories we host at GitLab. Rename following for a file's history
even prior to this regression can be expensive for large repositories.
While the original patch and this series do improve correctness of
following a file through history, I wonder if the increased cost is
ultimately worth it in most scenarios. Maybe this could be something
opt-in with another flag instead of toggled on my default?
With this in mind, maybe it would be worth it to revert 304812ed33de
(log: improve --follow following renames for non-linear history,
2026-06-22) and restart with a fresh series. IMO, it might make the
changes here a bit easier to validate also.
Thanks,
-Justin
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
2026-11-11 0:40 ` [PATCH v5 0/5] " Justin Tobler
@ 2026-11-11 10:43 ` Patrick Steinhardt
2026-11-11 12:41 ` Miklos Vajna
2026-11-11 16:44 ` [PATCH v5 0/5] log: speed up --follow following renames for non-linear history Junio C Hamano
1 sibling, 1 reply; 43+ messages in thread
From: Patrick Steinhardt @ 2026-11-11 10:43 UTC (permalink / raw)
To: Justin Tobler; +Cc: Miklos Vajna, gitster, szeder.dev, git
On Tue, Nov 10, 2026 at 06:40:00PM -0600, Justin Tobler wrote:
> On 26/11/09 09:03AM, Miklos Vajna wrote:
> > Hi Patrick,
> >
> > Thanks for the reviews of v4. Changes in v5:
> >
> > - The fix of the old 6/6 is now squashed into the speedup patch (5/5),
> > so no commit in the series loses history, and its test is moved to
> > 1/5. While at it, I simplified 5/5: instead of two passes of diffs
> > against the parents, it first finds a parent where the path is
> > unchanged, and then only diffs against the other parents if they
> > still need renames to be followed. Because of the squash, the
> > range-diff pairs the new 5/5 with the old 6/6.
> >
> > - 5/5 now also follows renames when a parent has no merge base with
> > the parent where the path is unchanged, e.g. a subtree merge of
> > unrelated histories. Added a test for this to 1/5.
> >
> > - 5/5 now also follows copies, not only renames, when the merge dropped
> > the edit of the copy source. Added your test for this to 1/5. With
> > this, the linux.git case takes about 42s, instead of 37s.
>
> I'm a bit late to the party, but I decided to run some benchmarks
> against linux.git to showcase performance with 304812ed33de (log:
> improve --follow following renames for non-linear history, 2026-06-22)
> reverted, at the regression, and with v5 of this series applied:
>
> Benchmark 1: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted)
> Time (mean ± σ): 23.493 s ± 0.645 s [User: 23.001 s, System: 0.389 s]
> Range (min … max): 22.748 s … 23.883 s 3 runs
>
> Benchmark 2: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
> Time (mean ± σ): 335.883 s ± 4.866 s [User: 333.773 s, System: 0.577 s]
> Range (min … max): 332.296 s … 341.422 s 3 runs
>
> Benchmark 3: git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
> Time (mean ± σ): 38.688 s ± 0.379 s [User: 38.081 s, System: 0.415 s]
> Range (min … max): 38.250 s … 38.923 s 3 runs
>
> Summary
> git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) ran
> 14.30 ± 0.44 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
> 1.65 ± 0.05 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
>
> While the fix in this series does improve the state of things, it is
> still overall a significant performance regression that I expect will
> result in increased file history timeouts on the server-side for some
> repositories we host at GitLab. Rename following for a file's history
> even prior to this regression can be expensive for large repositories.
>
> While the original patch and this series do improve correctness of
> following a file through history, I wonder if the increased cost is
> ultimately worth it in most scenarios. Maybe this could be something
> opt-in with another flag instead of toggled on my default?
Yeah, by now I'm also leaning into this direction. The performance cost
is definitely way too high for us, so we need a way to disable this new
feature altogether at GitLab.
> With this in mind, maybe it would be worth it to revert 304812ed33de
> (log: improve --follow following renames for non-linear history,
> 2026-06-22) and restart with a fresh series. IMO, it might make the
> changes here a bit easier to validate also.
I don't have a strong sense on whether or not we should revert the whole
series. I find the patches quite hard to review, but that may not be
because we're fixing things incrementally and rather be because I myself
am not familiar with this subsystem.
But in case we decide to not revert I'd like to see a toggle to disable
this feature as a first step. As mentioned, this change has caused a
rather big regression for GitLab, so we'd ideally have a way to disable
this feature soon so that we can mitigate it.
I don't want to make you (Miklos) responsible for delivering that of
course, I just want us to align on a fast path forward. In case we don't
revert we can help out and contribute such a default-disabled toggle.
And if so, that would also give us more time to fix the issue properly.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
2026-11-11 10:43 ` Patrick Steinhardt
@ 2026-11-11 12:41 ` Miklos Vajna
2026-11-11 13:29 ` Patrick Steinhardt
0 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-11-11 12:41 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Justin Tobler, gitster, szeder.dev, git
Hi Patrick,
On Wed, Nov 11, 2026 at 11:43:50AM +0100, Patrick Steinhardt <ps@pks.im> wrote:
> > While the original patch and this series do improve correctness of
> > following a file through history, I wonder if the increased cost is
> > ultimately worth it in most scenarios. Maybe this could be something
> > opt-in with another flag instead of toggled on my default?
>
> Yeah, by now I'm also leaning into this direction. The performance cost
> is definitely way too high for us, so we need a way to disable this new
> feature altogether at GitLab.
I have no problem with turning this from default to opt-in. I would be
happy to not revert, since a lot of effort went into this, and some see
this as a bug in git-log, not showing some history which is there.
> I don't have a strong sense on whether or not we should revert the whole
> series. I find the patches quite hard to review, but that may not be
> because we're fixing things incrementally and rather be because I myself
> am not familiar with this subsystem.
>
> But in case we decide to not revert I'd like to see a toggle to disable
> this feature as a first step. As mentioned, this change has caused a
> rather big regression for GitLab, so we'd ideally have a way to disable
> this feature soon so that we can mitigate it.
Makes sense. To be honest, I'm not too happy about the complexity the
last patch in the series introduces -- but having performance that is
somewhat close to the old behavior seems to justify it.
> I don't want to make you (Miklos) responsible for delivering that of
> course, I just want us to align on a fast path forward. In case we don't
> revert we can help out and contribute such a default-disabled toggle.
> And if so, that would also give us more time to fix the issue properly.
Yes, aligning on what we want would be good. :-) I'm OK to replace the
last patch in the series with a patch that turns this off by default,
but then how to opt in, would be be fine to have a new config option for
this (similar to log.follow)? Would that do what you need or you want
something else?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
2026-11-11 12:41 ` Miklos Vajna
@ 2026-11-11 13:29 ` Patrick Steinhardt
2026-11-12 9:05 ` [PATCH] log: make --follow following renames for non-linear history optional Miklos Vajna
0 siblings, 1 reply; 43+ messages in thread
From: Patrick Steinhardt @ 2026-11-11 13:29 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Justin Tobler, gitster, szeder.dev, git
On Wed, Nov 11, 2026 at 12:41:21PM +0000, Miklos Vajna wrote:
> On Wed, Nov 11, 2026 at 11:43:50AM +0100, Patrick Steinhardt <ps@pks.im> wrote:
[snip]
> > I don't want to make you (Miklos) responsible for delivering that of
> > course, I just want us to align on a fast path forward. In case we don't
> > revert we can help out and contribute such a default-disabled toggle.
> > And if so, that would also give us more time to fix the issue properly.
>
> Yes, aligning on what we want would be good. :-) I'm OK to replace the
> last patch in the series with a patch that turns this off by default,
> but then how to opt in, would be be fine to have a new config option for
> this (similar to log.follow)? Would that do what you need or you want
> something else?
I don't really have a good sense of how far this patch series is on its
road to getting merged. So how about we first land a single-patch series
that introduces the knob that we can merge without the remaining parts
in this series here?
(For context, we deploy from Git's master branch at GitLab, and hence
my desire to have this regression fixed fast. As said, I'm happy to
help out with that, I don't want to push the whole burden on you
here.)
With regards to how to opt-in, I can see two ways:
- We could extend "--follow" to take an optional parameter so that the
user can pass "--follow=merges". Same with "log.follow", we could
teach it another option "merges".
- We could introduce "--follow-merges" and "log.followMerges".
I'm leaning towards the first option myself.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH] log: make --follow following renames for non-linear history optional
2026-11-11 13:29 ` Patrick Steinhardt
@ 2026-11-12 9:05 ` Miklos Vajna
2026-11-12 19:09 ` Junio C Hamano
0 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-11-12 9:05 UTC (permalink / raw)
To: gitster; +Cc: ps, jltobler, szeder.dev, git
Since commit 304812ed33 (log: improve --follow following renames for
non-linear history, 2026-06-22), 'git log --stat --follow --
include/linux/rsc_table.h' on linux.git takes 6m43s, while it took about
28 seconds before.
What happens is that log_tree_commit() diffs each merge against all of
its parents with rename following, to find the path to follow in each
parent. diff_tree_oid() only does the expensive rename detection when
the path is added relative to a parent, but that is common for merges,
e.g. when a branch forked before the path was created merges mainline.
Fix the problem by making this per-parent diffing opt-in: plain --follow
behaves as before 304812ed33 again, and 'git log --follow=merges'
follows the path separately in each parent of a merge. Add a matching
log.follow=merges configuration option as well.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
Hi Junio,
On Wed, Nov 11, 2026 at 08:44:42AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> This sounds sensible, and is far better than reverting and declaring
> "The original 'checkbox hack' should stay here forever because doing
> it right will be very expensive".
If that is fine with you, I suggest just replacing the
mv/log-follow-speedup branch with this patch.
Hi Patrick,
On Wed, Nov 11, 2026 at 02:29:22PM +0100, Patrick Steinhardt <ps@pks.im> wrote:
> I don't really have a good sense of how far this patch series is on its
> road to getting merged. So how about we first land a single-patch series
> that introduces the knob that we can merge without the remaining parts
> in this series here?
I like the idea, here is a patch to do that.
> With regards to how to opt-in, I can see two ways:
>
> - We could extend "--follow" to take an optional parameter so that the
> user can pass "--follow=merges". Same with "log.follow", we could
> teach it another option "merges".
>
> - We could introduce "--follow-merges" and "log.followMerges".
>
> I'm leaning towards the first option myself.
Yes, --follow=merges also appears more natural to me, so I implemented
it that way.
Thanks,
Miklos
Documentation/config/log.adoc | 4 +++-
Documentation/git-log.adoc | 7 +++++--
builtin/log.c | 15 +++++++++++++--
diff.c | 15 +++++++++++----
diff.h | 2 ++
log-tree.c | 6 ++++--
t/t4219-log-follow-merge.sh | 32 +++++++++++++++++++++++++-------
7 files changed, 63 insertions(+), 18 deletions(-)
diff --git a/Documentation/config/log.adoc b/Documentation/config/log.adoc
index f7dfce69b5..72a3486a73 100644
--- a/Documentation/config/log.adoc
+++ b/Documentation/config/log.adoc
@@ -53,7 +53,9 @@ This is the same as the `--decorate` option of the `git log`.
`log.follow`::
If `true`, `git log` will act as if the `--follow` option was used when
a single <path> is given. This has the same limitations as `--follow`,
- i.e. it cannot be used to follow multiple files.
+ i.e. it cannot be used to follow multiple files and does not work well
+ on non-linear history. If `merges`, `git log` will act as if the
+ `--follow=merges` option was used.
`log.graphColors`::
A list of colors, separated by commas, that can be used to draw
diff --git a/Documentation/git-log.adoc b/Documentation/git-log.adoc
index fb3ac11283..742b832bbf 100644
--- a/Documentation/git-log.adoc
+++ b/Documentation/git-log.adoc
@@ -27,9 +27,12 @@ each commit introduces are shown.
OPTIONS
-------
-`--follow`::
+`--follow[=merges]`::
Continue listing the history of a file beyond renames
- (works only for a single file).
+ (works only for a single file). This does not work well on
+ non-linear history, unless `merges` is given: then the path is
+ followed separately for each parent of a merge, which is more
+ expensive.
`--no-decorate`::
`--decorate[=(short|full|auto|no)]`::
diff --git a/builtin/log.c b/builtin/log.c
index 350b35c556..db707877f6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -108,6 +108,7 @@ static int parse_decoration_style(const char *value)
struct log_config {
int default_abbrev_commit;
int default_show_root;
+ /* 0: no, 1: yes, 2: merges */
int default_follow;
int default_show_signature;
int default_encode_email_headers;
@@ -205,6 +206,8 @@ static void cmd_log_init_defaults(struct rev_info *rev,
get_commit_format(cfg->fmt_pretty, rev);
if (cfg->default_follow)
rev->diffopt.flags.default_follow_renames = 1;
+ if (cfg->default_follow == 2)
+ rev->diffopt.flags.default_follow_merges = 1;
rev->verbose_header = 1;
init_diffstat_widths(&rev->diffopt);
rev->diffopt.flags.recursive = 1;
@@ -505,7 +508,10 @@ static int git_log_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "log.follow")) {
- cfg->default_follow = git_config_bool(var, value);
+ if (value && !strcmp(value, "merges"))
+ cfg->default_follow = 2;
+ else
+ cfg->default_follow = git_config_bool(var, value);
return 0;
}
if (skip_prefix(var, "color.decorate.", &slot_name))
@@ -814,9 +820,14 @@ int cmd_log_reflog(int argc,
static void log_setup_revisions_tweak(struct rev_info *rev)
{
+ /* If --follow is not used, use the log.follow option. */
if (rev->diffopt.flags.default_follow_renames &&
- diff_check_follow_pathspec(&rev->prune_data, 0))
+ !rev->diffopt.flags.follow_renames &&
+ diff_check_follow_pathspec(&rev->prune_data, 0)) {
rev->diffopt.flags.follow_renames = 1;
+ rev->diffopt.flags.follow_merges =
+ rev->diffopt.flags.default_follow_merges;
+ }
if (rev->first_parent_only)
diff_merges_default_to_first_parent(rev);
diff --git a/diff.c b/diff.c
index 414532d09f..b633cbea09 100644
--- a/diff.c
+++ b/diff.c
@@ -5739,12 +5739,19 @@ static int diff_opt_follow(const struct option *opt,
{
struct diff_options *options = opt->value;
- BUG_ON_OPT_ARG(arg);
if (unset) {
options->flags.follow_renames = 0;
+ options->flags.follow_merges = 0;
options->flags.default_follow_renames = 0;
- } else {
+ options->flags.default_follow_merges = 0;
+ } else if (!arg) {
options->flags.follow_renames = 1;
+ options->flags.follow_merges = 0;
+ } else if (!strcmp(arg, "merges")) {
+ options->flags.follow_renames = 1;
+ options->flags.follow_merges = 1;
+ } else {
+ return error(_("invalid argument to --follow: '%s'"), arg);
}
return 0;
}
@@ -6161,9 +6168,9 @@ struct option *add_diff_options(const struct option *opts,
0, PARSE_OPT_NONEG),
OPT_BOOL(0, "rename-empty", &options->flags.rename_empty,
N_("use empty blobs as rename source")),
- OPT_CALLBACK_F(0, "follow", options, NULL,
+ OPT_CALLBACK_F(0, "follow", options, N_("merges"),
N_("continue listing the history of a file beyond renames"),
- PARSE_OPT_NOARG, diff_opt_follow),
+ PARSE_OPT_OPTARG, diff_opt_follow),
OPT_INTEGER('l', NULL, &options->rename_limit,
N_("limit to cheap rename/copy detection if the number of rename/copy targets exceeds this value")),
diff --git a/diff.h b/diff.h
index bb5cddaf34..2548739e83 100644
--- a/diff.h
+++ b/diff.h
@@ -154,6 +154,7 @@ struct diff_flags {
unsigned find_copies_harder;
unsigned follow_renames;
+ unsigned follow_merges;
unsigned rename_empty;
/* Internal; used for optimization to see if there is any change. */
@@ -202,6 +203,7 @@ struct diff_flags {
unsigned dirstat_by_line;
unsigned funccontext;
unsigned default_follow_renames;
+ unsigned default_follow_merges;
unsigned stat_with_summary;
unsigned suppress_diff_headers;
unsigned dual_color_diffed_diffs;
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..93cc61af2d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1277,7 +1277,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
opt->diffopt.no_free = 1;
/* Any recorded path for this commit? If so, restore it */
- if (opt->diffopt.flags.follow_renames) {
+ if (opt->diffopt.flags.follow_renames &&
+ opt->diffopt.flags.follow_merges) {
const char *stored = recall_follow_pathspec(opt, commit);
if (stored) {
const char *current = pathspec_single_path(&opt->diffopt.pathspec);
@@ -1300,7 +1301,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
show_diff_of_diff(opt);
/* Record what path each parent of this commit should use */
- if (opt->diffopt.flags.follow_renames) {
+ if (opt->diffopt.flags.follow_renames &&
+ opt->diffopt.flags.follow_merges) {
struct commit_list *parents = get_saved_parents(opt, commit);
if (parents && parents->next) {
struct commit_list *p;
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index e370f82955..c2bc2b0c13 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='Test --follow follows renames across merges'
+test_description='Test --follow=merges follows renames across merges'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
@@ -25,12 +25,30 @@ test_expect_success 'setup subtree-merged repository' '
git -C outer commit -m "Merge inner repo into inner/ subdirectory"
'
-test_expect_success '--follow finds the pre-merge commit through a subtree merge' '
- git -C outer log --follow --pretty=tformat:%s inner/inner.txt >actual &&
+test_expect_success '--follow=merges finds the pre-merge commit through a subtree merge' '
+ git -C outer log --follow=merges --pretty=tformat:%s inner/inner.txt >actual &&
echo "inner init" >expect &&
test_cmp expect actual
'
+test_expect_success 'log.follow=merges acts as --follow=merges' '
+ git -C outer -c log.follow=merges log --pretty=tformat:%s \
+ inner/inner.txt >actual &&
+ echo "inner init" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--follow overrides log.follow=merges' '
+ git -C outer -c log.follow=merges log --follow \
+ --pretty=tformat:%s inner/inner.txt >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success '--follow rejects an unknown argument' '
+ test_must_fail git -C outer log --follow=foo inner/inner.txt 2>err &&
+ test_grep "invalid argument to --follow: ${SQ}foo${SQ}" err
+'
+
test_expect_success 'setup merge of two branches that both renamed a file to README' '
git init foo &&
mkdir foo/foo &&
@@ -65,8 +83,8 @@ test_expect_success 'setup merge of two branches that both renamed a file to REA
git -C foo commit -m "merge bar into foo"
'
-test_expect_success '--follow follows renames across both sides of a merge' '
- git -C foo log --follow --pretty=tformat:%s README >actual &&
+test_expect_success '--follow=merges follows renames across both sides of a merge' '
+ git -C foo log --follow=merges --pretty=tformat:%s README >actual &&
sort actual >actual.sorted &&
cat >expect <<-\EOF &&
add bar README
@@ -112,8 +130,8 @@ test_expect_success 'setup diamond with renames on both sides of a fork' '
git -C diamond commit -am "Z: modify path"
'
-test_expect_success '--follow follows renames through a fork in a single history' '
- git -C diamond log --follow --pretty=tformat:%s path >actual &&
+test_expect_success '--follow=merges follows renames through a fork in a single history' '
+ git -C diamond log --follow=merges --pretty=tformat:%s path >actual &&
sort actual >actual.sorted &&
cat >expect <<-\EOF &&
A: add path0
--
2.51.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH] log: make --follow following renames for non-linear history optional
2026-11-12 9:05 ` [PATCH] log: make --follow following renames for non-linear history optional Miklos Vajna
@ 2026-11-12 19:09 ` Junio C Hamano
0 siblings, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2026-11-12 19:09 UTC (permalink / raw)
To: Miklos Vajna; +Cc: ps, jltobler, szeder.dev, git
Miklos Vajna <vmiklos@collabora.com> writes:
> Hi Junio,
>
> On Wed, Nov 11, 2026 at 08:44:42AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
>> This sounds sensible, and is far better than reverting and declaring
>> "The original 'checkbox hack' should stay here forever because doing
>> it right will be very expensive".
>
> If that is fine with you, I suggest just replacing the
> mv/log-follow-speedup branch with this patch.
>
> Hi Patrick,
>
> On Wed, Nov 11, 2026 at 02:29:22PM +0100, Patrick Steinhardt <ps@pks.im> wrote:
>> I don't really have a good sense of how far this patch series is on its
>> road to getting merged. So how about we first land a single-patch series
>> that introduces the knob that we can merge without the remaining parts
>> in this series here?
>
> I like the idea, here is a patch to do that.
OK, let me eject the "speedup" topic and queue this one instead for
now. We can discuss how the --follow-merge code can be improved
separately once we are happy with this "opt-in" change.
Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
2026-11-11 0:40 ` [PATCH v5 0/5] " Justin Tobler
2026-11-11 10:43 ` Patrick Steinhardt
@ 2026-11-11 16:44 ` Junio C Hamano
1 sibling, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2026-11-11 16:44 UTC (permalink / raw)
To: Justin Tobler; +Cc: Miklos Vajna, szeder.dev, ps, git
Justin Tobler <jltobler@gmail.com> writes:
> Summary
> git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) ran
> 14.30 ± 0.44 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
> 1.65 ± 0.05 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
> ...
> While the original patch and this series do improve correctness of
> following a file through history, I wonder if the increased cost is
> ultimately worth it in most scenarios. Maybe this could be something
> opt-in with another flag instead of toggled on my default?
>
> With this in mind, maybe it would be worth it to revert 304812ed33de
> (log: improve --follow following renames for non-linear history,
> 2026-06-22) and restart with a fresh series. IMO, it might make the
> changes here a bit easier to validate also.
This sounds sensible, and is far better than reverting and declaring
"The original 'checkbox hack' should stay here forever because doing
it right will be very expensive". Giving it a fresh start would be
valuable as following a single the is an important operation in some
workflows.
Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread