* Performance regression in `git log --follow`
@ 2026-10-13 13:55 Patrick Steinhardt
2026-10-13 21:24 ` SZEDER Gábor
2026-10-14 6:16 ` Performance regression in `git log --follow` Miklos Vajna
0 siblings, 2 replies; 43+ messages in thread
From: Patrick Steinhardt @ 2026-10-13 13:55 UTC (permalink / raw)
To: git; +Cc: Miklos Vajna
Hi,
a user on IRC reported a performance regression in linux.git:
$ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
$ git -C linux log --stat --follow -- include/linux/rsc_table.h
I was able to reproduce the performance regression: while this command
takes roughly 33 seconds with Git 2.55, it takes almost 8 minutes with
Git 2.56.
I've bisected the regression to 304812ed33 (log: improve --follow
following renames for non-linear history, 2026-06-22). I don't have time
right now to debug any further, so I decided to just forward the report
to the mailing list. I've also Cc'd the original author of the above
commit.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: Performance regression in `git log --follow`
2026-10-13 13:55 Performance regression in `git log --follow` Patrick Steinhardt
@ 2026-10-13 21:24 ` SZEDER Gábor
2026-10-15 14:48 ` [PATCH] log: speed up --follow following renames for non-linear history Miklos Vajna
2026-10-14 6:16 ` Performance regression in `git log --follow` Miklos Vajna
1 sibling, 1 reply; 43+ messages in thread
From: SZEDER Gábor @ 2026-10-13 21:24 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Miklos Vajna
On Tue, Oct 13, 2026 at 03:55:01PM +0200, Patrick Steinhardt wrote:
> Hi,
>
> a user on IRC reported a performance regression in linux.git:
>
> $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> $ git -C linux log --stat --follow -- include/linux/rsc_table.h
>
> I was able to reproduce the performance regression: while this command
> takes roughly 33 seconds with Git 2.55, it takes almost 8 minutes with
> Git 2.56.
>
> I've bisected the regression to 304812ed33 (log: improve --follow
> following renames for non-linear history, 2026-06-22). I don't have time
> right now to debug any further, so I decided to just forward the report
> to the mailing list. I've also Cc'd the original author of the above
> commit.
The performance regression is clearly noticable in the git repository,
though the slowdown is not as pronounced as in linux.git:
Benchmark 1: ./304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh
Time (mean ± σ): 3.275 s ± 0.022 s [User: 3.157 s, System: 0.116 s]
Range (min … max): 3.240 s … 3.313 s 10 runs
Benchmark 2: ./304812ed33/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh
Time (mean ± σ): 8.825 s ± 0.029 s [User: 8.554 s, System: 0.264 s]
Range (min … max): 8.769 s … 8.864 s 10 runs
Summary
'./304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh' ran
2.69 ± 0.02 times faster than './304812ed33/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh'
That's 5.5 seconds of runtime difference, and with a bit of trace
performance hack[1] it turns out that most of it is spent in a call to
propagate_follow_pathspec_to_parent() to compare the current commit
with its second...N-th parent:
$ GIT_TRACE_PERFORMANCE=1 ./304812ed33/bin/git --no-pager log --follow --oneline v2.56.0 t/valgrind/valgrind.sh >/dev/null
21:59:30.465592 read-cache.c:2364 performance: 0.000626524 s: read cache .git/index
21:59:39.246697 log-tree.c:1158 performance: 1.374980154 s: first parent propagate_follow_pathspec_to_parent()
21:59:39.247164 log-tree.c:1159 performance: 4.448692273 s: N-th parents propagate_follow_pathspec_to_parent()
21:59:39.247192 trace.c:416 performance: 8.799236172 s: git command: ./304812ed33/bin/git --no-pager log --follow --oneline v2.56.0 t/valgrind/valgrind.sh
Interestingly, although line-level log performs its own rename
tracking, it's still notably faster than "--follow" even before this
perf regression (that "-L1,40" means "track the whole file"):
Benchmark 1: ./304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh
Time (mean ± σ): 3.260 s ± 0.019 s [User: 3.140 s, System: 0.118 s]
Range (min … max): 3.219 s … 3.284 s 10 runs
Benchmark 2: ./304812ed33^/bin/git --no-pager log --oneline --no-patch -L1,40:t/valgrind/valgrind.sh v2.56.0
Time (mean ± σ): 1.454 s ± 0.014 s [User: 1.369 s, System: 0.084 s]
Range (min … max): 1.441 s … 1.481 s 10 runs
Summary
'./304812ed33^/bin/git --no-pager log --oneline --no-patch -L1,40:t/valgrind/valgrind.sh v2.56.0' ran
2.24 ± 0.02 times faster than './304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh'
The reason for this might be:
- If line-level log finds that the interesting path is not modified
between a merge commit and its first parent, then it doesn't
perform a diff between that merge commit and its second..N-th
parent.
"--follow" seems to performs a diff between a merge commit and all
its parents.
- Line-level log checks for renames only when the path is not
present in the parent commit.
"--follow" seems to always look for renames.
[1] It really is just a hack:
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..fd4824fd3e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -34,6 +34,7 @@
#include "wildmatch.h"
#include "write-or-die.h"
#include "pager.h"
+#include "trace.h"
static struct decoration name_decoration = { "object names" };
static int decoration_loaded;
@@ -1144,6 +1145,8 @@ static void free_follow_pathspec_slot(char **slot)
FREE_AND_NULL(*slot);
}
+static uint64_t t_first = 0, t_nth = 0;
+
void release_follow_pathspec_slab(struct rev_info *opt)
{
if (!opt->follow_pathspec_slab)
@@ -1151,6 +1154,9 @@ void release_follow_pathspec_slab(struct rev_info *opt)
deep_clear_follow_pathspec_slab(opt->follow_pathspec_slab,
free_follow_pathspec_slot);
FREE_AND_NULL(opt->follow_pathspec_slab);
+
+ trace_performance(t_first, "first parent propagate_follow_pathspec_to_parent()");
+ trace_performance(t_nth, "N-th parents propagate_follow_pathspec_to_parent()");
}
/* Compute a path to follow in parent, if there is one */
@@ -1304,9 +1310,18 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
struct commit_list *parents = get_saved_parents(opt, commit);
if (parents && parents->next) {
struct commit_list *p;
- for (p = parents; p; p = p->next)
+ for (p = parents; p; p = p->next) {
+ if (p == parents)
+ t_first -= getnanotime();
+ else
+ t_nth -= getnanotime();
propagate_follow_pathspec_to_parent(opt, commit,
p->item);
+ if (p == parents)
+ t_first += getnanotime();
+ else
+ t_nth += getnanotime();
+ }
} else if (parents) {
remember_follow_pathspec(opt, parents->item,
pathspec_single_path(&opt->diffopt.pathspec));
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: Performance regression in `git log --follow`
2026-10-13 13:55 Performance regression in `git log --follow` Patrick Steinhardt
2026-10-13 21:24 ` SZEDER Gábor
@ 2026-10-14 6:16 ` Miklos Vajna
1 sibling, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-14 6:16 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
Hi Patrick,
On Tue, Oct 13, 2026 at 03:55:01PM +0200, Patrick Steinhardt <ps@pks.im> wrote:
> I've bisected the regression to 304812ed33 (log: improve --follow
> following renames for non-linear history, 2026-06-22). I don't have time
> right now to debug any further, so I decided to just forward the report
> to the mailing list. I've also Cc'd the original author of the above
> commit.
Thanks for the report.
To some extent, this is expected: previously log --follow had a design
issue, as described at
<https://lore.kernel.org/git/xmqqjysz7r41.fsf@gitster.g/>. Now log
--follow finds more commits has it traverses non-linear history, which
takes more time.
Let's see if we can find some creative solution that keeps the new
design (storing / restoring the path to follow as you traverse
non-linear history) and at the same time improves performance for your
use-case. Quite possibly my patch is/was naive and we can avoid some not
needed work while still correctly following a path across renames.
I see Gabor already commented in the thread and same some ideas, but I
haven't read those yet.
Regards,
Miklos
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH] log: speed up --follow following renames for non-linear history
2026-10-13 21:24 ` SZEDER Gábor
@ 2026-10-15 14:48 ` Miklos Vajna
2026-10-15 16:57 ` Junio C Hamano
2026-10-16 20:37 ` [PATCH] " Junio C Hamano
0 siblings, 2 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-15 14:48 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Patrick Steinhardt, 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 propagate_follow_pathspec_to_parent() does a
somewhat expensive rename detection of the current commit vs all of its
parents, and this slows down 'git log --follow'.
Fix the problem by introducing two shortcuts:
1) Only do that when the path is added relative to a parent and
2) If the path is the same as in one of the parents, then just follow
that history
These shortcuts keep all the uses-cases in t4219-log-follow-merge.sh
working, but now the above use case takes 29s, close to the original
cost.
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
Hi Gabor, Patrick,
Here is my suggested speed-up patch. I think it's correct to take these two shortcuts since the
original use cases continue to work, and then we're almost back to the original performance.
Let me know what you think; if you don't see problems with it, then a bit later I would send the
patch to Junio.
Thanks,
Miklos
log-tree.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 4 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..0d7bf001f0 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1180,6 +1180,74 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
diff_free(&diff_opts);
}
+enum follow_path_change {
+ FOLLOW_PATH_SAME,
+ FOLLOW_PATH_ADDED,
+ FOLLOW_PATH_OTHER,
+};
+
+/* How does the followed path change from parent to commit? */
+static enum follow_path_change get_follow_path_change(struct rev_info *opt,
+ struct commit *commit,
+ struct commit *parent)
+{
+ struct diff_options diff_opts;
+ enum follow_path_change ret = FOLLOW_PATH_OTHER;
+
+ 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.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);
+
+ if (!diff_queued_diff.nr)
+ ret = FOLLOW_PATH_SAME;
+ else if (diff_queued_diff.nr == 1 &&
+ !DIFF_FILE_VALID(diff_queued_diff.queue[0]->one))
+ ret = FOLLOW_PATH_ADDED;
+
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
+/* Compute a path to follow in each parent of a merge */
+static void propagate_follow_pathspec_to_parents(struct rev_info *opt,
+ struct commit *commit,
+ struct commit_list *parents)
+{
+ const char *path = pathspec_single_path(&opt->diffopt.pathspec);
+ struct commit_list *p;
+ enum follow_path_change *changes;
+ int nr = commit_list_count(parents), i;
+ int has_same = 0;
+
+ if (!path)
+ return;
+
+ ALLOC_ARRAY(changes, nr);
+ for (p = parents, i = 0; p; p = p->next, i++) {
+ changes[i] = get_follow_path_change(opt, commit, p->item);
+ if (changes[i] == FOLLOW_PATH_SAME) {
+ has_same = 1;
+ break;
+ }
+ }
+
+ for (p = parents, i = 0; p; p = p->next, i++) {
+ if (!has_same && changes[i] == FOLLOW_PATH_ADDED)
+ /* Never the same, added here: expensive rename detection */
+ propagate_follow_pathspec_to_parent(opt, commit, p->item);
+ else
+ remember_follow_pathspec(opt, p->item, path);
+ }
+ free(changes);
+}
+
/*
* Show the diff of a commit.
*
@@ -1303,10 +1371,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
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);
+ propagate_follow_pathspec_to_parents(opt, commit,
+ parents);
} else if (parents) {
remember_follow_pathspec(opt, parents->item,
pathspec_single_path(&opt->diffopt.pathspec));
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH] log: speed up --follow following renames for non-linear history
2026-10-15 14:48 ` [PATCH] log: speed up --follow following renames for non-linear history Miklos Vajna
@ 2026-10-15 16:57 ` Junio C Hamano
2026-10-19 6:10 ` [PATCH v2] " Miklos Vajna
2026-10-16 20:37 ` [PATCH] " Junio C Hamano
1 sibling, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2026-10-15 16:57 UTC (permalink / raw)
To: Miklos Vajna; +Cc: SZEDER Gábor, Patrick Steinhardt, git
Miklos Vajna <vmiklos@collabora.com> writes:
> 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 propagate_follow_pathspec_to_parent() does a
> somewhat expensive rename detection of the current commit vs all of its
> parents, and this slows down 'git log --follow'.
>
> Fix the problem by introducing two shortcuts:
> 1) Only do that when the path is added relative to a parent and
> 2) If the path is the same as in one of the parents, then just follow
> that history
>
> These shortcuts keep all the uses-cases in t4219-log-follow-merge.sh
> working, but now the above use case takes 29s, close to the original
> cost.
The original follow-renames implementation kicked in only when the
path being followed did not exist in the parent. log_tree_commit()
calls log_tree_diff() to show the diff for the current commit, which
calls diff_tree_oid(), which checks with diff_might_be_rename() to
see if the path we have been following is missing in the parent and
only then runs try_to_follow_renames(), which is expensive, to find
out where it came from. It avoided the expensive second diff when
we know the path we have been following existed in the parent.
But reading log_tree_commit() in today's code, it seems that
propagate_follow_pathspec_to_parent() is called, after we have
ALREADY spent cycles to run the log_tree_diff() and flushed its
output. The precious clue that tells you if the path you have been
following is shared with the parent has already been flushed at that
point.
Perhaps inside log_tree_diff_flush() while we still have the list of
filepairs in diff_queue is where you should be able to see if any of
the paths you have been following appeared anew relative to the
parent(s) and to decide if you must adjust the list of paths you
follow for renames, instead of calling the diff machinery all the
time from propagate_follow_pathspec_to_parent() unconditionally.
I am not sure if it may be cleaner to first rip out all the change
introduced in 304812ed33 (log: improve --follow following renames
for non-linear history, 2026-06-22) and then rethink the approach,
instead of piling more code with additional patches. I dunno.
Thanks.
> log-tree.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 70 insertions(+), 4 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index 83a3c4bf9b..0d7bf001f0 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -1180,6 +1180,74 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
> diff_free(&diff_opts);
> }
>
> +enum follow_path_change {
> + FOLLOW_PATH_SAME,
> + FOLLOW_PATH_ADDED,
> + FOLLOW_PATH_OTHER,
> +};
> +
> +/* How does the followed path change from parent to commit? */
> +static enum follow_path_change get_follow_path_change(struct rev_info *opt,
> + struct commit *commit,
> + struct commit *parent)
> +{
> + struct diff_options diff_opts;
> + enum follow_path_change ret = FOLLOW_PATH_OTHER;
> +
> + 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.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);
> +
> + if (!diff_queued_diff.nr)
> + ret = FOLLOW_PATH_SAME;
> + else if (diff_queued_diff.nr == 1 &&
> + !DIFF_FILE_VALID(diff_queued_diff.queue[0]->one))
> + ret = FOLLOW_PATH_ADDED;
> +
> + diff_queue_clear(&diff_queued_diff);
> + diff_free(&diff_opts);
> + return ret;
> +}
> +
> +/* Compute a path to follow in each parent of a merge */
> +static void propagate_follow_pathspec_to_parents(struct rev_info *opt,
> + struct commit *commit,
> + struct commit_list *parents)
> +{
> + const char *path = pathspec_single_path(&opt->diffopt.pathspec);
> + struct commit_list *p;
> + enum follow_path_change *changes;
> + int nr = commit_list_count(parents), i;
> + int has_same = 0;
> +
> + if (!path)
> + return;
> +
> + ALLOC_ARRAY(changes, nr);
> + for (p = parents, i = 0; p; p = p->next, i++) {
> + changes[i] = get_follow_path_change(opt, commit, p->item);
> + if (changes[i] == FOLLOW_PATH_SAME) {
> + has_same = 1;
> + break;
> + }
> + }
> +
> + for (p = parents, i = 0; p; p = p->next, i++) {
> + if (!has_same && changes[i] == FOLLOW_PATH_ADDED)
> + /* Never the same, added here: expensive rename detection */
> + propagate_follow_pathspec_to_parent(opt, commit, p->item);
> + else
> + remember_follow_pathspec(opt, p->item, path);
> + }
> + free(changes);
> +}
> +
> /*
> * Show the diff of a commit.
> *
> @@ -1303,10 +1371,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
> 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);
> + propagate_follow_pathspec_to_parents(opt, commit,
> + parents);
> } else if (parents) {
> remember_follow_pathspec(opt, parents->item,
> pathspec_single_path(&opt->diffopt.pathspec));
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] log: speed up --follow following renames for non-linear history
2026-10-15 14:48 ` [PATCH] log: speed up --follow following renames for non-linear history Miklos Vajna
2026-10-15 16:57 ` Junio C Hamano
@ 2026-10-16 20:37 ` Junio C Hamano
1 sibling, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2026-10-16 20:37 UTC (permalink / raw)
To: Miklos Vajna; +Cc: SZEDER Gábor, Patrick Steinhardt, git
Miklos Vajna <vmiklos@collabora.com> writes:
> +/* Compute a path to follow in each parent of a merge */
> +static void propagate_follow_pathspec_to_parents(struct rev_info *opt,
> + struct commit *commit,
> + struct commit_list *parents)
> +{
> + const char *path = pathspec_single_path(&opt->diffopt.pathspec);
> + struct commit_list *p;
> + enum follow_path_change *changes;
> + int nr = commit_list_count(parents), i;
> + int has_same = 0;
> +
> + if (!path)
> + return;
> +
> + ALLOC_ARRAY(changes, nr);
> + for (p = parents, i = 0; p; p = p->next, i++) {
> + changes[i] = get_follow_path_change(opt, commit, p->item);
> + if (changes[i] == FOLLOW_PATH_SAME) {
> + has_same = 1;
> + break;
> + }
> + }
> +
> + for (p = parents, i = 0; p; p = p->next, i++) {
> + if (!has_same && changes[i] == FOLLOW_PATH_ADDED)
> + /* Never the same, added here: expensive rename detection */
> + propagate_follow_pathspec_to_parent(opt, commit, p->item);
> + else
> + remember_follow_pathspec(opt, p->item, path);
> + }
> + free(changes);
> +}
Hmph, so if you have two branches A and B that forked from a common
ancestor that had file F,
- A updates F in place
- B renames F to G in one commit
- B then updates the renamed G on in another commit
- A merges B but decides to keep the original name F and
original content, deleting G.
then because has_same would be the same, you do not call
propagate_follow_pathspec_to_parent() to prepare the traversal into
branch B's history when you cross the merge, even when we are not
simplifying the history?
That somehow sounds broken to me. Am I missing something?
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2] log: speed up --follow following renames for non-linear history
2026-10-15 16:57 ` Junio C Hamano
@ 2026-10-19 6:10 ` Miklos Vajna
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
0 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-10-19 6:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: SZEDER Gábor, Patrick Steinhardt, 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 propagate_follow_pathspec_to_parent() diffs the
current commit against all of its parents with rename following, and
this slows down 'git log --follow'.
Fix the problem by removing propagate_follow_pathspec_to_parent() and
instead recording the path to follow for each parent in the parent loop
of log_tree_diff(), which already calls diff_tree_oid() with rename
following, so for non-merge commits the diff that we show is reused.
Merges are not diffed by default, so in that case run the same loop, but
only to find the paths to follow, without showing anything. With -c,
--cc or --remerge-diff, merges are diffed, but not in this loop, so run
the loop for them as well and only show their merge diff after that,
using the path of the merge itself. Also, do this in two passes: first
look for a parent without rename following, and if the path is
unchanged against a parent, then just follow that history in all
parents. Only do the expensive 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.
Following the same path in all parents is not enough when the other side
renamed the path, though: if the merge kept F from one parent, but the
other parent renamed F to G and then modified G, then the commits
modifying G would be lost. Handle this by still doing the rename
following against a parent if that 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. 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.
Now the above use case takes 37s, somewhat close to the original cost.
Also add new testcases: the -c/--cc/--remerge-diff options and the
--no-ff merging case were working, but were untested, so the
optimization could have broken them. And cover following a rename on the
side the merge did not take as well.
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
Hi Junio,
On Thu, Oct 15, 2026 at 09:57:39AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> I am not sure if it may be cleaner to first rip out all the change
> introduced in 304812ed33 (log: improve --follow following renames
> for non-linear history, 2026-06-22) and then rethink the approach,
> instead of piling more code with additional patches. I dunno.
Here is my attempt at the redo approach: instead of adding more diffing
on top of 304812ed33, I removed its
propagate_follow_pathspec_to_parent() and I do the path recording in the
existing parent loop of log_tree_diff(). For merges, there is no diff
to reuse by default, so the same loop runs for them without showing
anything.
Most of the performance benefit from v1 is still there.
I also found a few cases which had no test coverage, I added tests to
t4219-log-follow-merge.sh.
On Fri, Oct 16, 2026 at 01:37:28PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Hmph, so if you have two branches A and B that forked from a common
> ancestor that had file F,
>
> - A updates F in place
> - B renames F to G in one commit
> - B then updates the renamed G on in another commit
> - A merges B but decides to keep the original name F and
> original content, deleting G.
>
> then because has_same would be the same, you do not call
> propagate_follow_pathspec_to_parent() to prepare the traversal into
> branch B's history when you cross the merge, even when we are not
> simplifying the history?
>
> That somehow sounds broken to me. Am I missing something?
I didn't consider that case. I've now added a follow_needs_renames() to
handle this.
This leads to a larger patch: should I extract the "rethink the
approach" and the new tests to separate patches or it doesn't matter?
To sum up, compared to v1, now I 1) redid the approach to avoid
propagate_follow_pathspec_to_parent() 2) added a few more tests and 3)
the above "following a rename on the side the merge did not take" case
now works as well.
Thanks,
Miklos
log-tree.c | 201 +++++++++++++++++++++++++-----------
t/t4219-log-follow-merge.sh | 112 ++++++++++++++++++++
2 files changed, 255 insertions(+), 58 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..d5ce58d91d 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,31 +1169,47 @@ 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)
+/*
+ * 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,
+ struct object_id *oid)
{
- 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);
+ 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);
}
/*
@@ -1188,10 +1220,12 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
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, *rename_parents = NULL;
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)
return 0;
@@ -1220,47 +1254,113 @@ 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->separate_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 && !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 (opt->diffopt.flags.follow_renames)
+ 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);
- log_tree_diff_flush(opt);
+ if (follow_only) {
+ changed = diff_queued_diff.nr;
+ diff_queue_clear(&diff_queued_diff);
+ opt->diffopt.found_follow = 0;
+ } else
+ log_tree_diff_flush(opt);
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 && 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 || opt->first_parent_merges)
break;
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ if (follow_only)
+ continue;
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)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ showed_log = log_tree_merge_diff(opt, commit, all_parents, oid);
+ }
+ commit_list_free(rename_parents);
+ free(path);
return showed_log;
}
@@ -1298,21 +1398,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;
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index e370f82955..9616ac64ed 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -126,4 +126,116 @@ 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_done
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v3 0/4] log: speed up --follow following renames for non-linear history
2026-10-19 6:10 ` [PATCH v2] " Miklos Vajna
@ 2026-10-22 20:45 ` Miklos Vajna
2026-10-22 20:45 ` [PATCH v3 1/4] t4219: test more --follow cases across merges Miklos Vajna
` (5 more replies)
0 siblings, 6 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-22 20:45 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Hi Junio,
On Mon, Oct 19, 2026 at 08:11:05AM +0200, Miklos Vajna <vmiklos@collabora.com> wrote:
> This leads to a larger patch: should I extract the "rethink the
> approach" and the new tests to separate patches or it doesn't matter?
Here is a series where I split the changes into 4 patches: add more
tests, a refactor, then your suggestion to start over (with one more
test) and finally the speedup.
Thanks,
Miklos
Miklos Vajna (4):
t4219: test more --follow cases across merges
log: extract showing the remerge/combined diff of a merge commit
log: simplify --follow following renames for non-linear history
log: speed up --follow following renames for non-linear history
log-tree.c | 201 +++++++++++++++++++++++++-----------
t/t4219-log-follow-merge.sh | 128 +++++++++++++++++++++++
2 files changed, 271 insertions(+), 58 deletions(-)
Interdiff against v2:
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index 9616ac64ed..c8474ed151 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 v3 1/4] t4219: test more --follow cases across merges
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
@ 2026-10-22 20:45 ` Miklos Vajna
2026-10-22 20:46 ` [PATCH v3 2/4] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
` (4 subsequent siblings)
5 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-22 20:45 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
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---
t/t4219-log-follow-merge.sh | 112 ++++++++++++++++++++++++++++++++++++
1 file changed, 112 insertions(+)
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index e370f82955..9616ac64ed 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -126,4 +126,116 @@ 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_done
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v3 2/4] log: extract showing the remerge/combined diff of a merge commit
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
2026-10-22 20:45 ` [PATCH v3 1/4] t4219: test more --follow cases across merges Miklos Vajna
@ 2026-10-22 20:46 ` Miklos Vajna
2026-10-22 20:46 ` [PATCH v3 3/4] log: simplify --follow following renames for non-linear history Miklos Vajna
` (3 subsequent siblings)
5 siblings, 0 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-22 20:46 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 v3 3/4] log: simplify --follow following renames for non-linear history
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
2026-10-22 20:45 ` [PATCH v3 1/4] t4219: test more --follow cases across merges Miklos Vajna
2026-10-22 20:46 ` [PATCH v3 2/4] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
@ 2026-10-22 20:46 ` Miklos Vajna
2026-10-26 11:40 ` Patrick Steinhardt
2026-10-22 20:47 ` [PATCH v3 4/4] log: speed up " Miklos Vajna
` (2 subsequent siblings)
5 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-10-22 20:46 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 fixing its performance would require adding even
more custom code, instead of just extending the existing code in
log_tree_diff().
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. This gets rid of the additional call to diff_tree_oid(). The
rework has no positive performance impact yet, it just simplifies the
code.
This also fixes 'git log -m --follow': -m diffs the merge against each
parent with rename following, so a rename found against the first parent
updated the followed path, and the second parent was then diffed with
the already renamed path. Also, propagate_follow_pathspec_to_parent()
started from that same updated path, so history was lost on the side of
the second parent. Now the path of the merge is restored before diffing
each parent. 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 | 84 +++++++++++++++++--------------------
t/t4219-log-follow-merge.sh | 16 +++++++
2 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index b4c97b86f5..c5ac6c2bcb 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,12 @@ 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)
return 0;
@@ -1240,35 +1215,67 @@ 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 (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;
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)
break;
+ if (path)
+ set_pathspec_to_single_path(&opt->diffopt.pathspec,
+ path);
+ if (follow_only)
+ continue;
log->parent = parents->item;
opt->loginfo = log;
}
+ 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;
}
@@ -1306,21 +1313,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;
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index 9616ac64ed..c8474ed151 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 v3 4/4] log: speed up --follow following renames for non-linear history
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
` (2 preceding siblings ...)
2026-10-22 20:46 ` [PATCH v3 3/4] log: simplify --follow following renames for non-linear history Miklos Vajna
@ 2026-10-22 20:47 ` Miklos Vajna
2026-10-26 11:40 ` Patrick Steinhardt
2026-10-26 11:42 ` [PATCH v3 0/4] " Patrick Steinhardt
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
5 siblings, 1 reply; 43+ messages in thread
From: Miklos Vajna @ 2026-10-22 20:47 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 | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 89 insertions(+), 4 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index c5ac6c2bcb..d5ce58d91d 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
@@ -1251,13 +1300,46 @@ 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 || opt->first_parent_merges)
break;
if (path)
@@ -1268,6 +1350,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)
@@ -1275,6 +1359,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 v3 3/4] log: simplify --follow following renames for non-linear history
2026-10-22 20:46 ` [PATCH v3 3/4] log: simplify --follow following renames for non-linear history Miklos Vajna
@ 2026-10-26 11:40 ` Patrick Steinhardt
0 siblings, 0 replies; 43+ messages in thread
From: Patrick Steinhardt @ 2026-10-26 11:40 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, git
On Thu, Oct 22, 2026 at 10:46:39PM +0200, Miklos Vajna wrote:
> 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 fixing its performance would require adding even
It's somewhat curious that you start talking about performance here
without even mentioning that performance is what breaks the current
approach. You mention contributing factors to the performance regression
without really calling it that.
> more custom code, instead of just extending the existing code in
> log_tree_diff().
>
> 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. This gets rid of the additional call to diff_tree_oid(). The
> rework has no positive performance impact yet, it just simplifies the
> code.
> This also fixes 'git log -m --follow': -m diffs the merge against each
> parent with rename following, so a rename found against the first parent
> updated the followed path, and the second parent was then diffed with
> the already renamed path.
So next to a performance regression there's also a bug? Interesting. I
feel like giving a bit more time to introduce and explain the bug would
have helped.
> Also, propagate_follow_pathspec_to_parent()
> started from that same updated path, so history was lost on the side of
> the second parent. Now the path of the merge is restored before diffing
> each parent. Add a test for this, which shows the full history of the
> diamond case with -m.
And another bug?
So in summary we have two bugs and one performance regression? The two
bugs are fixed by this change, and it also prepares us for the fix to
the performance regression? It might be easier to reason through
all of that if we were able to split this so that we fix one issue at a
time, but that may of course not be possible (or just very awkward).
> diff --git a/log-tree.c b/log-tree.c
> index b4c97b86f5..c5ac6c2bcb 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -1240,35 +1215,67 @@ 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 (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;
>
> 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);
Nit: style, if once condition body requires curly braces all of them
should have it. Sorry, I cannot really provide more valuable feedback on
this patch than this.
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3 4/4] log: speed up --follow following renames for non-linear history
2026-10-22 20:47 ` [PATCH v3 4/4] log: speed up " Miklos Vajna
@ 2026-10-26 11:40 ` Patrick Steinhardt
0 siblings, 0 replies; 43+ messages in thread
From: Patrick Steinhardt @ 2026-10-26 11:40 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, git
On Thu, Oct 22, 2026 at 10:47:06PM +0200, Miklos Vajna wrote:
> diff --git a/log-tree.c b/log-tree.c
> index c5ac6c2bcb..d5ce58d91d 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;
Doesn't this miss the reverse case where we have the path in "parent",
but not in "same"? So basically this test case, which is very similar to
the one you already added to 4219:
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
'
That test passes with HEAD~, but fails with HEAD.
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3 0/4] log: speed up --follow following renames for non-linear history
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
` (3 preceding siblings ...)
2026-10-22 20:47 ` [PATCH v3 4/4] log: speed up " Miklos Vajna
@ 2026-10-26 11:42 ` Patrick Steinhardt
2026-10-30 20:07 ` [PATCH v4 0/6] " Miklos Vajna
5 siblings, 0 replies; 43+ messages in thread
From: Patrick Steinhardt @ 2026-10-26 11:42 UTC (permalink / raw)
To: Miklos Vajna; +Cc: gitster, szeder.dev, git
On Thu, Oct 22, 2026 at 10:45:19PM +0200, Miklos Vajna wrote:
> Hi Junio,
>
> On Mon, Oct 19, 2026 at 08:11:05AM +0200, Miklos Vajna <vmiklos@collabora.com> wrote:
> > This leads to a larger patch: should I extract the "rethink the
> > approach" and the new tests to separate patches or it doesn't matter?
>
> Here is a series where I split the changes into 4 patches: add more
> tests, a refactor, then your suggestion to start over (with one more
> test) and finally the speedup.
Thanks for your work.
I cannot really provide much of value as I'm way out of my depth here,
but will try to do so anyway as the regression has now caused an
incident for GitLab.com. Hope to get this resolved sooner rather than
later so that I can ideally avoid backporting patches in favor of
getting this series landed.
Patrick
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v4 0/6] log: speed up --follow following renames for non-linear history
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
` (4 preceding siblings ...)
2026-10-26 11:42 ` [PATCH v3 0/4] " Patrick Steinhardt
@ 2026-10-30 20:07 ` Miklos Vajna
2026-10-30 20:07 ` [PATCH v4 1/6] t4219: test more --follow cases across merges Miklos Vajna
` (7 more replies)
5 siblings, 8 replies; 43+ messages in thread
From: Miklos Vajna @ 2026-10-30 20:07 UTC (permalink / raw)
To: gitster; +Cc: szeder.dev, ps, git
Hi Patrick,
Thanks for the reviews of v3. Changes in v4:
- The -m --follow fix is now a separate patch (3/6), so the simplify
patch (4/6) no longer changes behavior. The two bugs you pointed out
in the old commit message have one cause: the followed path is not
restored between the parents of a merge. So 3/6 fixes both. The
commit message of 4/6 now says upfront that 304812ed33 (log: improve
--follow following renames for non-linear history, 2026-06-22) caused
a performance regression.
- Fixed the brace style nit in the simplify patch.
- 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.
- 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.
Thanks,
Miklos
Miklos Vajna (6):
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: fix --follow following an edit on a side the merge did not take
log-tree.c | 265 +++++++++++++++++++++++++++++-------
t/t4219-log-follow-merge.sh | 171 +++++++++++++++++++++++
2 files changed, 388 insertions(+), 48 deletions(-)
Interdiff against v3:
diff --git a/log-tree.c b/log-tree.c
index d5ce58d91d..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;
}
@@ -1295,8 +1371,9 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
changed = diff_queued_diff.nr;
diff_queue_clear(&diff_queued_diff);
opt->diffopt.found_follow = 0;
- } else
+ } else {
log_tree_diff_flush(opt);
+ }
showed_log |= !opt->loginfo;
@@ -1312,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
@@ -1340,8 +1417,15 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
opt->diffopt.flags.follow_renames = 1;
parents = all_parents;
}
- if (!parents || opt->first_parent_merges)
+ if (!parents)
break;
+ 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,
path);
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index c8474ed151..a85fb78ca2 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -254,4 +254,47 @@ test_expect_success '--follow follows a rename on the side the merge did not tak
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_done
--
2.56.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [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
* [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 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 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 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 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
* 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
* 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
* [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
* [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
* [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 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
* 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
* 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
* 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
* [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
end of thread, other threads:[~2026-11-12 19:09 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 13:55 Performance regression in `git log --follow` Patrick Steinhardt
2026-10-13 21:24 ` SZEDER Gábor
2026-10-15 14:48 ` [PATCH] log: speed up --follow following renames for non-linear history Miklos Vajna
2026-10-15 16:57 ` Junio C Hamano
2026-10-19 6:10 ` [PATCH v2] " Miklos Vajna
2026-10-22 20:45 ` [PATCH v3 0/4] " Miklos Vajna
2026-10-22 20:45 ` [PATCH v3 1/4] t4219: test more --follow cases across merges Miklos Vajna
2026-10-22 20:46 ` [PATCH v3 2/4] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-10-22 20:46 ` [PATCH v3 3/4] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-10-26 11:40 ` Patrick Steinhardt
2026-10-22 20:47 ` [PATCH v3 4/4] log: speed up " Miklos Vajna
2026-10-26 11:40 ` Patrick Steinhardt
2026-10-26 11:42 ` [PATCH v3 0/4] " Patrick Steinhardt
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 ` [PATCH v4 3/6] log: fix -m --follow losing the history of later parents Miklos Vajna
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 ` [PATCH v4 5/6] log: speed up " Miklos Vajna
2026-11-06 14:14 ` Patrick Steinhardt
2026-11-09 8:02 ` Miklos Vajna
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
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
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-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
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 ` [PATCH v5 4/5] log: simplify --follow following renames for non-linear history 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
2026-11-11 10:43 ` Patrick Steinhardt
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
2026-11-12 19:09 ` Junio C Hamano
2026-11-11 16:44 ` [PATCH v5 0/5] log: speed up --follow following renames for non-linear history Junio C Hamano
2026-10-16 20:37 ` [PATCH] " Junio C Hamano
2026-10-14 6:16 ` Performance regression in `git log --follow` Miklos Vajna
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox