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 v3 0/5] fix interactions with "-w" and "--exit-code"
Date: Fri, 18 Aug 2023 16:59:27 -0700	[thread overview]
Message-ID: <20230818235932.3253552-1-gitster@pobox.com> (raw)
In-Reply-To: <20230817222949.3835424-1-gitster@pobox.com>

Paul Watson reported that "diff --no-index --exit-code
--ignore-all-space" does not work when used with "--shortstat".  It
turns out that this is not limited to "--no-index" mode, and it is
not limited to "--shortstat".  Anything that does not use the "--patch"
machinery to discover the content level differences ignored --exit-code
when used with options like "-w" and always exited with 0.  In fact,
even the "--patch" machinery was slightly faulty in corner cases.

And here is another round to fix it.
Previous one is at https://lore.kernel.org/git/20230817222949.3835424-1-gitster@pobox.com/

The interdiff is not all that interesting.  One patch dropped, two
patches stay the same, one patch has one-line change, and the final
one patch has been completely reworked.  Here is the summary:

 * The first patch about dirstat leak-fix is now gone.  The series
   has instead been rebased on top of the official "dirstat leak-fix"
   series that was merged in Git 2.41.

 * The next patch (preliminary code clean-up) stays the same.  It is
   now [1/5] instead of being the second step.

 * The next patch is to fix the corner case bug of "--patch"
   machinery.  The code stays the same, but the tests were asking
   the filesystem to do something impossible when they do not have
   executable bit support, so a prerequisite has been added to work
   around them.

 * The next patch is to fix "--stat -w --exit-code", which stays
   the same.

 * The last step is completely new.  v2 took an approach to reuse
   the "--patch" machinery even for "--raw" and other modes, but it
   would mean that "diff --raw --exit-code" may exit with 0 even
   when it has different paths to report, which is confusing.  v3
   changes the approach to align the exit status with the presence
   of reported paths that are different better.  "--raw" has ignored
   "-w" when producing its output.  It should ignore "-w" when
   reporting differences with its exit code, instead of always
   exiting with 0.

Junio C Hamano (5):
  diff: move the fallback "--exit-code" code down
  diff: mode-only change should be noticed by "--patch -w --exit-code"
  diff: teach "--stat -w --exit-code" to notice differences
  t4040: remove test that succeeded for a wrong reason
  diff: the -w option breaks --exit-code for --raw and other output
    modes

 diff.c                       | 40 ++++++++++++++++++++++--------------
 t/t4015-diff-whitespace.sh   | 39 ++++++++++++++++++++++++++++++++++-
 t/t4040-whitespace-status.sh |  3 +--
 3 files changed, 64 insertions(+), 18 deletions(-)

Range-diff against v2:
1:  65ff4655a2 < -:  ---------- diff: --dirstat leakfix
2:  533f974c9b ! 1:  df869ac550 diff: move the fallback "--exit-code" code down
    @@ Commit message
         diff: move the fallback "--exit-code" code down
     
         When "--exit-code" is asked and the code cannot just answer by
    -    comparing the object names on both sides but need to inspect and
    +    comparing the object names on both sides but needs to inspect and
         compare the contents, there are two ways that the result is found
         out.
     
         Some output modes, like "--stat" and "--patch", inherently have to
    -    inspect the contents in order to _show_ the differences in the way
    +    inspect the contents in order to show the differences in the way
         they do.  The codepaths for these modes set the .found_changes bit
         as they compute what to show.
     
         However, other output modes do not need to inspect the contents to
         show the differences in the way they do.  The most notable example
    -    is "--quiet", which does not need to compute any output.  When they
    -    are asked to report "--exit-code", they run the codepaths for the
    -    "--patch" output with their output redirected to "/dev/null", only
    -    to set the .found_changes bit.
    +    is "--quiet", which does not need to compute any output to show.
    +    When they are asked to report "--exit-code", they run the codepaths
    +    for the "--patch" output with their output redirected to "/dev/null",
    +    only to set the .found_changes bit.
     
         Currently, this fallback invocation of "--patch" output is done
         after the "--stat" output format and its friends and before the
3:  89338b9302 ! 2:  b349ad5278 diff: mode-only change should be noticed by "--patch -w --exit-code"
    @@ Metadata
      ## Commit message ##
         diff: mode-only change should be noticed by "--patch -w --exit-code"
     
    -    The codepath to notice the content-level changes, taking certaion
    +    The codepath to notice the content-level changes, taking certain
         no-op changes like "ignore whitespace" into account, forgot that
         a mode-only change is still a change.  This resulted in
     
    @@ t/t4015-diff-whitespace.sh: TEST_PASSES_SANITIZE_LEAK=true
     +		test_expect_code 1 git diff -w $opts --exit-code x
     +	'
     +
    -+	test_expect_success "status with $opts (mode differs)" '
    ++	test_expect_success POSIXPERM "status with $opts (mode differs)" '
     +		test_when_finished "git update-index --chmod=-x x" &&
     +		echo foo >x &&
     +		git add x &&
4:  8fc63f4a04 ! 3:  be50b023a8 diff: teach "--stat -w --exit-code" to notice differences
    @@ Commit message
     
             $ git diff --stat -w --exit-code
     
    -    for a change that does have an outout to exit with status 0.
    +    for a change that does have an output to exit with status 0.
     
         Set the bit by inspecting the list of paths the diffstat output is
         given for (a mode-only change will still appear as a "0-line added
5:  200593e9e0 < -:  ---------- diff: teach "--name-status" and friends to honor "--exit-code -w"
-:  ---------- > 4:  d1a6fa7d17 t4040: remove test that succeeded for a wrong reason
-:  ---------- > 5:  573f810cce diff: the -w option breaks --exit-code for --raw and other output modes

base-commit: 83973981eb475ce90f829f8a5bd6ea99cd3bbd8e
-- 
2.42.0-rc2-7-gf9972720e9


  parent reply	other threads:[~2023-08-19  0:00 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               ` [PATCH v2 5/5] diff: teach "--name-status" and friends to honor "--exit-code -w" Junio C Hamano
2023-08-17 22:37               ` [PATCH v2 0/5] fix interactions with "-w" and "--exit-code" Junio C Hamano
2023-08-18 23:59               ` Junio C Hamano [this message]
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=20230818235932.3253552-1-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).