git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v2 5/5] diff: teach "--name-status" and friends to honor "--exit-code -w"
Date: Thu, 17 Aug 2023 15:29:49 -0700	[thread overview]
Message-ID: <20230817222949.3835424-6-gitster@pobox.com> (raw)
In-Reply-To: <20230817222949.3835424-1-gitster@pobox.com>

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
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 <gitster@pobox.com>
---
 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)" '
-- 
2.42.0-rc2


  parent reply	other threads:[~2023-08-17 22:30 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-04 16:46 git bug report Paul Watson
2023-08-04 17:28 ` rsbecker
2023-08-04 17:48   ` [EXTERNAL] " Paul Watson
2023-08-04 18:12     ` rsbecker
2023-08-07 15:46       ` Paul Watson
2023-08-08 13:07         ` Paul Watson
2023-08-08 13:28           ` rsbecker
2023-08-08 17:07 ` Junio C Hamano
2023-08-16 23:45   ` [PATCH] diff: tighten interaction between -w and --exit-code Junio C Hamano
2023-08-17  5:10     ` Jeff King
2023-08-17 16:12       ` Junio C Hamano
2023-08-17 19:49         ` Jeff King
2023-08-17 19:56           ` Junio C Hamano
2023-08-17 22:29             ` [PATCH v2 0/5] fix interactions with "-w" and "--exit-code" Junio C Hamano
2023-08-17 22:29               ` [PATCH v2 1/5] diff: --dirstat leakfix Junio C Hamano
2023-08-17 22:29               ` [PATCH v2 2/5] diff: move the fallback "--exit-code" code down Junio C Hamano
2023-08-17 22:29               ` [PATCH v2 3/5] diff: mode-only change should be noticed by "--patch -w --exit-code" Junio C Hamano
2023-08-18 21:15                 ` Junio C Hamano
2023-08-17 22:29               ` [PATCH v2 4/5] diff: teach "--stat -w --exit-code" to notice differences Junio C Hamano
2023-08-17 22:29               ` Junio C Hamano [this message]
2023-08-17 22:37               ` [PATCH v2 0/5] fix interactions with "-w" and "--exit-code" Junio C Hamano
2023-08-18 23:59               ` [PATCH v3 " Junio C Hamano
2023-08-18 23:59                 ` [PATCH v3 1/5] diff: move the fallback "--exit-code" code down Junio C Hamano
2023-08-21 20:41                   ` Jeff King
2023-08-18 23:59                 ` [PATCH v3 2/5] diff: mode-only change should be noticed by "--patch -w --exit-code" Junio C Hamano
2023-08-18 23:59                 ` [PATCH v3 3/5] diff: teach "--stat -w --exit-code" to notice differences Junio C Hamano
2023-08-21 20:45                   ` Jeff King
2023-08-18 23:59                 ` [PATCH v3 4/5] t4040: remove test that succeeded for a wrong reason Junio C Hamano
2023-08-18 23:59                 ` [PATCH v3 5/5] diff: the -w option breaks --exit-code for --raw and other output modes Junio C Hamano
2023-08-21 21:00                   ` Jeff King
2023-08-21 21:08                     ` Jeff King
2023-08-21 22:23                       ` Jeff King
2023-08-21 22:16                     ` Junio C Hamano
2023-08-21 22:26                     ` Junio C Hamano
2023-08-22  1:30                       ` Jeff King
2023-08-21 21:02                 ` [PATCH v3 0/5] fix interactions with "-w" and "--exit-code" Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230817222949.3835424-6-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).