Git development
 help / color / mirror / Atom feed
From: Miklos Vajna <vmiklos@collabora.com>
To: gitster@pobox.com
Cc: ps@pks.im, jltobler@gmail.com, szeder.dev@gmail.com, git@vger.kernel.org
Subject: [PATCH] log: make --follow following renames for non-linear history optional
Date: Thu, 12 Nov 2026 09:05:07 +0000	[thread overview]
Message-ID: <avWBQ_10mHUNQcFi@collabora.com> (raw)
In-Reply-To: <avRtsm5VQpC_g9yN@pks.im>

Since commit 304812ed33 (log: improve --follow following renames for
non-linear history, 2026-06-22), 'git log --stat --follow --
include/linux/rsc_table.h' on linux.git takes 6m43s, while it took about
28 seconds before.

What happens is that log_tree_commit() diffs each merge against all of
its parents with rename following, to find the path to follow in each
parent. diff_tree_oid() only does the expensive rename detection when
the path is added relative to a parent, but that is common for merges,
e.g. when a branch forked before the path was created merges mainline.

Fix the problem by making this per-parent diffing opt-in: plain --follow
behaves as before 304812ed33 again, and 'git log --follow=merges'
follows the path separately in each parent of a merge. Add a matching
log.follow=merges configuration option as well.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---

Hi Junio,

On Wed, Nov 11, 2026 at 08:44:42AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> This sounds sensible, and is far better than reverting and declaring
> "The original 'checkbox hack' should stay here forever because doing
> it right will be very expensive".

If that is fine with you, I suggest just replacing the
mv/log-follow-speedup branch with this patch.

Hi Patrick,

On Wed, Nov 11, 2026 at 02:29:22PM +0100, Patrick Steinhardt <ps@pks.im> wrote:
> I don't really have a good sense of how far this patch series is on its
> road to getting merged. So how about we first land a single-patch series
> that introduces the knob that we can merge without the remaining parts
> in this series here?

I like the idea, here is a patch to do that.

> With regards to how to opt-in, I can see two ways:
> 
>   - We could extend "--follow" to take an optional parameter so that the
>     user can pass "--follow=merges". Same with "log.follow", we could
>     teach it another option "merges".
> 
>   - We could introduce "--follow-merges" and "log.followMerges".
> 
> I'm leaning towards the first option myself.

Yes, --follow=merges also appears more natural to me, so I implemented
it that way.

Thanks,

Miklos

 Documentation/config/log.adoc |  4 +++-
 Documentation/git-log.adoc    |  7 +++++--
 builtin/log.c                 | 15 +++++++++++++--
 diff.c                        | 15 +++++++++++----
 diff.h                        |  2 ++
 log-tree.c                    |  6 ++++--
 t/t4219-log-follow-merge.sh   | 32 +++++++++++++++++++++++++-------
 7 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/Documentation/config/log.adoc b/Documentation/config/log.adoc
index f7dfce69b5..72a3486a73 100644
--- a/Documentation/config/log.adoc
+++ b/Documentation/config/log.adoc
@@ -53,7 +53,9 @@ This is the same as the `--decorate` option of the `git log`.
 `log.follow`::
 	If `true`, `git log` will act as if the `--follow` option was used when
 	a single <path> is given.  This has the same limitations as `--follow`,
-	i.e. it cannot be used to follow multiple files.
+	i.e. it cannot be used to follow multiple files and does not work well
+	on non-linear history.  If `merges`, `git log` will act as if the
+	`--follow=merges` option was used.
 
 `log.graphColors`::
 	A list of colors, separated by commas, that can be used to draw
diff --git a/Documentation/git-log.adoc b/Documentation/git-log.adoc
index fb3ac11283..742b832bbf 100644
--- a/Documentation/git-log.adoc
+++ b/Documentation/git-log.adoc
@@ -27,9 +27,12 @@ each commit introduces are shown.
 OPTIONS
 -------
 
-`--follow`::
+`--follow[=merges]`::
 	Continue listing the history of a file beyond renames
-	(works only for a single file).
+	(works only for a single file). This does not work well on
+	non-linear history, unless `merges` is given: then the path is
+	followed separately for each parent of a merge, which is more
+	expensive.
 
 `--no-decorate`::
 `--decorate[=(short|full|auto|no)]`::
diff --git a/builtin/log.c b/builtin/log.c
index 350b35c556..db707877f6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -108,6 +108,7 @@ static int parse_decoration_style(const char *value)
 struct log_config {
 	int default_abbrev_commit;
 	int default_show_root;
+	/* 0: no, 1: yes, 2: merges */
 	int default_follow;
 	int default_show_signature;
 	int default_encode_email_headers;
@@ -205,6 +206,8 @@ static void cmd_log_init_defaults(struct rev_info *rev,
 		get_commit_format(cfg->fmt_pretty, rev);
 	if (cfg->default_follow)
 		rev->diffopt.flags.default_follow_renames = 1;
+	if (cfg->default_follow == 2)
+		rev->diffopt.flags.default_follow_merges = 1;
 	rev->verbose_header = 1;
 	init_diffstat_widths(&rev->diffopt);
 	rev->diffopt.flags.recursive = 1;
@@ -505,7 +508,10 @@ static int git_log_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "log.follow")) {
-		cfg->default_follow = git_config_bool(var, value);
+		if (value && !strcmp(value, "merges"))
+			cfg->default_follow = 2;
+		else
+			cfg->default_follow = git_config_bool(var, value);
 		return 0;
 	}
 	if (skip_prefix(var, "color.decorate.", &slot_name))
@@ -814,9 +820,14 @@ int cmd_log_reflog(int argc,
 
 static void log_setup_revisions_tweak(struct rev_info *rev)
 {
+	/* If --follow is not used, use the log.follow option. */
 	if (rev->diffopt.flags.default_follow_renames &&
-	    diff_check_follow_pathspec(&rev->prune_data, 0))
+	    !rev->diffopt.flags.follow_renames &&
+	    diff_check_follow_pathspec(&rev->prune_data, 0)) {
 		rev->diffopt.flags.follow_renames = 1;
+		rev->diffopt.flags.follow_merges =
+			rev->diffopt.flags.default_follow_merges;
+	}
 
 	if (rev->first_parent_only)
 		diff_merges_default_to_first_parent(rev);
diff --git a/diff.c b/diff.c
index 414532d09f..b633cbea09 100644
--- a/diff.c
+++ b/diff.c
@@ -5739,12 +5739,19 @@ static int diff_opt_follow(const struct option *opt,
 {
 	struct diff_options *options = opt->value;
 
-	BUG_ON_OPT_ARG(arg);
 	if (unset) {
 		options->flags.follow_renames = 0;
+		options->flags.follow_merges = 0;
 		options->flags.default_follow_renames = 0;
-	} else {
+		options->flags.default_follow_merges = 0;
+	} else if (!arg) {
 		options->flags.follow_renames = 1;
+		options->flags.follow_merges = 0;
+	} else if (!strcmp(arg, "merges")) {
+		options->flags.follow_renames = 1;
+		options->flags.follow_merges = 1;
+	} else {
+		return error(_("invalid argument to --follow: '%s'"), arg);
 	}
 	return 0;
 }
@@ -6161,9 +6168,9 @@ struct option *add_diff_options(const struct option *opts,
 			      0, PARSE_OPT_NONEG),
 		OPT_BOOL(0, "rename-empty", &options->flags.rename_empty,
 			 N_("use empty blobs as rename source")),
-		OPT_CALLBACK_F(0, "follow", options, NULL,
+		OPT_CALLBACK_F(0, "follow", options, N_("merges"),
 			       N_("continue listing the history of a file beyond renames"),
-			       PARSE_OPT_NOARG, diff_opt_follow),
+			       PARSE_OPT_OPTARG, diff_opt_follow),
 		OPT_INTEGER('l', NULL, &options->rename_limit,
 			    N_("limit to cheap rename/copy detection if the number of rename/copy targets exceeds this value")),
 
diff --git a/diff.h b/diff.h
index bb5cddaf34..2548739e83 100644
--- a/diff.h
+++ b/diff.h
@@ -154,6 +154,7 @@ struct diff_flags {
 	unsigned find_copies_harder;
 
 	unsigned follow_renames;
+	unsigned follow_merges;
 	unsigned rename_empty;
 
 	/* Internal; used for optimization to see if there is any change. */
@@ -202,6 +203,7 @@ struct diff_flags {
 	unsigned dirstat_by_line;
 	unsigned funccontext;
 	unsigned default_follow_renames;
+	unsigned default_follow_merges;
 	unsigned stat_with_summary;
 	unsigned suppress_diff_headers;
 	unsigned dual_color_diffed_diffs;
diff --git a/log-tree.c b/log-tree.c
index 83a3c4bf9b..93cc61af2d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1277,7 +1277,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	opt->diffopt.no_free = 1;
 
 	/* Any recorded path for this commit? If so, restore it */
-	if (opt->diffopt.flags.follow_renames) {
+	if (opt->diffopt.flags.follow_renames &&
+	    opt->diffopt.flags.follow_merges) {
 		const char *stored = recall_follow_pathspec(opt, commit);
 		if (stored) {
 			const char *current = pathspec_single_path(&opt->diffopt.pathspec);
@@ -1300,7 +1301,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 		show_diff_of_diff(opt);
 
 	/* Record what path each parent of this commit should use */
-	if (opt->diffopt.flags.follow_renames) {
+	if (opt->diffopt.flags.follow_renames &&
+	    opt->diffopt.flags.follow_merges) {
 		struct commit_list *parents = get_saved_parents(opt, commit);
 		if (parents && parents->next) {
 			struct commit_list *p;
diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index e370f82955..c2bc2b0c13 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='Test --follow follows renames across merges'
+test_description='Test --follow=merges follows renames across merges'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
@@ -25,12 +25,30 @@ test_expect_success 'setup subtree-merged repository' '
 	git -C outer commit -m "Merge inner repo into inner/ subdirectory"
 '
 
-test_expect_success '--follow finds the pre-merge commit through a subtree merge' '
-	git -C outer log --follow --pretty=tformat:%s inner/inner.txt >actual &&
+test_expect_success '--follow=merges finds the pre-merge commit through a subtree merge' '
+	git -C outer log --follow=merges --pretty=tformat:%s inner/inner.txt >actual &&
 	echo "inner init" >expect &&
 	test_cmp expect actual
 '
 
+test_expect_success 'log.follow=merges acts as --follow=merges' '
+	git -C outer -c log.follow=merges log --pretty=tformat:%s \
+		inner/inner.txt >actual &&
+	echo "inner init" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success '--follow overrides log.follow=merges' '
+	git -C outer -c log.follow=merges log --follow \
+		--pretty=tformat:%s inner/inner.txt >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success '--follow rejects an unknown argument' '
+	test_must_fail git -C outer log --follow=foo inner/inner.txt 2>err &&
+	test_grep "invalid argument to --follow: ${SQ}foo${SQ}" err
+'
+
 test_expect_success 'setup merge of two branches that both renamed a file to README' '
 	git init foo &&
 	mkdir foo/foo &&
@@ -65,8 +83,8 @@ test_expect_success 'setup merge of two branches that both renamed a file to REA
 	git -C foo commit -m "merge bar into foo"
 '
 
-test_expect_success '--follow follows renames across both sides of a merge' '
-	git -C foo log --follow --pretty=tformat:%s README >actual &&
+test_expect_success '--follow=merges follows renames across both sides of a merge' '
+	git -C foo log --follow=merges --pretty=tformat:%s README >actual &&
 	sort actual >actual.sorted &&
 	cat >expect <<-\EOF &&
 	add bar README
@@ -112,8 +130,8 @@ test_expect_success 'setup diamond with renames on both sides of a fork' '
 	git -C diamond commit -am "Z: modify path"
 '
 
-test_expect_success '--follow follows renames through a fork in a single history' '
-	git -C diamond log --follow --pretty=tformat:%s path >actual &&
+test_expect_success '--follow=merges follows renames through a fork in a single history' '
+	git -C diamond log --follow=merges --pretty=tformat:%s path >actual &&
 	sort actual >actual.sorted &&
 	cat >expect <<-\EOF &&
 	A: add path0
-- 
2.51.0


  reply	other threads:[~2026-11-12  9:05 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 13:55 Performance regression in `git log --follow` Patrick Steinhardt
2026-10-13 21:24 ` SZEDER Gábor
2026-10-15 14:48   ` [PATCH] log: speed up --follow following renames for non-linear history Miklos Vajna
2026-10-15 16:57     ` Junio C Hamano
2026-10-19  6:10       ` [PATCH v2] " Miklos Vajna
2026-10-22 20:45         ` [PATCH v3 0/4] " Miklos Vajna
2026-10-22 20:45           ` [PATCH v3 1/4] t4219: test more --follow cases across merges Miklos Vajna
2026-10-22 20:46           ` [PATCH v3 2/4] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-10-22 20:46           ` [PATCH v3 3/4] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-10-26 11:40             ` Patrick Steinhardt
2026-10-22 20:47           ` [PATCH v3 4/4] log: speed up " Miklos Vajna
2026-10-26 11:40             ` Patrick Steinhardt
2026-10-26 11:42           ` [PATCH v3 0/4] " Patrick Steinhardt
2026-10-30 20:07           ` [PATCH v4 0/6] " Miklos Vajna
2026-10-30 20:07             ` [PATCH v4 1/6] t4219: test more --follow cases across merges Miklos Vajna
2026-10-30 20:07             ` [PATCH v4 2/6] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-10-30 20:08             ` [PATCH v4 3/6] log: fix -m --follow losing the history of later parents Miklos Vajna
2026-10-30 20:08             ` [PATCH v4 4/6] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-10-30 20:09             ` [PATCH v4 5/6] log: speed up " Miklos Vajna
2026-11-06 14:14               ` Patrick Steinhardt
2026-11-09  8:02                 ` Miklos Vajna
2026-10-30 20:10             ` [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take Miklos Vajna
2026-11-06 14:14               ` Patrick Steinhardt
2026-11-09  8:03                 ` Miklos Vajna
2026-11-06 14:14             ` [PATCH v4 0/6] log: speed up --follow following renames for non-linear history Patrick Steinhardt
2026-11-09  8:01               ` Miklos Vajna
2026-11-09  8:03             ` [PATCH v5 0/5] " Miklos Vajna
2026-11-09  8:04               ` [PATCH v5 1/5] t4219: test more --follow cases across merges Miklos Vajna
2026-11-10 23:06                 ` Justin Tobler
2026-11-09  8:05               ` [PATCH v5 2/5] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-11-09  8:05               ` [PATCH v5 3/5] log: fix -m --follow losing the history of later parents Miklos Vajna
2026-11-09  8:05               ` [PATCH v5 4/5] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-11-10 16:45                 ` Toon Claes
2026-11-09  8:06               ` [PATCH v5 5/5] log: speed up " Miklos Vajna
2026-11-11  0:40               ` [PATCH v5 0/5] " Justin Tobler
2026-11-11 10:43                 ` Patrick Steinhardt
2026-11-11 12:41                   ` Miklos Vajna
2026-11-11 13:29                     ` Patrick Steinhardt
2026-11-12  9:05                       ` Miklos Vajna [this message]
2026-11-11 16:44                 ` Junio C Hamano
2026-10-16 20:37     ` [PATCH] " Junio C Hamano
2026-10-14  6:16 ` Performance regression in `git log --follow` Miklos Vajna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=avWBQ_10mHUNQcFi@collabora.com \
    --to=vmiklos@collabora.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=ps@pks.im \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox