diff: fix --exit-code with external diff

You can ask the diff machinery to let the exit code indicate whether there are changes, e.g. with --exit-code. It as two ways to calculate that bit: The quick one assumes blobs with different hashes have different content, and the more elaborate way actually compares the contents, possibly applying transformations like ignoring whitespace. Always use the slower path by setting the flag diff_from_contents, because any of the files could have an external diff driver set via an attribute, which might consider binary differences irrelevant, like e.g. Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed May 5, 2024 at 12:20 UTC 11be65cfa43416219e85384a3a80d672b65b76ba
2 files changed +38 -3
diff.c
+30 -3
@@ -40,6 +40,7 @@
40 #include "setup.h"
41 #include "strmap.h"
42 #include "ws.h"
43 +#include "write-or-die.h"
44
45 #ifdef NO_FAST_WORKING_DIRECTORY
46 #define FAST_WORKING_DIRECTORY 0
@@ -4396,8 +4397,33 @@ static void run_external_diff(const char *pgm,
4397 diff_free_filespec_data(one);
4398 diff_free_filespec_data(two);
4399 cmd.use_shell = 1;
4399 - if (run_command(&cmd))
4400 - die(_("external diff died, stopping at %s"), name);
4400 + if (o->flags.diff_from_contents) {
4401 + int got_output = 0;
4402 + cmd.out = -1;
4403 + if (start_command(&cmd))
4404 + die(_("external diff died, stopping at %s"), name);
4405 + for (;;) {
4406 + char buffer[8192];
4407 + ssize_t len = xread(cmd.out, buffer, sizeof(buffer));
4408 + if (!len)
4409 + break;
4410 + if (len < 0)
4411 + die(_("unable to read from external diff,"
4412 + " stopping at %s"), name);
4413 + got_output = 1;
4414 + if (write_in_full(1, buffer, len) < 0)
4415 + die(_("unable to write output of external diff,"
4416 + " stopping at %s"), name);
4417 + }
4418 + close(cmd.out);
4419 + if (finish_command(&cmd))
4420 + die(_("external diff died, stopping at %s"), name);
4421 + if (got_output)
4422 + o->found_changes = 1;
4423 + } else {
4424 + if (run_command(&cmd))
4425 + die(_("external diff died, stopping at %s"), name);
4426 + }
4427
4428 remove_tempfile();
4429 }
@@ -4844,6 +4870,7 @@ void diff_setup_done(struct diff_options *options)
4870 */
4871
4872 if ((options->xdl_opts & XDF_WHITESPACE_FLAGS) ||
4873 + options->flags.exit_with_status ||
4874 options->ignore_regex_nr)
4875 options->flags.diff_from_contents = 1;
4876 else
@@ -6732,7 +6759,7 @@ void diff_flush(struct diff_options *options)
6759 if (output_format & DIFF_FORMAT_CALLBACK)
6760 options->format_callback(q, options, options->format_callback_data);
6761
6735 - if (output_format & DIFF_FORMAT_NO_OUTPUT &&
6762 + if ((!output_format || output_format & DIFF_FORMAT_NO_OUTPUT) &&
6763 options->flags.exit_with_status &&
6764 options->flags.diff_from_contents) {
6765 /*
t/t4020-diff-external.sh
+8
@@ -172,6 +172,14 @@ test_expect_success 'no diff with -diff' '
172 grep Binary out
173 '
174
175 +test_expect_success 'diff.external and --exit-code with output' '
176 + test_expect_code 1 git -c diff.external=echo diff --exit-code
177 +'
178 +
179 +test_expect_success 'diff.external and --exit-code without output' '
180 + git -c diff.external=true diff --exit-code
181 +'
182 +
183 echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
184
185 test_expect_success 'force diff with "diff"' '