[v2,5/5] diff: teach "--name-status" and friends to honor "--exit-code -w"

Commit Message

Junio C Hamano Aug. 17, 2023, 10:29 p.m. UTC

We have fallback code that is used when "-s -w --exit-code" options
are used together to compute the exit code, because the exit code
must take the whitespace-ignoring comparison into account over the
contents, but with "-s", the normal codepath does not even have to
look at the contents at all.  The fallback code simply runs a "git

Patch

diff --patch" while sending the output to "/dev/null".

The codepaths for some other output modes, like "--name-status" and
"--raw", share the same trait as "-s" in that they do not look at
the contents for their output generation.  Extend the fallback code
to cover these output modes as well.

Note that they may still not be correct in that a path whose
contents have no differences other than whitespace changes would
still show up in the "diff -w --name-only --exit-code" output, even
though the exit status may say there is no differences.  Arguably
this is better than status quo, even though it still may be wrong.

Signed-off-by: Junio C Hamano <[e-mail]>
---
 diff.c                     | 23 +++++++++++++++++++----
 t/t4015-diff-whitespace.sh |  3 ++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index 7213237675..3bb9d11bfe 100644
--- a/diff.c
+++ b/diff.c
@@ -6573,13 +6573,28 @@  void diff_flush(struct diff_options *options)
 	if (output_format & DIFF_FORMAT_CALLBACK)
 		options->format_callback(q, options, options->format_callback_data);
 
-	if (output_format & DIFF_FORMAT_NO_OUTPUT &&
+	if (((output_format & DIFF_FORMAT_NO_OUTPUT) ||
+	     /* these compute .found_changes properly */
+	     !(output_format & (DIFF_FORMAT_DIFFSTAT|
+				DIFF_FORMAT_SHORTSTAT|
+				DIFF_FORMAT_NUMSTAT|
+				DIFF_FORMAT_DIRSTAT|
+				DIFF_FORMAT_PATCH))) &&
 	    options->flags.exit_with_status &&
 	    options->flags.diff_from_contents) {
 		/*
-		 * run diff_flush_patch for the exit status. setting
-		 * options->file to /dev/null should be safe, because we
-		 * aren't supposed to produce any output anyway.
+		 * We need to inspect the contents, not just object
+		 * names, to determine the exit status, but the usual
+		 * processing for the output format specified does not
+		 * have to work with and does not look at the
+		 * contents.  Run an extra and silent "diff --patch"
+		 * but discard the output to /dev/null, so that we
+		 * would set the .found_changes bit correctly.
+		 *
+		 * We can safely close and discard the original output
+		 * file here, since all that is left to do from this
+		 * point is to return (we don't do the FORMAT_PATCH
+		 * thing below).
 		 */
 		diff_free_file(options);
 		options->file = xfopen("/dev/null", "w");
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 355d96aa14..412d20181c 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -11,7 +11,8 @@  TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-diff.sh
 
-for opts in --patch --quiet -s --stat --shortstat --dirstat=lines
+for opts in --patch --quiet -s --stat --shortstat --dirstat=lines \
+	    --name-only --raw --name-status --summary
 do
 
 	test_expect_success "status with $opts (different)" '