[PATCH] diff-cache: diff-patch (-p) format fixes.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:41:58
Subsystem:
the rest · Maintainer:
Linus Torvalds
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 <redacted> --- diff.c | 26 ++++++++++++++++++++++---- 1 files changed, 22 insertions(+), 4 deletions(-)
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; ------------------------------------------------