Re: Regression in `git diff --quiet HEAD` when a new file is staged
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 21, 2025, 14:38 UTC
- Message-ID
- <xmqqy0p4wcac.fsf@gitster.g>
- In-Reply-To
- <20251021073640.GB259661@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 10 quoted lines
>> Both. We do not have to call flush_quietly() and can call the real >> thing with output disabled. The dry-run bit was only added to >> implement the flush_quietly() variant. If we lose the only caller >> to flush_quietly(), all of the supporting infrastructure can go. > > It's not the only caller, though. b55e6d36eb added another earlier in > diff_flush(), to handle --name-status, etc (which was its original > goal). That code possibly remains broken, even with my patch, and > would wait either on Lidong's dry-run fixes, or lifting the /dev/null > into the flush_quietly() function.
Ah, OK. That makes sense.
> So really, the regression fix should probably cover both of them (which > it would if we move the /dev/null redirection into the flush_quietly() > variant).
Do you mean something like this on top of your patch for 'maint', and the latest from Lidong to the 'master' front, then?
Having calls to this helper in two loops in one function looks a bit awkward but the conditions to enter these two loops are mutually exclusive, so it is not like we can remember the result of the calls we make in the first loop and reuse in the second loop, so this probably is the best we can do.
--- >8 --- Subject: diff: fix "-w -I<regex> --quiet"
An earlier fix made sure we stay quiet during "dry run" patch output taken for the purpose of choosing which filepairs should be shown, but the same helper function needs to be made silent when we iterate over the diff-queue to compute the exit status.
diff.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git c/diff.c w/diff.c index 9b8d658b9e..1492ae108f 100644 --- c/diff.c +++ w/diff.c @@ -6172,6 +6172,8 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o) run_diff(p, o); } +static void diff_free_file(struct diff_options *options); + /* return 1 if any change is found; otherwise, return 0 */ static int diff_flush_patch_quietly(struct diff_filepair *p, struct diff_options *o) { @@ -6179,6 +6181,15 @@ static int diff_flush_patch_quietly(struct diff_filepair *p, struct diff_options int saved_found_changes = o->found_changes; int ret; + /* + * 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(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); @@ -6876,15 +6887,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))