From: Jeff King <peff@peff.net>
Date: Thu, 23 Oct 2025 12:30:55 GMT
Subject: Re: [PATCH v4] diff: stop output garbled message in dry run mode
Message-ID: <20251023123055.GA1160519@coredump.intra.peff.net>
In-Reply-To: <20251019163024.18939-1-yldhome2d2@gmail.com>

On Mon, Oct 20, 2025 at 12:30:24AM +0800, Lidong Yan wrote:

> @@ -4420,7 +4423,7 @@ static void run_external_diff(const struct external_diff *pgm,
>  {
>  	struct child_process cmd = CHILD_PROCESS_INIT;
>  	struct diff_queue_struct *q = &diff_queued_diff;
> -	int quiet = !(o->output_format & DIFF_FORMAT_PATCH);
> +	int quiet = !(o->output_format & DIFF_FORMAT_PATCH) || o->dry_run;
>  	int rc;
>  
>  	/*

BTW, this hunk is interesting because it is the one spot (that we know
of!) which cannot be found by looking for mentions of o->file. But I
think that is a sign that it was already buggy, because it is not
respecting o->file in the first place!

If I make a simple commit like this:

  git init
  echo old >file && git add file && git commit -m old
  echo new >file && git add file && git commit -m new

and then run this:

  git diff-tree --output=foo.out -p HEAD^ HEAD

I should get the diff in foo.out, and I do. But if I instead do:

  GIT_EXTERNAL_DIFF='echo doing diff:' \
    git diff-tree --output=foo.out -p --ext-diff HEAD^ HEAD

then the external diff output goes to stdout. Whoops.

AFAICT this has been the case since "--output" was added. So we don't
need to worry about it in the context of the current regression.

Probably the solution is something like:

diff --git a/diff.c b/diff.c
index dac3ea9e01..15ef06ac9e 100644
--- a/diff.c
+++ b/diff.c
@@ -4458,6 +4458,8 @@ static void run_external_diff(const struct external_diff *pgm,
 	diff_free_filespec_data(two);
 	cmd.use_shell = 1;
 	cmd.no_stdout = quiet;
+	fflush(o->file);
+	cmd.out = fileno(o->file);
 	rc = run_command(&cmd);
 	if (!pgm->trust_exit_code && rc == 0)
 		o->found_changes = 1;

but I didn't test it beyond seeing that it makes the command above work.

-Peff