From: D. Ben Knoble Date: Sun, 09 Nov 2025 17:34:54 GMT Subject: Re: [PATCH v2] diff: disable rename detection with --quiet Message-ID: In-Reply-To: <8796cd59-2335-4674-823d-d682ce7b7f8e@web.de> On Sun, Nov 9, 2025 at 11:43 AM René Scharfe wrote: > > Detecting renames and copies improves diff's output. This effort is > wasted if we don't show any. Disable detection in that case. Indeed. I've confirmed this and v1 both fix the issue, although v2 is significantly faster (which is great for the intended use in git-prompt.sh!): λ hyperfine -NiP v 1 2 ~/code/git/'buildv{v}/git diff --cached --quiet --no-ext-diff --find-copies-harder' Benchmark 1: /home/benknoble/code/git/buildv1/git diff --cached --quiet --no-ext-diff --find-copies-harder Time (mean ± σ): 72.0 ms ± 3.3 ms [User: 45.2 ms, System: 26.2 ms] Range (min … max): 67.6 ms … 79.6 ms 42 runs Warning: Ignoring non-zero exit code. Benchmark 2: /home/benknoble/code/git/buildv2/git diff --cached --quiet --no-ext-diff --find-copies-harder Time (mean ± σ): 19.9 ms ± 1.5 ms [User: 8.9 ms, System: 10.6 ms] Range (min … max): 16.1 ms … 24.0 ms 151 runs Warning: Ignoring non-zero exit code. Summary /home/benknoble/code/git/buildv2/git diff --cached --quiet --no-ext-diff --find-copies-harder ran 3.61 ± 0.31 times faster than /home/benknoble/code/git/buildv1/git diff --cached --quiet --no-ext-diff --find-copies-harder > This actually fixes the error code when using the options --cached, > --find-copies-harder, --no-ext-diff and --quiet together: > run_diff_index() indirectly calls diff-lib.c::show_modified(), which > queues even non-modified entries using diff_change() because we need > them for copy detection. diff_change() sets flags.has_changes, though, > which causes diff_can_quit_early() to declare we're done after seeing > only the very first entry -- way too soon. This does describe the behavior I saw, but it seems to me that, if we have changes, then we ought to be able to quit early for --quiet, no? So there's some other knock-on effect that causes quitting early to be wrong here, and I'm not exactly sure what it is (other than the diff queues being different sizes when we hit relevant parts of diffcore_std, though it's the working case that has the larger queue). So I'm having a hard time tying this paragraph to the actual issue (mostly due to my complete unfamiliarity with the diffing subsystem). > Using --cached, --find-copies-harder and --quiet together without > --no-ext-diff was not affected even before, as it causes the flag > flags.diff_from_contents to be set, which disables the optimization > in a different way. > > Reported-by: D. Ben Knoble > Suggested-by: Phillip Wood > Signed-off-by: René Scharfe > --- > diff.c | 2 ++ > t/t4007-rename-3.sh | 10 ++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/diff.c b/diff.c > index a1961526c0..efa8d9773c 100644 > --- a/diff.c > +++ b/diff.c > @@ -4987,6 +4987,8 @@ void diff_setup_done(struct diff_options *options) > if (options->flags.quick) { > options->output_format = DIFF_FORMAT_NO_OUTPUT; > options->flags.exit_with_status = 1; > + options->detect_rename = 0; > + options->flags.find_copies_harder = 0; > } > > /* > diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh > index e8faf0dd2e..3fc81bcd76 100755 > --- a/t/t4007-rename-3.sh > +++ b/t/t4007-rename-3.sh > @@ -41,6 +41,16 @@ test_expect_success 'copy detection, cached' ' > compare_diff_raw current expected > ' > > +test_expect_success 'exit code of quiet copy detection' ' > + test_expect_code 1 \ > + git diff --quiet --cached --find-copies-harder $tree > +' > + > +test_expect_success 'exit code of quiet copy detection with --no-ext-diff' ' > + test_expect_code 1 \ > + git diff --quiet --cached --find-copies-harder --no-ext-diff $tree > +' > + > # In the tree, there is only path0/COPYING. In the cache, path0 and > # path1 both have COPYING and the latter is a copy of path0/COPYING. > # However when we say we care only about path1, we should just see > -- > 2.51.2 Covering both seems like the right move to me, thanks! -- D. Ben Knoble