From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Jake Zimmerman <jake@zimmerman.io>,
Lidong Yan <yldhome2d2@gmail.com>,
git@vger.kernel.org
Subject: Re: Regression in `git diff --quiet HEAD` when a new file is staged
Date: Tue, 21 Oct 2025 07:38:03 -0700 [thread overview]
Message-ID: <xmqqy0p4wcac.fsf@gitster.g> (raw)
In-Reply-To: <20251021073640.GB259661@coredump.intra.peff.net> (Jeff King's message of "Tue, 21 Oct 2025 03:36:40 -0400")
Jeff King <peff@peff.net> writes:
>> 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))
next prev parent reply other threads:[~2025-10-21 14:38 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-17 0:09 Regression in `git diff --quiet HEAD` when a new file is staged Jake Zimmerman
2025-10-17 7:51 ` Jeff King
2025-10-17 8:36 ` [PATCH] diff: restore redirection to /dev/null for diff_from_contents Jeff King
2025-10-17 18:22 ` Junio C Hamano
2025-10-19 21:09 ` Johannes Schindelin
2025-10-21 7:52 ` Jeff King
2025-10-17 11:44 ` Regression in `git diff --quiet HEAD` when a new file is staged Johannes Schindelin
2025-10-17 17:45 ` Junio C Hamano
2025-10-18 1:04 ` Lidong Yan
2025-10-18 9:42 ` Jeff King
2025-10-18 9:40 ` Jeff King
2025-10-18 15:23 ` Junio C Hamano
2025-10-21 7:36 ` Jeff King
2025-10-21 14:38 ` Junio C Hamano [this message]
2025-10-22 4:46 ` Lidong Yan
2025-10-22 9:14 ` Jeff King
2025-10-22 14:20 ` Lidong Yan
2025-10-22 14:31 ` Junio C Hamano
2025-10-22 16:28 ` Junio C Hamano
2025-10-22 9:11 ` Jeff King
2025-10-22 16:48 ` Junio C Hamano
2025-10-23 12:01 ` Jeff King
2025-10-23 12:15 ` Jeff King
2025-10-23 13:35 ` Junio C Hamano
2025-10-22 17:39 ` Junio C Hamano
2025-10-23 0:33 ` Lidong Yan
2025-10-23 13:42 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy0p4wcac.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jake@zimmerman.io \
--cc=peff@peff.net \
--cc=yldhome2d2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).