[Bug?] "git show -s" still worries about renameLimit?

[Bug?] "git show -s" still worries about renameLimit?

[Junio C Hamano]

$ git show -s | cat
warning: exhaustive rename detection was skipped due to too many files.
warning: you may want to set your diff.renameLimit variable to at least 6123 and retry the command.
commit a3a9dd8be6b8767e690b014715aefa2ba39672e2 (HEAD -> master)
Author: Junio C Hamano <gitster@pobox.com>
Date:   Sat Apr 19 14:27:03 2025 -0700

    Something something something

As we have -M (rename detection) on by default these days, and this
particular commit has very many deletions and creations, if we were
asking to show some diff (not necessarily patch text output, but
just "--stat" or even "--raw") it is fair to warn about rename
detection being limited by diff.renameLimit.

But the command knows that with "-s" the user declined to show any
diff computation, so it feels wrong to even _count_ how many
diff_filepairs there are and comparing with the renameLimit, in
order to warn about busting the limit.

Re: [Bug?] "git show -s" still worries about renameLimit?

[Jeff King]

On Sun, May 04, 2025 at 01:00:16AM -0700, Junio C Hamano wrote:

> $ git show -s | cat
> warning: exhaustive rename detection was skipped due to too many files.
> warning: you may want to set your diff.renameLimit variable to at least 6123 and retry the command.
> commit a3a9dd8be6b8767e690b014715aefa2ba39672e2 (HEAD -> master)
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Sat Apr 19 14:27:03 2025 -0700
> 
>     Something something something
> 
> As we have -M (rename detection) on by default these days, and this
> particular commit has very many deletions and creations, if we were
> asking to show some diff (not necessarily patch text output, but
> just "--stat" or even "--raw") it is fair to warn about rename
> detection being limited by diff.renameLimit.
> 
> But the command knows that with "-s" the user declined to show any
> diff computation, so it feels wrong to even _count_ how many
> diff_filepairs there are and comparing with the renameLimit, in
> order to warn about busting the limit.

This seemed eerily familiar. See this thread:

  https://lore.kernel.org/git/87h750q1b9.fsf@gnu.org/

and in particular this proposal:

  https://lore.kernel.org/git/YqI%2FTcZyXomxtXtN@coredump.intra.peff.net/

I've been carrying that patch in my tree (reproduced below), but I don't
remember why I never polished it. I wonder if it was the question about
--exit-code below. Or maybe I was just nervous about other corner cases.

-- >8 --
Subject: [PATCH] show: skip diff when possible

Running:

  git show -s $commit

will still compute a diff for $commit, even though we aren't going to
show it. This is wasted computation, since it cannot affect the output
or exit code of the program.

In the more general case:

  - if the requested diff format is NO_OUTPUT, then we won't change the
    output of the diff itself

  - if rev_info.always_show_header is set, then we will show the commit
    regardless of whether the diff is empty (which is true for git-show,
    for example, but not git-log)

  - we don't use --exit-code here (should check?)

Signed-off-by: Jeff King <peff@peff.net>
---
 log-tree.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/log-tree.c b/log-tree.c
index a4d4ab59ca..740219ce99 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1105,6 +1105,10 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	if (!all_need_diff && !opt->merges_need_diff)
 		return 0;
 
+	if (opt->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT &&
+	    opt->always_show_header)
+		return 0;
+
 	parse_commit_or_die(commit);
 	oid = get_commit_tree_oid(commit);
 
-- 
2.49.0.754.gd827f9aa09