git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH 3/3] diff: show submodule object name when available even on the work tree side
Date: Sun,  2 Mar 2008 01:43:32 -0800	[thread overview]
Message-ID: <1204451012-17487-4-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1204451012-17487-3-git-send-email-gitster@pobox.com>

Traditionally, we consistently showed 0{40} when work tree side was
different from what it was being compared against.  This was primarily
because it did not make sense to re-hash potentially large blobs every
time diff-files and non-cached diff-index were asked for.

However, we can afford to read from submodule HEAD, as it is much cheaper
than hashing blobs.

This makes the output somewhat inconsistent, but it probably is a good
move to give easily available information than not giving it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This could be enhanced to also hash symlink blobs as they tend to be
   very short.

   HOWEVER.

   This changes the semantics established since almost day-one of git (at
   least this 0{40} convention can be traced back to show-diff as of Apr
   26, 2005), and tests (e.g. t3040) demonstrate that it breaks existing
   callers such as git-commit.

   Which means that I am sending this out only for discussion at this
   time, not for inclusion in the near term.

 diff-lib.c                |   32 +++++++++++++++++++++++++++++++-
 t/t4027-diff-submodule.sh |    2 +-
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 4581b59..94e17a6 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -10,6 +10,26 @@
 #include "cache-tree.h"
 #include "path-list.h"
 #include "unpack-trees.h"
+#include "refs.h"
+
+/*
+ * Traditionally, we have _always_ showed 0{40} on the work tree
+ * side if there is a difference.  However, reading from HEAD of
+ * a submodule is much cheaper than rehashing regular blob objects.
+ */
+static const unsigned char *get_gitlink_data(const char *path)
+{
+	static unsigned char subhead[20];
+	/*
+	 * NOTE: strictly speaking, this makes it non re-entrant, but
+	 * we know the nature of the callchain makes it very
+	 * short-lived --- the caller will give this to
+	 * diff_addremove() or diff_change() immediately.
+	 */
+	if (!resolve_gitlink_ref(path, "HEAD", subhead))
+		return subhead;
+	return null_sha1;
+}
 
 /*
  * diff-files
@@ -349,6 +369,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 		struct stat st;
 		unsigned int oldmode, newmode;
 		struct cache_entry *ce = active_cache[i];
+		const unsigned char *worktree_sha1;
 		int changed;
 
 		if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
@@ -454,8 +475,15 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			continue;
 		oldmode = ce->ce_mode;
 		newmode = ce_mode_from_stat(ce, st.st_mode);
+
+		if (S_ISGITLINK(newmode))
+			worktree_sha1 = get_gitlink_data(ce->name);
+		else if (changed)
+			worktree_sha1 = null_sha1;
+		else
+			worktree_sha1 = ce->sha1;
 		diff_change(&revs->diffopt, oldmode, newmode,
-			    ce->sha1, (changed ? null_sha1 : ce->sha1),
+			    ce->sha1, worktree_sha1,
 			    ce->name, NULL);
 
 	}
@@ -501,6 +529,8 @@ static int get_stat_data(struct cache_entry *ce,
 		if (changed) {
 			mode = ce_mode_from_stat(ce, st.st_mode);
 			sha1 = null_sha1;
+			if (S_ISGITLINK(mode))
+				sha1 = get_gitlink_data(ce->name);
 		}
 	}
 
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 3d2d081..c1c2500 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -32,7 +32,7 @@ test_expect_success setup '
 		cd sub &&
 		git rev-list HEAD
 	) &&
-	echo ":160000 160000 $3 $_z40 M	sub" >expect
+	echo ":160000 160000 $3 $2 M	sub" >expect
 '
 
 test_expect_success 'git diff --raw HEAD' '
-- 
1.5.4.3.468.g36990


  reply	other threads:[~2008-03-02  9:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-02  9:43 [PATCH 0/3] fix "diff --raw" on the work tree side Junio C Hamano
2008-03-02  9:43 ` [PATCH 1/3] diff-lib.c: constness strengthening Junio C Hamano
2008-03-02  9:43   ` [PATCH 2/3] diff: make sure work tree side is shown as 0{40} when different Junio C Hamano
2008-03-02  9:43     ` Junio C Hamano [this message]
2008-03-02 10:41 ` [PATCH 0/3] fix "diff --raw" on the work tree side Ping Yin
2008-03-02 17:11   ` Junio C Hamano
2008-03-02 17:15     ` Ping Yin
2008-03-02 17:48       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1204451012-17487-4-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).