diff: do not quit early on stat-dirty files

When QUICK is set (i.e. with --quiet) we try to do as little work as
possible, stopping after seeing the first change. stat-dirty is
considered a "change" but it may turn out not, if no actual content is
changed. The actual content test is performed too late in the process
and the shortcut may be taken prematurely, leading to incorrect return
code.

Assume we do "git diff --quiet". If we have a stat-dirty file "a" and
a really dirty file "b". We break the loop in run_diff_files() and
stop after "a" because we have got a "change". Later in
diffcore_skip_stat_unmatch() we find out "a" is actually not
changed. But there's nothing else in the diff queue, we incorrectly
declare "no change", ignoring the fact that "b" is changed.

This also happens to "git diff --quiet HEAD" when it hits
diff_can_quit_early() in oneway_diff().

This patch does the content test earlier in order to keep going if "a"
is unchanged. The test result is cached so that when
diffcore_skip_stat_unmatch() is done in the end, we spend no cycles on
re-testing "a".

Reported-by: IWAMOTO Toshihiro <iwamoto@valinux.co.jp>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 1e46b575e1940da435a27c10c3ae8a25752efa61..691578b2c6f6d980638980b0c575a8356af5c64d 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -4594,6 +4594,11 @@ static int diff_filespec_is_identical(struct diff_filespec *one,
 
 static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
 {
 
 static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
 {

+       if (p->done_skip_stat_unmatch)
+               return p->skip_stat_unmatch_result;
+
+       p->done_skip_stat_unmatch = 1;
+       p->skip_stat_unmatch_result = 0;

        /*
         * 1. Entries that come from stat info dirtiness
         *    always have both sides (iow, not create/delete),
        /*
         * 1. Entries that come from stat info dirtiness
         *    always have both sides (iow, not create/delete),


@@ -4615,8 +4620,8 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
            diff_populate_filespec(p->two, 1) ||
            (p->one->size != p->two->size) ||
            !diff_filespec_is_identical(p->one, p->two)) /* (2) */
            diff_populate_filespec(p->two, 1) ||
            (p->one->size != p->two->size) ||
            !diff_filespec_is_identical(p->one, p->two)) /* (2) */

-               return 1;
-       return 0;
+               p->skip_stat_unmatch_result = 1;
+       return p->skip_stat_unmatch_result;

 }
 
 static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 }
 
 static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)


@@ -4792,6 +4797,7 @@ void diff_change(struct diff_options *options,
                 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
        struct diff_filespec *one, *two;
                 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
        struct diff_filespec *one, *two;

+       struct diff_filepair *p;

 
        if (S_ISGITLINK(old_mode) && S_ISGITLINK(new_mode) &&
            is_submodule_ignored(concatpath, options))
 
        if (S_ISGITLINK(old_mode) && S_ISGITLINK(new_mode) &&
            is_submodule_ignored(concatpath, options))


@@ -4818,10 +4824,16 @@ void diff_change(struct diff_options *options,
        fill_filespec(two, new_sha1, new_sha1_valid, new_mode);
        one->dirty_submodule = old_dirty_submodule;
        two->dirty_submodule = new_dirty_submodule;
        fill_filespec(two, new_sha1, new_sha1_valid, new_mode);
        one->dirty_submodule = old_dirty_submodule;
        two->dirty_submodule = new_dirty_submodule;

+       p = diff_queue(&diff_queued_diff, one, two);

 
 

-       diff_queue(&diff_queued_diff, one, two);
-       if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
-               DIFF_OPT_SET(options, HAS_CHANGES);
+       if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
+               return;
+
+       if (DIFF_OPT_TST(options, QUICK) && options->skip_stat_unmatch &&
+           !diff_filespec_check_stat_unmatch(p))
+               return;
+
+       DIFF_OPT_SET(options, HAS_CHANGES);

 }
 
 struct diff_filepair *diff_unmerge(struct diff_options *options, const char *path)
 }
 
 struct diff_filepair *diff_unmerge(struct diff_options *options, const char *path)

diff --git a/diffcore.h b/diffcore.h
index 1c16c8595b21c2712259041c01d4b58d76a60222..6b538bc2fbeb1b0577d5e233357178b1482f1786 100644 (file)
--- a/diffcore.h
+++ b/diffcore.h
@@ -70,6 +70,8 @@ struct diff_filepair {
        unsigned broken_pair : 1;
        unsigned renamed_pair : 1;
        unsigned is_unmerged : 1;
        unsigned broken_pair : 1;
        unsigned renamed_pair : 1;
        unsigned is_unmerged : 1;

+       unsigned done_skip_stat_unmatch : 1;
+       unsigned skip_stat_unmatch_result : 1;

 };
 #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)
 
 };
 #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)
 

diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh
index 231412d1008e45e1d788660930cab708d3912ed4..e8ae2a03fdcf5ac57dc0e5bcacd40501960bce1c 100755 (executable)
--- a/t/t4035-diff-quiet.sh
+++ b/t/t4035-diff-quiet.sh
@@ -148,4 +148,10 @@ test_expect_success 'git diff --ignore-all-space, both files outside repo' '
        )
 '
 
        )
 '
 

+test_expect_success 'git diff --quiet ignores stat-change only entries' '
+       test-chmtime +10 a &&
+       echo modified >>b &&
+       test_expect_code 1 git diff --quiet
+'
+

 test_done
 test_done