Add that ability in the form of the new configuration options
diff.trustExitCode and diff.<driver>.trustExitCode and the environment
variable GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE. They pair with the config
options diff.external and diff.<driver>.command and the environment
variable GIT_EXTERNAL_DIFF, respectively.
The new options are off by default, keeping the old behavior. Enabling
them indicates that the external diff returns exit code 1 if it finds
significant changes and 0 if it doesn't, like diff(1).
The name of the new options is taken from the git difftool and mergetool
options of similar purpose. (There they enable passing on the exit code
of a diff tool and to infer whether a merge done by a merge tool is
successful.)
The new feature sets the diff flag diff_from_contents in
diff_setup_done() if we need the exit code and are allowed to call
external diffs. This disables the optimization that avoids calling the
program with --quiet. Add it back by skipping the call if the external
evaluating the file-specific attributes in run_external_diff().
If we do run the external diff with --quiet, send its output to
/dev/null.
I considered checking the output of the external diff to check whether
its empty. It was added as 11be65cfa4 (diff: fix --exit-code with
external diff, 2024-05-05) and quickly reverted, as it does not work
with external diffs that do not write to stdout. There's no reason why
a graphical diff tool would even need to write anything there at all.
I also considered using a non-zero exit code for empty diffs, which
could be done without adding new configuration options. We'd need to
disable the optimization that allows git diff --quiet to skip calling
external diffs, though -- that might be quite surprising if graphical
exit codes compared to diff(1) and git diff --exit-code to the external
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Documentation/config/diff.txt | 18 +++++++++++++++++
Documentation/diff-options.txt | 6 +++++-
Documentation/git.txt | 10 +++++++++
Documentation/gitattributes.txt | 5 +++++
diff.c | 36 ++++++++++++++++++++++++++++++++-
t/t4020-diff-external.sh | 33 +++++++++++++++++++++---------
userdiff.c | 4 ++++
userdiff.h | 1 +
8 files changed, 101 insertions(+), 12 deletions(-)
@@ -79,6 +79,15 @@ diff.external::
you want to use an external diff program only on a subset of
your files, you might want to use linkgit:gitattributes[5] instead.
+diff.trustExitCode::
+ If this boolean value is set to true then the
+ `diff.external` command is expected to return exit code
+ 0 if it considers the input files to be equal or 1 if it
+ considers them to be different, like `diff(1)`.
+ If it is set to false, which is the default, then the command
+ is expected to return exit code 0 regardless of equality.
+ Any other exit code causes Git to report a fatal error.
+
diff.ignoreSubmodules::
Sets the default value of --ignore-submodules. Note that this
affects only 'git diff' Porcelain, and not lower level 'diff'
@@ -164,6 +173,15 @@ diff.<driver>.command::
The custom diff driver command. See linkgit:gitattributes[5]
for details.
+diff.<driver>.trustExitCode::
+ If this boolean value is set to true then the
+ `diff.<driver>.command` command is expected to return exit code
+ 0 if it considers the input files to be equal or 1 if it
+ considers them to be different, like `diff(1)`.
+ If it is set to false, which is the default, then the command
+ is expected to return exit code 0 regardless of equality.
+ Any other exit code causes Git to report a fatal error.
+
diff.<driver>.xfuncname::
The regular expression that the diff driver should use to
recognize the hunk header. A built-in pattern may also be used.
@@ -820,7 +820,11 @@ ifndef::git-log[]
--quiet::
Disable all output of the program. Implies `--exit-code`.
- Disables execution of external diff helpers.
+ Disables execution of external diff helpers whose exit code
+ is not trusted, i.e. their respective configuration option
+ `diff.trustExitCode` or `diff.<driver>.trustExitCode` or
+ environment variable `GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE` is
+ false.
endif::git-log[]
endif::git-format-patch[]
@@ -644,6 +644,16 @@ parameter, <path>.
For each path `GIT_EXTERNAL_DIFF` is called, two environment variables,
`GIT_DIFF_PATH_COUNTER` and `GIT_DIFF_PATH_TOTAL` are set.
+`GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE`::
+ If this Boolean environment variable is set to true then the
+ `GIT_EXTERNAL_DIFF` command is expected to return exit code
+ 0 if it considers the input files to be equal or 1 if it
+ considers them to be different, like `diff(1)`.
+ If it is set to false, which is the default, then the command
+ is expected to return exit code 0 regardless of equality.
+ Any other exit code causes Git to report a fatal error.
+
+
`GIT_DIFF_PATH_COUNTER`::
A 1-based counter incremented by one for every path.
@@ -776,6 +776,11 @@ with the above configuration, i.e. `j-c-diff`, with 7
parameters, just like `GIT_EXTERNAL_DIFF` program is called.
See linkgit:git[1] for details.
+If the program is able to ignore certain changes (similar to
+`git diff --ignore-space-change`), then also set the option
+`trustExitCode` to true. It is then expected to return exit code 1 if
+it finds significant changes and 0 if it doesn't.
+
Setting the internal diff algorithm
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -432,6 +432,10 @@ int git_diff_ui_config(const char *var, const char *value,
}
if (!strcmp(var, "diff.external"))
return git_config_string(&external_diff_cfg.cmd, var, value);
+ if (!strcmp(var, "diff.trustexitcode")) {
+ external_diff_cfg.trust_exit_code = git_config_bool(var, value);
+ return 0;
+ }
if (!strcmp(var, "diff.wordregex"))
return git_config_string(&diff_word_regex_cfg, var, value);
if (!strcmp(var, "diff.orderfile"))
@@ -556,6 +560,8 @@ static const struct external_diff *external_diff(void)
if (done_preparing)
return external_diff_ptr;
external_diff_env.cmd = xstrdup_or_null(getenv("GIT_EXTERNAL_DIFF"));
+ if (git_env_bool("GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE", 0))
+ external_diff_env.trust_exit_code = 1;
if (external_diff_env.cmd)
external_diff_ptr = &external_diff_env;
else if (external_diff_cfg.cmd)
@@ -4387,6 +4393,19 @@ 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 rc;
+
+ /*
+ * Trivial equality is handled by diff_unmodified_pair() before
+ * we get here. If we don't need to show the diff and the
+ * external diff program lacks the ability to tell us whether
+ * it's empty then we consider it non-empty without even asking.
+ */
+ if (!pgm->trust_exit_code && quiet) {
+ o->found_changes = 1;
+ return;
+ }
strvec_push(&cmd.args, pgm->cmd);
strvec_push(&cmd.args, name);
@@ -4408,7 +4427,15 @@ static void run_external_diff(const struct external_diff *pgm,
diff_free_filespec_data(one);
diff_free_filespec_data(two);
cmd.use_shell = 1;
- if (run_command(&cmd))
+ cmd.no_stdout = quiet;
+ rc = run_command(&cmd);
+ if (!pgm->trust_exit_code && rc == 0)
+ o->found_changes = 1;
+ else if (pgm->trust_exit_code && rc == 0)
+ ; /* nothing */
+ else if (pgm->trust_exit_code && rc == 1)
+ o->found_changes = 1;
+ else
die(_("external diff died, stopping at %s"), name);
remove_tempfile();
@@ -4926,6 +4953,13 @@ void diff_setup_done(struct diff_options *options)
options->flags.exit_with_status = 1;
}
+ /*
+ * External diffs could declare non-identical contents equal
+ * (think diff --ignore-space-change).
+ */
+ if (options->flags.allow_external && options->flags.exit_with_status)
+ options->flags.diff_from_contents = 1;
+
options->diff_path_counter = 0;
if (options->flags.follow_renames)
@@ -177,15 +177,17 @@ check_external_diff () {
expect_out=$2
expect_err=$3
command_code=$4
- shift 4
+ trust_exit_code=$5
+ shift 5
options="$@"
command="echo output; exit $command_code;"
- desc="external diff '$command'"
+ desc="external diff '$command' with trustExitCode=$trust_exit_code"
with_options="${options:+ with }$options"
test_expect_success "$desc via attribute$with_options" "
test_config diff.foo.command \"$command\" &&
+ test_config diff.foo.trustExitCode $trust_exit_code &&
echo \"file diff=foo\" >.gitattributes &&
test_expect_code $expect_code git diff $options >out 2>err &&
test_cmp $expect_out out &&
@@ -194,6 +196,7 @@ check_external_diff () {
test_expect_success "$desc via diff.external$with_options" "
test_config diff.external \"$command\" &&
+ test_config diff.trustExitCode $trust_exit_code &&
>.gitattributes &&
test_expect_code $expect_code git diff $options >out 2>err &&
test_cmp $expect_out out &&
@@ -204,6 +207,7 @@ check_external_diff () {
>.gitattributes &&
test_expect_code $expect_code env \
GIT_EXTERNAL_DIFF=\"$command\" \
+ GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE=$trust_exit_code \
git diff $options >out 2>err &&
test_cmp $expect_out out &&
test_cmp $expect_err err
@@ -216,14 +220,23 @@ test_expect_success 'setup output files' '
echo "fatal: external diff died, stopping at file" >error
'
-check_external_diff 0 output empty 0
-check_external_diff 128 output error 1
-
-check_external_diff 1 output empty 0 --exit-code
-check_external_diff 128 output error 1 --exit-code
-
-check_external_diff 1 empty empty 0 --quiet
-check_external_diff 1 empty empty 1 --quiet # we don't even call the program
+check_external_diff 0 output empty 0 off
+check_external_diff 128 output error 1 off
+check_external_diff 0 output empty 0 on
+check_external_diff 0 output empty 1 on
+check_external_diff 128 output error 2 on
+
+check_external_diff 1 output empty 0 off --exit-code
+check_external_diff 128 output error 1 off --exit-code
+check_external_diff 0 output empty 0 on --exit-code
+check_external_diff 1 output empty 1 on --exit-code
+check_external_diff 128 output error 2 on --exit-code
+
+check_external_diff 1 empty empty 0 off --quiet
+check_external_diff 1 empty empty 1 off --quiet # we don't even call the program
+check_external_diff 0 empty empty 0 on --quiet
+check_external_diff 1 empty empty 1 on --quiet
+check_external_diff 128 empty error 2 on --quiet
echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
@@ -446,6 +446,10 @@ int userdiff_config(const char *k, const char *v)
return parse_tristate(&drv->binary, k, v);
if (!strcmp(type, "command"))
return git_config_string(&drv->external.cmd, k, v);
+ if (!strcmp(type, "trustexitcode")) {
+ drv->external.trust_exit_code = git_config_bool(k, v);
+ return 0;
+ }
if (!strcmp(type, "textconv"))
return git_config_string(&drv->textconv, k, v);
if (!strcmp(type, "cachetextconv"))
@@ -13,6 +13,7 @@ struct userdiff_funcname {
struct external_diff {
char *cmd;
+ unsigned trust_exit_code:1;
};
struct userdiff_driver {