[PATCH] diff-cache: diff-patch (-p) format fixes.

From

Junio C Hamano <junkio@cox.net>

Date

May 29, 2005, 05:23 UTC

Message-ID

<7voeaur4fu.fsf_-_@assigned-by-dhcp.cox.net>

In-Reply-To

<7v3bs6slwl.fsf_-_@assigned-by-dhcp.cox.net>

This fixes two more bugs.

 - When diff-cache is run with -R and without --cached, a
   stat-dirty but otherwise unmodified file still produced an
   empty "diff --git" header (earlier fix was only for the case
   without -R).

 - When diff-cache is run without --cached, an unmodifed file
   that has different mode bits from the tree incorrectly picked
   up the mode bits from the filesystem, when it should have
   used what was recorded in the tree object.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -183,6 +183,9 @@ void fill_filespec(struct diff_filespec 
  * Given a name and sha1 pair, if the dircache tells us the file in
  * the work tree has that object contents, return true, so that
  * prepare_temp_file() does not have to inflate and extract.
+ *
+ * NOTE: this function does not use the mode bits, so diff_filespec
+ * users must be careful about mode handling!
  */
 static int work_tree_matches(const char *name, const unsigned char *sha1)
 {
@@ -394,6 +397,10 @@ static void prepare_temp_file(const char
 
 	if (!one->sha1_valid ||
 	    work_tree_matches(name, one->sha1)) {
+		/* NOTE: we only say the matching file has the same
+		 * contents.  It may have a different mode, in which
+		 * case we should not pick it up from the filesystem!
+		 */
 		struct stat st;
 		if (lstat(name, &st) < 0) {
 			if (errno == ENOENT)
@@ -421,8 +428,15 @@ static void prepare_temp_file(const char
 				strcpy(temp->hex, sha1_to_hex(null_sha1));
 			else
 				strcpy(temp->hex, sha1_to_hex(one->sha1));
-			sprintf(temp->mode, "%06o",
-				S_IFREG |ce_permissions(st.st_mode));
+			/* even though we borrow the contents from the
+			 * work tree, we want our mode if we are not told
+			 * to look at the filesystem.
+			 */
+			if (one->sha1_valid)
+				sprintf(temp->mode, "%06o", one->mode);
+			else
+				sprintf(temp->mode, "%06o",
+					S_IFREG | ce_permissions(st.st_mode));
 		}
 		return;
 	}
@@ -716,9 +730,13 @@ static void diff_flush_patch(struct diff
 	 * need to see if the two are the same and if so not to emit
 	 * anything at all.  Avoid is_exact_match() comparison when it
 	 * does not matter.
+	 * Notice that "look at the filesystem" can happen on p->one
+	 * if we are operating under reverse-diff option.
 	 */
-	if ((DIFF_FILE_VALID(p->two) && !p->two->sha1_valid) &&
-	    is_exact_match(p->one, p->two))
+	if ( ((DIFF_FILE_VALID(p->one) && !p->one->sha1_valid) ||
+	      (DIFF_FILE_VALID(p->two) && !p->two->sha1_valid)) &&
+	     is_exact_match(p->one, p->two) &&
+	     (p->one->mode == p->two->mode) )
 		return;
 
 	name = p->one->path;
------------------------------------------------