Re: [PATCH] diff: exit(1) if 'diff --quiet <repo file> <external file>' finds changes

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 15, 2012, 18:45 UTC
Message-ID: <7vzk849zxg.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1339781463-13536-1-git-send-email-tim.henigan@gmail.com>

Tim Henigan <tim.henigan@gmail.com> writes:

> This was the least invasive fix that I found.

I think this breaks a normal case of comparing revisions and tracked contents in a big way.

The "quick" thing is meant to notice any difference in the paths involved (e.g. "git diff maint master -- t/") at the blob object name level without having to look at the contents when there is no DIFF_FROM_CONTENTS processing is needed. We call into the more expensive diff_flush_patch() codepath only when things like "ignore whitespace change" is given, in which case we would need to compare the contents.

Your patch seems to be making us go through diff_flush_patch() codepath unconditionally, and it looks like it is only to sweep some other breakage in diff-no-index codepath under the rug.

Have you looked at how "git diff --quiet HEAD^" (without any other option) for tracked files notices that there is a difference and exit with non-zero status? Is it doing something wrong? Otherwise why can't the no-index codepath do the same thing?

I think the following may be a lot closer to the correct fix; I didn't test many combinations of options with it, though.

 diff-no-index.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index f0b0010..ed74e27 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -172,7 +172,7 @@ void diff_no_index(struct rev_info *revs,
 		   int argc, const char **argv,
 		   int nongit, const char *prefix)
 {
-	int i;
+	int i, result;
 	int no_index = 0;
 	unsigned options = 0;
 
@@ -273,5 +273,6 @@ 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);
+	result = !!diff_result_code(&revs->diffopt, 0);
+	exit(result);
 }