Re: [PATCH v2] diff: disable rename detection with --quiet

> On Sun, Nov 9, 2025 at 11:43 AM René Scharfe <l.s.r@web.de> wrote:
>>
> λ 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 <ben.knoble@gmail.com>
>> Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>>   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
>