git-diff: don't squelch the new SHA1 in submodule diffs

The code to squelch empty diffs introduced by commit
fb13227e089f22dc31a3b1624559153821056848 would inadvertently
populate filespec "two" of a submodule change using the uninitialized
(null) SHA1, thereby replacing the submodule SHA1 by 0{40} in the output.

This change teaches diffcore_skip_stat_unmatch to handle
submodule changes correctly.

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 0d30d05263f631c2c88ebb1b8af7e96bb2cfd56a..1aca5df522d1c5249c9d1ce8d030849d59a5eb6a 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -3144,6 +3144,22 @@ static void diffcore_apply_filter(const char *filter)
        *q = outq;
 }
 
        *q = outq;
 }
 

+/* Check whether two filespecs with the same mode and size are identical */
+static int diff_filespec_is_identical(struct diff_filespec *one,
+                                     struct diff_filespec *two)
+{
+       if (S_ISGITLINK(one->mode)) {
+               diff_fill_sha1_info(one);
+               diff_fill_sha1_info(two);
+               return !hashcmp(one->sha1, two->sha1);
+       }
+       if (diff_populate_filespec(one, 0))
+               return 0;
+       if (diff_populate_filespec(two, 0))
+               return 0;
+       return !memcmp(one->data, two->data, one->size);
+}
+

 static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 {
        int i;
 static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 {
        int i;


@@ -3175,10 +3191,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
                    diff_populate_filespec(p->one, 1) ||
                    diff_populate_filespec(p->two, 1) ||
                    (p->one->size != p->two->size) ||
                    diff_populate_filespec(p->one, 1) ||
                    diff_populate_filespec(p->two, 1) ||
                    (p->one->size != p->two->size) ||

-
-                   diff_populate_filespec(p->one, 0) || /* (2) */
-                   diff_populate_filespec(p->two, 0) ||
-                   memcmp(p->one->data, p->two->data, p->one->size))
+                   !diff_filespec_is_identical(p->one, p->two)) /* (2) */

                        diff_q(&outq, p);
                else {
                        /*
                        diff_q(&outq, p);
                else {
                        /*

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 9d142ed649ef98adcbf7c6d158890c9f55370ef7..4fe3a41f07f2f05865b219ff00afaa5786c679d5 100755 (executable)
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -152,6 +152,10 @@ test_expect_success 'the --cached sha1 should be rev1' '
        git-submodule --cached status | grep "^+$rev1"
 '
 
        git-submodule --cached status | grep "^+$rev1"
 '
 

+test_expect_success 'git diff should report the SHA1 of the new submodule commit' '
+       git-diff | grep "^+Subproject commit $rev2"
+'
+

 test_expect_success 'update should checkout rev1' '
        git-submodule update &&
        head=$(cd lib && git rev-parse HEAD) &&
 test_expect_success 'update should checkout rev1' '
        git-submodule update &&
        head=$(cd lib && git rev-parse HEAD) &&