Skip to contentSkip to conversation

Performance regression in `git log --follow`

PATCH Superseded Patrick Steinhardt 6 participants 40 messages Oct 13 – Nov 11

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 11 lines · Patrick Steinhardt

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.

Quoted 5 lines

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:

log-tree.c +16 -1
index 83a3c4bf9b..fd4824fd3e 100644
@@ -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));
#2 lore raw

PATCH 1/1 log: speed up --follow following renames for non-linear history

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.

---

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(-)
log-tree.c +70 -4
index 83a3c4bf9b..0d7bf001f0 100644
@@ -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));
Signature · 1 line
2.56.0
#3 lore raw

Re: [PATCH] log: speed up --follow following renames for non-linear history

Quoted 9 lines · Miklos Vajna

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'.

Quoted 8 lines

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.

Quoted 95 lines
 log-tree.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 70 insertions(+), 4 deletions(-)
log-tree.c +70 -4
index 83a3c4bf9b..0d7bf001f0 100644
@@ -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));
#4 lore raw

PATCH v2 1/1 log: speed up --follow following renames for non-linear history

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.

---

Hi Junio,

Quoted 4 lines · Junio C Hamano

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.

Quoted 9 lines · Junio C Hamano

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.

Quoted 6 lines

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(-)
log-tree.c +143 -58
index 83a3c4bf9b..d5ce58d91d 100644
@@ -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;
t/t4219-log-follow-merge.sh +112 -0

116 lines not on this page · raw message

Signature · 1 line
2.56.0
#5 lore raw

v3 cover log: speed up --follow following renames for non-linear history

Hi Junio,

Quoted 2 lines · Miklos Vajna

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:

t/t4219-log-follow-merge.sh +16 -0
index 9616ac64ed..c8474ed151 100755
@@ -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.
Signature · 1 line
2.56.0
#6 lore raw

PATCH v3 1/4 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 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

---
 t/t4219-log-follow-merge.sh | 112 ++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)
t/t4219-log-follow-merge.sh +112 -0
index e370f82955..9616ac64ed 100755
@@ -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
Signature · 1 line
2.56.0
#7 lore raw

PATCH v3 2/4 log: extract showing the remerge/combined diff of a merge commit

In reply to Miklos Vajna

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.

---
 log-tree.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)
log-tree.c +22 -14
index 83a3c4bf9b..b4c97b86f5 100644
@@ -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 */
Signature · 1 line
2.56.0
#8 lore raw

PATCH v3 3/4 log: simplify --follow following renames for non-linear history

In reply to Miklos Vajna

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.

---
 log-tree.c                  | 84 +++++++++++++++++--------------------
 t/t4219-log-follow-merge.sh | 16 +++++++
 2 files changed, 54 insertions(+), 46 deletions(-)
log-tree.c +38 -46
index b4c97b86f5..c5ac6c2bcb 100644
@@ -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;
t/t4219-log-follow-merge.sh +16 -0
index 9616ac64ed..c8474ed151 100755
@@ -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.
Signature · 1 line
2.56.0
#9 lore raw

Re: [PATCH v3 3/4] log: simplify --follow following renames for non-linear history

Quoted 7 lines · Miklos Vajna

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.

Quoted 13 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

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).

Quoted 34 lines
log-tree.c +12 -2
index b4c97b86f5..c5ac6c2bcb 100644
@@ -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;

log-tree.c static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log · line 1250

Quoted 10 lines
log-tree.c +5 -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) {
+			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

#10 lore raw

PATCH v3 4/4 log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna

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.

---
 log-tree.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 89 insertions(+), 4 deletions(-)
log-tree.c +89 -4
index c5ac6c2bcb..d5ce58d91d 100644
@@ -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;
 }
Signature · 1 line
2.56.0
#11 lore raw

Re: [PATCH v3 4/4] log: speed up --follow following renames for non-linear history

Quoted 13 lines · Miklos Vajna
log-tree.c +5 -0
index c5ac6c2bcb..d5ce58d91d 100644
@@ -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.

log-tree.c void release_follow_pathspec_slab(struct rev_info *opt) · line 1186

Quoted 10 lines
log-tree.c +10 -0
+ */
+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

#12 lore raw

Re: [PATCH v3 0/4] log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna
Quoted 9 lines · Miklos Vajna

Hi Junio,

Quoted 2 lines · Miklos Vajna

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

#13 lore raw

v4 cover log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna

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:

log-tree.c +96 -12
index d5ce58d91d..c2c3473f25 100644
@@ -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);
t/t4219-log-follow-merge.sh +43 -0

47 lines not on this page · raw message

Signature · 1 line
2.56.0
#14 lore raw

PATCH v4 1/6 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 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

---
 t/t4219-log-follow-merge.sh | 124 ++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)
t/t4219-log-follow-merge.sh +124 -0
index e370f82955..c109c29f70 100755
@@ -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
Signature · 1 line
2.56.0
#15 lore raw

PATCH v4 2/6 log: extract showing the remerge/combined diff of a merge commit

In reply to Miklos Vajna

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.

---
 log-tree.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)
log-tree.c +22 -14
index 83a3c4bf9b..b4c97b86f5 100644
@@ -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 */
Signature · 1 line
2.56.0
#16 lore raw

PATCH v4 3/6 log: fix -m --follow losing the history of later parents

In reply to Miklos Vajna

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.

---
 log-tree.c                  | 13 +++++++++++++
 t/t4219-log-follow-merge.sh | 16 ++++++++++++++++
 2 files changed, 29 insertions(+)
log-tree.c +13 -0
index b4c97b86f5..befc5c8b22 100644
@@ -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;
 }
 
t/t4219-log-follow-merge.sh +16 -0
index c109c29f70..36204ae2bf 100755
@@ -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.
Signature · 1 line
2.56.0
#17 lore raw

PATCH v4 4/6 log: simplify --follow following renames for non-linear history

In reply to Miklos Vajna

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.

---
 log-tree.c | 96 +++++++++++++++++++++++-------------------------------
 1 file changed, 41 insertions(+), 55 deletions(-)
log-tree.c +41 -55
index befc5c8b22..0aa27fb210 100644
@@ -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;
Signature · 1 line
2.56.0
#18 lore raw

PATCH v4 5/6 log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna

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.

---
 log-tree.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 5 deletions(-)
log-tree.c +91 -5
index 0aa27fb210..0dc71be61e 100644
@@ -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;
 }
Signature · 1 line
2.56.0
#19 lore raw

Re: [PATCH v4 5/6] log: speed up --follow following renames for non-linear history

Quoted 20 lines · Miklos Vajna
log-tree.c +12 -0
index 0aa27fb210..0dc71be61e 100644
@@ -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;
+

log-tree.c void release_follow_pathspec_slab(struct rev_info *opt) · line 1193

Quoted 10 lines
log-tree.c +10 -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

#20 lore raw

Re: [PATCH v4 5/6] log: speed up --follow following renames for non-linear history

Hi Patrick,

Quoted 5 lines · Patrick Steinhardt

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

#21 lore raw

PATCH v4 6/6 log: fix --follow following an edit on a side the merge did not take

In reply to Miklos Vajna

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.

---
 log-tree.c                  | 96 +++++++++++++++++++++++++++++++++----
 t/t4219-log-follow-merge.sh | 31 ++++++++++++
 2 files changed, 117 insertions(+), 10 deletions(-)
log-tree.c +86 -10
index 0dc71be61e..c2c3473f25 100644
@@ -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
t/t4219-log-follow-merge.sh +31 -0
index 36204ae2bf..a85fb78ca2 100755
@@ -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
Signature · 1 line
2.56.0
#22 lore raw

Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take

Quoted 15 lines · Miklos Vajna

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.

Quoted 4 lines

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:

t/t4219-log-follow-merge.sh +32 -0
index a85fb78ca28..c67d9481ab5 100755
@@ -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

#23 lore raw

Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take

Hi Patrick,

Quoted 8 lines · Patrick Steinhardt
Quoted 4 lines

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

#24 lore raw

Re: [PATCH v4 0/6] log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna

Hi Miklos,

Quoted 3 lines · Miklos Vajna

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.

Quoted 6 lines

- 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.

Quoted 6 lines

- 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

#25 lore raw

Re: [PATCH v4 0/6] log: speed up --follow following renames for non-linear history

Hi Patrick,

Quoted 5 lines · Patrick Steinhardt

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.

Quoted 4 lines

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.

Quoted 5 lines
Quoted 1 line

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

#26 lore raw

v5 cover log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna

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 <[email protected]>
         Signed-off-by: Miklos Vajna <[email protected]>
     
      ## 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 <[email protected]>
     
      ## 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 <[email protected]>
    +    Now the above use case takes 42s, somewhat close to the original cost.
    +
    +    Helped-by: SZEDER Gábor <[email protected]>
    +    Helped-by: Patrick Steinhardt <[email protected]>
         Signed-off-by: Miklos Vajna <[email protected]>
     
      ## 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;
Signature · 1 line
2.51.0
#27 lore raw

PATCH v5 1/5 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 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

---
 t/t4219-log-follow-merge.sh | 214 ++++++++++++++++++++++++++++++++++++
 1 file changed, 214 insertions(+)
t/t4219-log-follow-merge.sh +214 -0
index e370f82955..806ccc24bd 100755
@@ -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
Signature · 1 line
2.51.0
#28 lore raw

Re: [PATCH v5 1/5] t4219: test more --follow cases across merges

Quoted 4 lines · Miklos Vajna

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:

Quoted 12 lines

- 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.

Quoted 22 lines

---
t/t4219-log-follow-merge.sh | 214 ++++++++++++++++++++++++++++++++++++

1 file changed, 214 insertions(+)
t/t4219-log-follow-merge.sh +0 -0
index e370f82955..806ccc24bd 100755
@@ -31,6 +31,33 @@ test_expect_success '--follow finds the pre-merge commit through a subtree merge
 	test_cmp expect actual

'

t/t4219-log-follow-merge.sh +7 -0
+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 &&

t/t4219-log-follow-merge.sh test_expect_success '--follow finds the pre-merge commit through a subtree merge · line 50

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+	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?

t/t4219-log-follow-merge.sh test_expect_success '--follow finds the pre-merge commit through a subtree merge · line 59

Quoted 9 lines
t/t4219-log-follow-merge.sh +9 -0
+
+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?

Quoted 15 lines

+
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 +0 -0
@@ -126,4 +153,191 @@ test_expect_success '--follow follows renames through a fork in a single history
 	test_cmp expect actual.sorted

'

t/t4219-log-follow-merge.sh +7 -0
+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

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 172

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+		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?

Quoted 31 lines
t/t4219-log-follow-merge.sh +31 -0
+
+# 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

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 213

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+		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.

Quoted 8 lines
t/t4219-log-follow-merge.sh +8 -0
+
+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

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 231

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+	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.

Quoted 14 lines
t/t4219-log-follow-merge.sh +14 -0
+
+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 &&

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 255

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+	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.

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 266

Quoted 11 lines
t/t4219-log-follow-merge.sh +11 -0
+
+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.

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 278

Quoted 12 lines
t/t4219-log-follow-merge.sh +12 -0
+
+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?

Quoted 11 lines
t/t4219-log-follow-merge.sh +11 -0
+
+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" &&

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 299

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+
+	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.

t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 309

Quoted 10 lines
t/t4219-log-follow-merge.sh +10 -0
+
+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.

Quoted 33 lines
t/t4219-log-follow-merge.sh +33 -0
+
+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
+'
+
Quoted 1 line

test_done

Ok, these tests are similar to last set, but copied instead of renamed.

-Justin

#29 lore raw

PATCH v5 2/5 log: extract showing the remerge/combined diff of a merge commit

In reply to Miklos Vajna

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.

---
 log-tree.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)
log-tree.c +22 -14
index 83a3c4bf9b..b4c97b86f5 100644
@@ -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 */
Signature · 1 line
2.51.0
#30 lore raw

PATCH v5 3/5 log: fix -m --follow losing the history of later parents

In reply to Miklos Vajna

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.

---
 log-tree.c                  | 13 +++++++++++++
 t/t4219-log-follow-merge.sh | 16 ++++++++++++++++
 2 files changed, 29 insertions(+)
log-tree.c +13 -0
index b4c97b86f5..befc5c8b22 100644
@@ -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;
 }
 
t/t4219-log-follow-merge.sh +16 -0
index 806ccc24bd..ac9ab85888 100755
@@ -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.
Signature · 1 line
2.51.0
#31 lore raw

PATCH v5 4/5 log: simplify --follow following renames for non-linear history

In reply to Miklos Vajna

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.

---
 log-tree.c | 96 +++++++++++++++++++++++-------------------------------
 1 file changed, 41 insertions(+), 55 deletions(-)
log-tree.c +41 -55
index befc5c8b22..0aa27fb210 100644
@@ -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;
Signature · 1 line
2.51.0
#32 lore raw

Re: [PATCH v5 4/5] log: simplify --follow following renames for non-linear history

Quoted 98 lines · Miklos Vajna

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.

---
 log-tree.c | 96 +++++++++++++++++++++++-------------------------------
 1 file changed, 41 insertions(+), 55 deletions(-)
log-tree.c +5 -30
index befc5c8b22..0aa27fb210 100644
@@ -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) {

log-tree.c static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log · line 1231

Quoted 10 lines
log-tree.c +4 -0
+		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)

Quoted 91 lines
log-tree.c +32 -25
 	}
 
-	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;
Signature · 1 line
2.51.0
Signature · 2 lines
Laters,
Toon
#33 lore raw

PATCH v5 5/5 log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna

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.

---
 log-tree.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 155 insertions(+), 4 deletions(-)
log-tree.c +155 -4
index 0aa27fb210..1dc02822f1 100644
@@ -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;
Signature · 1 line
2.51.0
#34 lore raw

Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna
Quoted 12 lines · Miklos Vajna

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.

Quoted 7 lines

- 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

#35 lore raw

Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history

Quoted 49 lines · Justin Tobler
Quoted 12 lines · Miklos Vajna

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.

Quoted 7 lines

- 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.

Quoted 4 lines

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.

Quoted 4 lines

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

#36 lore raw

Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history

Hi Patrick,

Quoted 8 lines · Patrick Steinhardt
Quoted 4 lines

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.

Quoted 9 lines

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.

Quoted 4 lines

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

#37 lore raw

Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history

Quoted 1 line · Miklos Vajna

On Wed, Nov 11, 2026 at 11:43:50AM +0100, Patrick Steinhardt <[email protected]> wrote:

[snip]

Quoted 10 lines
Quoted 4 lines

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

#38 lore raw

Re: [PATCH] log: speed up --follow following renames for non-linear history

In reply to Miklos Vajna
Quoted 22 lines · Miklos Vajna
log-tree.c +22 -0
+/* 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;
+		}
+	}

log-tree.c static void propagate_follow_pathspec_to_parent(struct rev_info *opt, · line 1249

Quoted 10 lines
log-tree.c +10 -0
+
+	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?

#39 lore raw
In reply to Patrick Steinhardt

Hi Patrick,

Quoted 5 lines · Patrick Steinhardt

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/[email protected]/>. 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