Re: Regression in `git diff --quiet HEAD` when a new file is staged
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 22, 2025, 16:28 UTC
- Message-ID
- <xmqqms5izysb.fsf@gitster.g>
- In-Reply-To
- <xmqqa51j0zzj.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> 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))