Re: Regression in `git diff --quiet HEAD` when a new file is staged
From: Junio C Hamano <hidden>
Date: 2025-10-22 16:28:23
Junio C Hamano [off-list ref] writes:
Lidong Yan [off-list ref] writes:quoted
quoted
+ 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))