From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 22 Oct 2025 16:28:20 GMT
Subject: Re: Regression in `git diff --quiet HEAD` when a new file is staged
Message-ID: <xmqqms5izysb.fsf@gitster.g>
In-Reply-To: <xmqqa51j0zzj.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Lidong Yan <yldhome2d2@gmail.com> writes:
>
>>> + diff_free_file(o);
>>> + o->file = xfopen("/dev/null", "w");
>>> + o->close_file = 1;
>>> + o->color_moved = 0;
>>> o->dry_run = 1;
>>> o->found_changes = 0;
>>> diff_flush_patch(p, o);
>>> 
>>
>> This would make everything going to "/dev/null" after the flush_quietly() call.
>> I think we need to restore o->file.
>
> Ah, true, the original location was only for NO_OUTPUT but the other
> caller to the diff_flush_patch_quietly() helper does deal with other
> cases as well.

Now it turns out to be rather ugly, having to go back and forth on a
few members of the diff_options structure.  I suspect there are
members other than color_moved that would not affect the outcome
(like --word-diff and --color-words) that cost us without giving any
benefit in this context that we may want to disable, but that would
make it even uglier.

I am having second thoughts on this approach to move the redirection
to patch_quietly(), which means for N-path change, we end up /dev/null
redirection N times.  We have two callers, so we may be better off
having the redirection around the loops that contain these callers?

I dunno.


 diff.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git c/diff.c w/diff.c
index 9b8d658b9e..d28f69e5ce 100644
--- c/diff.c
+++ w/diff.c
@@ -6177,14 +6177,28 @@ static int diff_flush_patch_quietly(struct diff_filepair *p, struct diff_options
 {
 	int saved_dry_run = o->dry_run;
 	int saved_found_changes = o->found_changes;
+	int saved_color_moved = o->color_moved;
+	FILE *saved_file = o->file;
 	int ret;
 
+	/*
+	 * Do the dry-run check while sending output to /dev/null and
+	 * extra computation like color_moved that would not change
+	 * the final outcome disabled.
+	 */
+	o->file = xfopen("/dev/null", "w");
+	o->color_moved = 0;
 	o->dry_run = 1;
 	o->found_changes = 0;
+
 	diff_flush_patch(p, o);
 	ret = o->found_changes;
+	fclose((o->file);
+
 	o->dry_run = saved_dry_run;
 	o->found_changes |= saved_found_changes;
+	o->color_moved = saved_color_moved;
+	o->file = saved_file;
 	return ret;
 }
 
@@ -6876,15 +6890,6 @@ 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))