From: Junio C Hamano Date: Fri, 17 Oct 2025 17:45:12 GMT Subject: Re: Regression in `git diff --quiet HEAD` when a new file is staged Message-ID: In-Reply-To: <20251017075153.GA4078773@coredump.intra.peff.net> Jeff King 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?