Re: [PATCH] diff: exit(1) if 'diff --quiet <repo file> <external file>' finds changes
From: Jeff King <hidden>
Date: 2016-06-15 22:54:07
On Fri, Jun 15, 2012 at 01:31:03PM -0400, Tim Henigan wrote:
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.
From your description, I would expect the fix to be in builtin/diff.c,
or in diff-no-index.c, since that is where the code paths diverge.
This was the least invasive fix that I found. I considered adding the following when the '--quiet' option is parsed instead: + DIFF_OPT_SET(options, EXIT_WITH_STATUS) + DIFF_OPT_SET(options, DIFF_FROM_CONTENTS)
We already set EXIT_WITH_STATUS when we see --quiet (we just do it a little later, during diff_setup_done). We would not want to set DIFF_FROM_CONTENTS all the time with --quiet. The point of that flag is "we cannot know just from seeing the path sha1s whether they are different or not, because we are doing content-level munging" (for example, things like ignoring whitespace changes). So we would not want to always set it whenever --quiet is given, because it means we must do a lot of extra work comparing file content.
quoted hunk ↗ jump to hunk
diff --git a/diff.c b/diff.c index 77edd50..b1d74fe 100644 --- a/diff.c +++ b/diff.c@@ -4432,9 +4432,10 @@ void diff_flush(struct diff_options *options) separator++; } - if (output_format & DIFF_FORMAT_NO_OUTPUT && + if ((output_format & DIFF_FORMAT_NO_OUTPUT && DIFF_OPT_TST(options, EXIT_WITH_STATUS) && - DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) { + DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) || + DIFF_OPT_TST(options, QUICK)) { /* * run diff_flush_patch for the exit status. setting * options->file to /dev/null should be safe, becaue we
And this is equally bad, because it means that --quiet gets much slower for _all_ cases, not just the no-index case. I suspect what you actually want is to set DIFF_FROM_CONTENTS in the no-index case, since we by definition do not have a pair of sha1s to compare. But it may also be that diff.c could detect this case automatically. I'd have to look closer. -Peff