diff: send external diff output to diff_options.file

Diff output usually goes to the process stdout, but it can be redirected
with the "--output" option. We store this in the "file" pointer of
diff_options, and all of the diff code should write there instead of to
stdout.

But there's one spot we missed: running an external diff cmd. We don't
redirect its output at all, so it just defaults to the stdout of the
parent process. We should instead point its stdout at our output file.
There are a few caveats to watch out for when doing so:

  - The stdout field takes a descriptor, not a FILE pointer. We can pull
    out the descriptor with fileno().

  - The run-command API always closes the stdout descriptor we pass to
    it. So we must duplicate it (otherwise we break the FILE pointer,
    since it now points to a closed descriptor).

  - We don't need to worry about closing our dup'd descriptor, since the
    point is that run-command will do it for us (even in the case of an
    error). But we do need to make sure we skip the dup() if we set
    no_stdout (because then run-command will not look at it at all).

  - When the output is going to stdout, it would not be wrong to dup()
    the descriptor, but we don't need to. We can skip that extra work
    with a simple pointer comparison.

  - It seems like you'd need to fflush() the descriptor before handing
    off a copy to the child process to prevent out-of-order writes. But
    that was true even before this patch! It works because run-command
    always calls fflush(NULL) before running the child.

The new test shows the breakage (and fix). The need for duplicating the
descriptor doesn't need a new test; that is covered by the later test
"GIT_EXTERNAL_DIFF with more than one changed files".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 10 insertions, 0 deletions

diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index c8a23d5148..7ec5854f74 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -44,6 +44,16 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '
 
 '
 
+test_expect_success 'GIT_EXTERNAL_DIFF and --output' '
+	cat >expect <<-EOF &&
+	file $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644
+	EOF
+	GIT_EXTERNAL_DIFF=echo git diff --output=out >stdout &&
+	cut -d" " -f1,3- <out >actual &&
+	test_must_be_empty stdout &&
+	test_cmp expect actual
+'
+
 test_expect_success SYMLINKS 'typechange diff' '
 	rm -f file &&
 	ln -s elif file &&