On Fri, Jun 15, 2012 at 11:45:47AM -0700, Junio C Hamano wrote:
> 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);
Hmm. This works because is checks HAS_CHANGES instead of found_changes.
I was somewhat surprised that the former works at all, but it is because
diffcore_std sets it if there is anything in diff_queued_diff. Which
makes sense; found_changes seems to be only about communicating changes
up from builtin_diff to the rest of the diff code; nobody should be
reading it outside of there.
Which left me to wonder how we tell when two files are actually the
same. The answer is that diffcore_skip_stat_unmatch actually loads and
memcmps each pair, removing ones that are identical. And diff_no_index
always sets skip_stat_unmatch, whether diff.autorefreshindex is on or
not.
So the patch I posted elsewhere in the thread is not right; we do not
need to do diff_flush_patch to actually compare, because the
stat_unmatch code will have done everything we want (unless
DIFF_FROM_CONTENTS really is set). And the bug is purely one of looking
at the wrong output flag.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html