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
Junio C Hamano <gitster@pobox.com>
Date
Oct 23, 2025, 13:35 UTC
Message-ID
<xmqqms5hwxkm.fsf@gitster.g>
In-Reply-To
<20251023120101.GA1123594@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 10 quoted lines
> That is because you are trying to redirect to /dev/null once at the
> beginning of the loop. But the loop is effectively:
>
>   for each pair
>     check for content changes with diff_flush_patch_quietly();
>     output actual pair data with flush_one_pair();
>
> We want the redirection to /dev/null for the first part of the loop
> body, but not the second. So you have to do the redirection inside the
> loop.
Yeah, my bad.  Lidong noticed the same thing.
Show 52 quoted lines
> I agree that opening /dev/null over and over is silly. But we can reuse
> the same filehandle for each one. I.e., like:
>
> diff --git a/diff.c b/diff.c
> index dac3ea9e01..e903afcf04 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -6835,11 +6835,11 @@ void diff_flush(struct diff_options *options)
>  		/*
>  		 * make sure diff_Flush_patch_quietly() to be silent.
>  		 */
> -		FILE *saved_file = options->file;
> +		FILE *dev_null = NULL;
>  		int saved_color_moved = options->color_moved;
>  
>  		if (options->flags.diff_from_contents) {
> -			options->file = xfopen("/dev/null", "w");
> +			dev_null = xfopen("/dev/null", "w");
>  			options->color_moved = 0;
>  		}
>  		for (i = 0; i < q->nr; i++) {
> @@ -6848,15 +6848,20 @@ void diff_flush(struct diff_options *options)
>  			if (!check_pair_status(p))
>  				continue;
>  
> -			if (options->flags.diff_from_contents &&
> -			    !diff_flush_patch_quietly(p, options))
> -				continue;
> +			if (options->flags.diff_from_contents) {
> +				FILE *saved_file = options->file;
> +				int r;
> +				options->file = dev_null;
> +				r = diff_flush_patch_quietly(p, options);
> +				options->file = saved_file;
> +				if (!r)
> +					continue;
> +			}
>  
>  			flush_one_pair(p, options);
>  		}
>  		if (options->flags.diff_from_contents) {
> -			fclose(options->file);
> -			options->file = saved_file;
> +			fclose(dev_null);
>  			options->color_moved = saved_color_moved;
>  		}
>  		separator++;
>
> You could even imagine diff_flush_patch_quietly() saving the /dev/null
> descriptor in a static variable and effectively leaking it (or if we
> want to be more structured, cached inside the diff_options struct). And
> then the callers do not have to worry about it at all.

That would be bigger change than a regression fix warrants, so let's leave it out, but let me use the above to replace my botched attempt.

Thanks, both of you.
Previous: Jeff KingNext: Junio C Hamano
Message 24 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.