Re: [PATCH] diff: restore redirection to /dev/null for diff_from_contents
From: Junio C Hamano <hidden>
Date: 2025-10-17 18:22:40
Jeff King [off-list ref] writes:
quoted
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:Here it is with a commit message and test, in case that is helpful. -- >8 -- Subject: [PATCH] diff: restore redirection to /dev/null for diff_from_contents ... I didn't test, but I also wondered if this might be necessary to avoid actual external diff programs from spewing to stdout. Looking at run_external_diff(), we do: int quiet = !(o->output_format & DIFF_FORMAT_PATCH); [...] cmd.no_stdout = quiet; so I _think_ it should be OK even without this patch. But again, I like the extra layer of protection here.
I do like this direction, in addition I really do appreciate your thought above on optimizing ext-diff and textconv away when they are not necessary (obviously outside the scope of the regression fix). I also wonder if we want to get rid of the new code related to the "dry-run" mode that we no longer have to use. But as a regression fix that wants to be minimum, I think this patch stops at the right place. Thanks.
quoted hunk
diff.c | 9 +++++++++ t/t4035-diff-quiet.sh | 4 ++++ 2 files changed, 13 insertions(+)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))diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh index 0352bf81a9..35eaf0855f 100755 --- a/t/t4035-diff-quiet.sh +++ b/t/t4035-diff-quiet.sh@@ -50,6 +50,10 @@ test_expect_success 'git diff-tree HEAD HEAD' ' test_expect_code 0 git diff-tree --quiet HEAD HEAD >cnt && test_line_count = 0 cnt ' +test_expect_success 'git diff-tree -w HEAD^ HEAD' ' + test_expect_code 1 git diff-tree --quiet -w HEAD^ HEAD >cnt && + test_line_count = 0 cnt +' test_expect_success 'git diff-files' ' test_expect_code 0 git diff-files --quiet >cnt && test_line_count = 0 cnt