git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Regression in `git diff --quiet HEAD` when a new file is staged

From
Lidong Yan <yldhome2d2@gmail.com>
Date
Oct 18, 2025, 01:04 UTC
Message-ID
<918E56B8-7009-4E8E-A98E-AC5B9CE4DD7C@gmail.com>
In-Reply-To
<xmqq7bwt1kyf.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 43 quoted lines
> 
> Jeff King <peff@peff.net> 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?

I believe we can move Peff’s code from diff_flush() to diff_flush_patch_quiet(). However, I'm unsure whether we should remove the dry-run logic. In dry-run mode, we would halt as early as possible in xdl_diff by using quick_consume().

Previous: Junio C HamanoNext: Jeff King
Message 9 of 27 in “Regression in `git diff --quiet HEAD` when a new file is staged”
  1. Jake ZimmermanOct 17, 2025
  2. Jeff KingOct 17, 2025
  3. diff: restore redirection to /dev/null for diff_from_contentsJeff King, Oct 17, 2025
  4. Junio C HamanoOct 17, 2025
  5. Johannes SchindelinOct 19, 2025
  6. Jeff KingOct 21, 2025
  7. Johannes SchindelinOct 17, 2025
  8. Junio C HamanoOct 17, 2025
  9. Lidong YanOct 18, 2025
  10. Jeff KingOct 18, 2025
  11. Jeff KingOct 18, 2025
  12. Junio C HamanoOct 18, 2025
  13. Jeff KingOct 21, 2025
  14. Junio C HamanoOct 21, 2025
  15. Lidong YanOct 22, 2025
  16. Jeff KingOct 22, 2025
  17. Lidong YanOct 22, 2025
  18. Junio C HamanoOct 22, 2025
  19. Junio C HamanoOct 22, 2025
  20. Jeff KingOct 22, 2025
  21. Junio C HamanoOct 22, 2025
  22. Jeff KingOct 23, 2025
  23. Jeff KingOct 23, 2025
  24. Junio C HamanoOct 23, 2025
  25. Junio C HamanoOct 22, 2025
  26. Lidong YanOct 23, 2025
  27. Junio C HamanoOct 23, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.