@matheusbsbs / matheus-bagliano / commits / d7b97b7185

diff: let external diffs report that changes are uninteresting

The options --exit-code and --quiet instruct git diff to indicate whether it found any significant changes by exiting with code 1 if it did and 0 if there were none. Currently this doesn't work if external diff programs are involved, as we have no way to learn what they found. 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 diff is not able to report empty diffs. We can only do that check after 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 diff programs are involved. And assigning the opposite meaning of the exit codes compared to diff(1) and git diff --exit-code to the external diff can cause unnecessary confusion. Suggested-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Jun 9, 2024 at 09:41 UTC d7b97b7185521e3b9364b3abc6553df2480da173

8 files changed +101 -12

Documentation/config/diff.txt

+18

@@ -79,6 +79,15 @@ diff.external::

79 you want to use an external diff program only on a subset of

80 your files, you might want to use linkgit:gitattributes[5] instead.

82 +diff.trustExitCode::

83 + If this boolean value is set to true then the

84 + `diff.external` command is expected to return exit code

85 + 0 if it considers the input files to be equal or 1 if it

86 + considers them to be different, like `diff(1)`.

87 + If it is set to false, which is the default, then the command

88 + is expected to return exit code 0 regardless of equality.

89 + Any other exit code causes Git to report a fatal error.

90 +

91 diff.ignoreSubmodules::

92 Sets the default value of --ignore-submodules. Note that this

93 affects only 'git diff' Porcelain, and not lower level 'diff'

@@ -164,6 +173,15 @@ diff.<driver>.command::

173 The custom diff driver command. See linkgit:gitattributes[5]

174 for details.

175

176 +diff.<driver>.trustExitCode::

177 + If this boolean value is set to true then the

178 + `diff.<driver>.command` command is expected to return exit code

179 + 0 if it considers the input files to be equal or 1 if it

180 + considers them to be different, like `diff(1)`.

181 + If it is set to false, which is the default, then the command

182 + is expected to return exit code 0 regardless of equality.

183 + Any other exit code causes Git to report a fatal error.

184 +

185 diff.<driver>.xfuncname::

186 The regular expression that the diff driver should use to

187 recognize the hunk header. A built-in pattern may also be used.

Documentation/diff-options.txt

+5 -1

@@ -820,7 +820,11 @@ ifndef::git-log[]

820

821 --quiet::

822 Disable all output of the program. Implies `--exit-code`.

823 - Disables execution of external diff helpers.

823 + Disables execution of external diff helpers whose exit code

824 + is not trusted, i.e. their respective configuration option

825 + `diff.trustExitCode` or `diff.<driver>.trustExitCode` or

826 + environment variable `GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE` is

827 + false.

828 endif::git-log[]

829 endif::git-format-patch[]

830

Documentation/git.txt

+10

@@ -644,6 +644,16 @@ parameter, <path>.

644 For each path `GIT_EXTERNAL_DIFF` is called, two environment variables,

645 `GIT_DIFF_PATH_COUNTER` and `GIT_DIFF_PATH_TOTAL` are set.

646

647 +`GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE`::

648 + If this Boolean environment variable is set to true then the

649 + `GIT_EXTERNAL_DIFF` command is expected to return exit code

650 + 0 if it considers the input files to be equal or 1 if it

651 + considers them to be different, like `diff(1)`.

652 + If it is set to false, which is the default, then the command

653 + is expected to return exit code 0 regardless of equality.

654 + Any other exit code causes Git to report a fatal error.

655 +

656 +

657 `GIT_DIFF_PATH_COUNTER`::

658 A 1-based counter incremented by one for every path.

659

Documentation/gitattributes.txt

@@ -776,6 +776,11 @@ with the above configuration, i.e. `j-c-diff`, with 7

776 parameters, just like `GIT_EXTERNAL_DIFF` program is called.

777 See linkgit:git[1] for details.

778

779 +If the program is able to ignore certain changes (similar to

780 +`git diff --ignore-space-change`), then also set the option

781 +`trustExitCode` to true. It is then expected to return exit code 1 if

782 +it finds significant changes and 0 if it doesn't.

783 +

784 Setting the internal diff algorithm

785 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

786

diff.c

+35 -1

@@ -432,6 +432,10 @@ int git_diff_ui_config(const char *var, const char *value,

432 }

433 if (!strcmp(var, "diff.external"))

434 return git_config_string(&external_diff_cfg.cmd, var, value);

435 + if (!strcmp(var, "diff.trustexitcode")) {

436 + external_diff_cfg.trust_exit_code = git_config_bool(var, value);

437 + return 0;

438 + }

439 if (!strcmp(var, "diff.wordregex"))

440 return git_config_string(&diff_word_regex_cfg, var, value);

441 if (!strcmp(var, "diff.orderfile"))

@@ -556,6 +560,8 @@ static const struct external_diff *external_diff(void)

560 if (done_preparing)

561 return external_diff_ptr;

562 external_diff_env.cmd = xstrdup_or_null(getenv("GIT_EXTERNAL_DIFF"));

563 + if (git_env_bool("GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE", 0))

564 + external_diff_env.trust_exit_code = 1;

565 if (external_diff_env.cmd)

566 external_diff_ptr = &external_diff_env;

567 else if (external_diff_cfg.cmd)

@@ -4387,6 +4393,19 @@ static void run_external_diff(const struct external_diff *pgm,

4393 {

4394 struct child_process cmd = CHILD_PROCESS_INIT;

4395 struct diff_queue_struct *q = &diff_queued_diff;

4396 + int quiet = !(o->output_format & DIFF_FORMAT_PATCH);

4397 + int rc;

4398 +

4399 + /*

4400 + * Trivial equality is handled by diff_unmodified_pair() before

4401 + * we get here. If we don't need to show the diff and the

4402 + * external diff program lacks the ability to tell us whether

4403 + * it's empty then we consider it non-empty without even asking.

4404 + */

4405 + if (!pgm->trust_exit_code && quiet) {

4406 + o->found_changes = 1;

4407 + return;

4408 + }

4409

4410 strvec_push(&cmd.args, pgm->cmd);

4411 strvec_push(&cmd.args, name);

@@ -4408,7 +4427,15 @@ static void run_external_diff(const struct external_diff *pgm,

4427 diff_free_filespec_data(one);

4428 diff_free_filespec_data(two);

4429 cmd.use_shell = 1;

4411 - if (run_command(&cmd))

4430 + cmd.no_stdout = quiet;

4431 + rc = run_command(&cmd);

4432 + if (!pgm->trust_exit_code && rc == 0)

4433 + o->found_changes = 1;

4434 + else if (pgm->trust_exit_code && rc == 0)

4435 + ; /* nothing */

4436 + else if (pgm->trust_exit_code && rc == 1)

4437 + o->found_changes = 1;

4438 + else

4439 die(_("external diff died, stopping at %s"), name);

4440

4441 remove_tempfile();

@@ -4926,6 +4953,13 @@ void diff_setup_done(struct diff_options *options)

4953 options->flags.exit_with_status = 1;

4954 }

4955

4956 + /*

4957 + * External diffs could declare non-identical contents equal

4958 + * (think diff --ignore-space-change).

4959 + */

4960 + if (options->flags.allow_external && options->flags.exit_with_status)

4961 + options->flags.diff_from_contents = 1;

4962 +

4963 options->diff_path_counter = 0;

4964

4965 if (options->flags.follow_renames)

t/t4020-diff-external.sh

+23 -10

@@ -177,15 +177,17 @@ check_external_diff () {

177 expect_out=$2

178 expect_err=$3

179 command_code=$4

180 - shift 4

180 + trust_exit_code=$5

181 + shift 5

182 options="$@"

183

184 command="echo output; exit $command_code;"

184 - desc="external diff '$command'"

185 + desc="external diff '$command' with trustExitCode=$trust_exit_code"

186 with_options="${options:+ with }$options"

187

188 test_expect_success "$desc via attribute$with_options" "

189 test_config diff.foo.command \"$command\" &&

190 + test_config diff.foo.trustExitCode $trust_exit_code &&

191 echo \"file diff=foo\" >.gitattributes &&

192 test_expect_code $expect_code git diff $options >out 2>err &&

193 test_cmp $expect_out out &&

@@ -194,6 +196,7 @@ check_external_diff () {

196

197 test_expect_success "$desc via diff.external$with_options" "

198 test_config diff.external \"$command\" &&

199 + test_config diff.trustExitCode $trust_exit_code &&

200 >.gitattributes &&

201 test_expect_code $expect_code git diff $options >out 2>err &&

202 test_cmp $expect_out out &&

@@ -204,6 +207,7 @@ check_external_diff () {

207 >.gitattributes &&

208 test_expect_code $expect_code env \

209 GIT_EXTERNAL_DIFF=\"$command\" \

210 + GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE=$trust_exit_code \

211 git diff $options >out 2>err &&

212 test_cmp $expect_out out &&

213 test_cmp $expect_err err

@@ -216,14 +220,23 @@ test_expect_success 'setup output files' '

220 echo "fatal: external diff died, stopping at file" >error

221 '

222

219 -check_external_diff 0 output empty 0

220 -check_external_diff 128 output error 1

221 -

222 -check_external_diff 1 output empty 0 --exit-code

223 -check_external_diff 128 output error 1 --exit-code

224 -

225 -check_external_diff 1 empty empty 0 --quiet

226 -check_external_diff 1 empty empty 1 --quiet # we don't even call the program

223 +check_external_diff 0 output empty 0 off

224 +check_external_diff 128 output error 1 off

225 +check_external_diff 0 output empty 0 on

226 +check_external_diff 0 output empty 1 on

227 +check_external_diff 128 output error 2 on

228 +

229 +check_external_diff 1 output empty 0 off --exit-code

230 +check_external_diff 128 output error 1 off --exit-code

231 +check_external_diff 0 output empty 0 on --exit-code

232 +check_external_diff 1 output empty 1 on --exit-code

233 +check_external_diff 128 output error 2 on --exit-code

234 +

235 +check_external_diff 1 empty empty 0 off --quiet

236 +check_external_diff 1 empty empty 1 off --quiet # we don't even call the program

237 +check_external_diff 0 empty empty 0 on --quiet

238 +check_external_diff 1 empty empty 1 on --quiet

239 +check_external_diff 128 empty error 2 on --quiet

240

241 echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file

242

userdiff.c

@@ -446,6 +446,10 @@ int userdiff_config(const char *k, const char *v)

446 return parse_tristate(&drv->binary, k, v);

447 if (!strcmp(type, "command"))

448 return git_config_string(&drv->external.cmd, k, v);

449 + if (!strcmp(type, "trustexitcode")) {

450 + drv->external.trust_exit_code = git_config_bool(k, v);

451 + return 0;

452 + }

453 if (!strcmp(type, "textconv"))

454 return git_config_string(&drv->textconv, k, v);

455 if (!strcmp(type, "cachetextconv"))

userdiff.h

@@ -13,6 +13,7 @@ struct userdiff_funcname {

14 struct external_diff {

15 char *cmd;

16 + unsigned trust_exit_code:1;

17 };

19 struct userdiff_driver {