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

Tim Henigan [off-list ref] 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);
 }