git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tim Henigan <tim.henigan@gmail.com>
To: gitster@pobox.com, git@vger.kernel.org, peff@peff.net
Cc: Tim Henigan <tim.henigan@gmail.com>
Subject: [PATCH 2/2] diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes
Date: Thu, 21 Jun 2012 14:09:51 -0400	[thread overview]
Message-ID: <1340302191-23444-2-git-send-email-tim.henigan@gmail.com> (raw)
In-Reply-To: <1340302191-23444-1-git-send-email-tim.henigan@gmail.com>

When running 'git diff --quiet <file1> <file2>', if file1 or file2
is outside the repository, it will exit(0) even if the files differ.
It should exit(1) when they differ.

This happens because 'diff_no_index' uses the 'found_changes' member
from 'diff_options' to determine if changes were made. This is the
wrong flag, since it is only set if xdiff is actually run and it
finds a change. The diff machinery will optimize out the xdiff call
when it is not necessary.

'diff_no_index' needs to check the 'HAS_CHANGES' flag instead, which
is done in the 'diff_result_code' function. This matches the code
paths used for regular index-aware diff.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

Patch 1/2 is new to this series, but 3 earlier drafts of this patch
(2/2) were sent to the list for review.

Changes in this version:
  - Improved commit message based on suggestions from Jeff King.
  - Removed declaration after statement in diff-no-index.c.
  - Removed space after redirection operator in t4035.
  - Changed non-git paths in t4035 to match naming used in t7810.
  - Changed non-git paths to be relative rather than absolute.


 diff-no-index.c       |  2 +-
 t/t4035-diff-quiet.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index e6b9952..63c31cc 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -256,5 +256,5 @@ void diff_no_index(struct rev_info *revs,
 	 * The return code for --no-index imitates diff(1):
 	 * 0 = no changes, 1 = changes, else error
 	 */
-	exit(revs->diffopt.found_changes);
+	exit(diff_result_code(&revs->diffopt, 0));
 }
diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh
index cdb9202..231412d 100755
--- a/t/t4035-diff-quiet.sh
+++ b/t/t4035-diff-quiet.sh
@@ -10,7 +10,22 @@ test_expect_success 'setup' '
 	git commit -m first &&
 	echo 2 >b &&
 	git add . &&
-	git commit -a -m second
+	git commit -a -m second &&
+	mkdir -p test-outside/repo && (
+		cd test-outside/repo &&
+		git init &&
+		echo "1 1" >a &&
+		git add . &&
+		git commit -m 1
+	) &&
+	mkdir -p test-outside/non/git && (
+		cd test-outside/non/git &&
+		echo "1 1" >a &&
+		echo "1 1" >matching-file &&
+		echo "1 1 " >trailing-space &&
+		echo "1   1" >extra-space &&
+		echo "2" >never-match
+	)
 '
 
 test_expect_success 'git diff-tree HEAD^ HEAD' '
@@ -77,4 +92,60 @@ test_expect_success 'git diff-index --cached HEAD' '
 	}
 '
 
+test_expect_success 'git diff, one file outside repo' '
+	(
+		cd test-outside/repo &&
+		test_expect_code 0 git diff --quiet a ../non/git/matching-file &&
+		test_expect_code 1 git diff --quiet a ../non/git/extra-space
+	)
+'
+
+test_expect_success 'git diff, both files outside repo' '
+	(
+		GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/test-outside" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd test-outside/non/git &&
+		test_expect_code 0 git diff --quiet a matching-file &&
+		test_expect_code 1 git diff --quiet a extra-space
+	)
+'
+
+test_expect_success 'git diff --ignore-space-at-eol, one file outside repo' '
+	(
+		cd test-outside/repo &&
+		test_expect_code 0 git diff --quiet --ignore-space-at-eol a ../non/git/trailing-space &&
+		test_expect_code 1 git diff --quiet --ignore-space-at-eol a ../non/git/extra-space
+	)
+'
+
+test_expect_success 'git diff --ignore-space-at-eol, both files outside repo' '
+	(
+		GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/test-outside" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd test-outside/non/git &&
+		test_expect_code 0 git diff --quiet --ignore-space-at-eol a trailing-space &&
+		test_expect_code 1 git diff --quiet --ignore-space-at-eol a extra-space
+	)
+'
+
+test_expect_success 'git diff --ignore-all-space, one file outside repo' '
+	(
+		cd test-outside/repo &&
+		test_expect_code 0 git diff --quiet --ignore-all-space a ../non/git/trailing-space &&
+		test_expect_code 0 git diff --quiet --ignore-all-space a ../non/git/extra-space &&
+		test_expect_code 1 git diff --quiet --ignore-all-space a ../non/git/never-match
+	)
+'
+
+test_expect_success 'git diff --ignore-all-space, both files outside repo' '
+	(
+		GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/test-outside" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd test-outside/non/git &&
+		test_expect_code 0 git diff --quiet --ignore-all-space a trailing-space &&
+		test_expect_code 0 git diff --quiet --ignore-all-space a extra-space &&
+		test_expect_code 1 git diff --quiet --ignore-all-space a never-match
+	)
+'
+
 test_done
-- 
1.7.11.3.gf4ddae1

      reply	other threads:[~2012-06-21 18:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-21 18:09 [PATCH 1/2] diff: handle relative paths in no-index Tim Henigan
2012-06-21 18:09 ` Tim Henigan [this message]

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=1340302191-23444-2-git-send-email-tim.henigan@gmail.com \
    --to=tim.henigan@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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).