From: Lidong Yan <yldhome2d2@gmail.com>
Date: Sat, 18 Oct 2025 01:04:40 GMT
Subject: Re: Regression in `git diff --quiet HEAD` when a new file is staged
Message-ID: <918E56B8-7009-4E8E-A98E-AC5B9CE4DD7C@gmail.com>
In-Reply-To: <xmqq7bwt1kyf.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:
> 
> Jeff King <peff@peff.net> writes:
> 
>> Looking at that patch, my biggest concern is: are we missing other spots
>> that need to special-case the dry_run setting? Because it's a regression
>> in a maint release, I'm tempted to say we should do the dumbest possible
>> thing that covers all cases and just revert this hunk from the original
>> patch, like:
>> 
>> diff --git a/diff.c b/diff.c
>> index 87fa16b730..687206f353 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -6890,6 +6890,15 @@ void diff_flush(struct diff_options *options)
>> if (output_format & DIFF_FORMAT_NO_OUTPUT &&
>>   options->flags.exit_with_status &&
>>   options->flags.diff_from_contents) {
>> + /*
>> + * run diff_flush_patch for the exit status. setting
>> + * options->file to /dev/null should be safe, because we
>> + * aren't supposed to produce any output anyway.
>> + */
>> + diff_free_file(options);
>> + options->file = xfopen("/dev/null", "w");
>> + options->close_file = 1;
>> + options->color_moved = 0;
>> for (i = 0; i < q->nr; i++) {
>> struct diff_filepair *p = q->queue[i];
>> if (check_pair_status(p))
>> 
>> That would catch the bug here, as well as any others lurking. And it
>> converts any missing dry_run from correctness problems (we definitely
>> will not produce extra output) into optimization problems (we might emit
>> data we do not need, but we can fix those separately). At least for the
>> normal code paths. I think without those extra fixes the problems that
>> b55e6d36eb tried to fix for "-I" would still be observable, but at least
>> its fixes could not regress the other code paths.
> 
> Ahh.  I like this "stupid but cannot be incorrect" version even
> better than the original one that introduced the "dry run" mode.
> 
> But once we go in that direction, do we still need the dry-run
> machinery with diff_flush_patch_quietly() helper function?

I believe we can move Peff’s code from diff_flush() to diff_flush_patch_quiet().
However, I'm unsure whether we should remove the dry-run logic. In dry-run
mode, we would halt as early as possible in xdl_diff by using quick_consume().