git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] git-diff: don't squelch the new SHA1 in submodule diffs
@ 2007-09-08 10:30 Sven Verdoolaege
  2007-09-09  0:18 ` Johannes Schindelin
  0 siblings, 1 reply; 2+ messages in thread
From: Sven Verdoolaege @ 2007-09-08 10:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

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>
---
 diff.c                     |   21 +++++++++++++++++----
 t/t7400-submodule-basic.sh |    4 ++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 0d30d05..1aca5df 100644
--- a/diff.c
+++ b/diff.c
@@ -3144,6 +3144,22 @@ static void diffcore_apply_filter(const char *filter)
 	*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;
@@ -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, 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 --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 9d142ed..4fe3a41 100755
--- 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"
 '
 
+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) &&
-- 
1.5.3.1.20.g3d038

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] git-diff: don't squelch the new SHA1 in submodule diffs
  2007-09-08 10:30 [PATCH] git-diff: don't squelch the new SHA1 in submodule diffs Sven Verdoolaege
@ 2007-09-09  0:18 ` Johannes Schindelin
  0 siblings, 0 replies; 2+ messages in thread
From: Johannes Schindelin @ 2007-09-09  0:18 UTC (permalink / raw)
  To: skimo; +Cc: Junio C Hamano, Git Mailing List

Hi,

On Sat, 8 Sep 2007, Sven Verdoolaege wrote:

> 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.

Ah, I was wondering... This struck me as odd in msysGit, but I did not 
know if it was a problem I introduced in 4msysgit or not.  Unfortunately, 
I did not have the time to investigate further.

Thanks,
Dscho

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-09-09  8:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-08 10:30 [PATCH] git-diff: don't squelch the new SHA1 in submodule diffs Sven Verdoolaege
2007-09-09  0:18 ` Johannes Schindelin

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).