Re: [PATCH v2 3/3] 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().
> 
> 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.

> +diff.trustExitCode::
> +	If this boolean value is set to true then the `diff.external`
> +	command is expected to return exit code 1 if it finds
> +	significant changes and 0 if it doesn't, like diff(1).  If it's
> +	false then the `diff.external` command is expected to always
> +	return exit code 0.  Defaults to false.

>   	strvec_push(&cmd.args, pgm->cmd);
>   	strvec_push(&cmd.args, name);
> @@ -4406,7 +4424,10 @@ 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))
> +	rc = run_command(&cmd);
> +	if ((!pgm->trust_exit_code && !rc) || (pgm->trust_exit_code && rc == 1))
> +		o->found_changes = 1;
> +	else if (!pgm->trust_exit_code || rc)
>   		die(_("external diff died, stopping at %s"), name);

> -check_external_exit_code   1 0 --exit-code
> -check_external_exit_code   1 0 --quiet
> -check_external_exit_code 128 1 --exit-code
> -check_external_exit_code   1 1 --quiet # we don't even call the program
> +check_external_exit_code   1 0 off --exit-code
> +check_external_exit_code   1 0 off --quiet
> +check_external_exit_code 128 1 off --exit-code
> +check_external_exit_code   1 1 off --quiet # we don't even call the program
> +
> +check_external_exit_code   0 0 on --exit-code
> +check_external_exit_code   0 0 on --quiet
> +check_external_exit_code   1 1 on --exit-code
> +check_external_exit_code   1 1 on --quiet
> +check_external_exit_code 128 2 on --exit-code
> +check_external_exit_code 128 2 on --quiet