Merge branch 'jc/maint-split-diff-metainfo' into maint

* jc/maint-split-diff-metainfo:
  diff.c: output correct index lines for a split diff

3 files changed, 99 insertions, 67 deletions

diff --git a/diff.c b/diff.c
index 5b85b4077a..416c5aa722 100644
--- a/diff.c
+++ b/diff.c
@@ -1999,16 +1999,86 @@ static void run_external_diff(const char *pgm,
 	}
 }
 
+static int similarity_index(struct diff_filepair *p)
+{
+	return p->score * 100 / MAX_SCORE;
+}
+
+static void fill_metainfo(struct strbuf *msg,
+			  const char *name,
+			  const char *other,
+			  struct diff_filespec *one,
+			  struct diff_filespec *two,
+			  struct diff_options *o,
+			  struct diff_filepair *p)
+{
+	strbuf_init(msg, PATH_MAX * 2 + 300);
+	switch (p->status) {
+	case DIFF_STATUS_COPIED:
+		strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
+		strbuf_addstr(msg, "\ncopy from ");
+		quote_c_style(name, msg, NULL, 0);
+		strbuf_addstr(msg, "\ncopy to ");
+		quote_c_style(other, msg, NULL, 0);
+		strbuf_addch(msg, '\n');
+		break;
+	case DIFF_STATUS_RENAMED:
+		strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
+		strbuf_addstr(msg, "\nrename from ");
+		quote_c_style(name, msg, NULL, 0);
+		strbuf_addstr(msg, "\nrename to ");
+		quote_c_style(other, msg, NULL, 0);
+		strbuf_addch(msg, '\n');
+		break;
+	case DIFF_STATUS_MODIFIED:
+		if (p->score) {
+			strbuf_addf(msg, "dissimilarity index %d%%\n",
+				    similarity_index(p));
+			break;
+		}
+		/* fallthru */
+	default:
+		/* nothing */
+		;
+	}
+	if (one && two && hashcmp(one->sha1, two->sha1)) {
+		int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
+
+		if (DIFF_OPT_TST(o, BINARY)) {
+			mmfile_t mf;
+			if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
+			    (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
+				abbrev = 40;
+		}
+		strbuf_addf(msg, "index %.*s..%.*s",
+			    abbrev, sha1_to_hex(one->sha1),
+			    abbrev, sha1_to_hex(two->sha1));
+		if (one->mode == two->mode)
+			strbuf_addf(msg, " %06o", one->mode);
+		strbuf_addch(msg, '\n');
+	}
+	if (msg->len)
+		strbuf_setlen(msg, msg->len - 1);
+}
+
 static void run_diff_cmd(const char *pgm,
 			 const char *name,
 			 const char *other,
 			 const char *attr_path,
 			 struct diff_filespec *one,
 			 struct diff_filespec *two,
-			 const char *xfrm_msg,
+			 struct strbuf *msg,
 			 struct diff_options *o,
-			 int complete_rewrite)
+			 struct diff_filepair *p)
 {
+	const char *xfrm_msg = NULL;
+	int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
+
+	if (msg) {
+		fill_metainfo(msg, name, other, one, two, o, p);
+		xfrm_msg = msg->len ? msg->buf : NULL;
+	}
+
 	if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
 		pgm = NULL;
 	else {
@@ -2048,11 +2118,6 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
 		hashclr(one->sha1);
 }
 
-static int similarity_index(struct diff_filepair *p)
-{
-	return p->score * 100 / MAX_SCORE;
-}
-
 static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
 {
 	/* Strip the prefix but do not molest /dev/null and absolute paths */
@@ -2066,13 +2131,11 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 {
 	const char *pgm = external_diff();
 	struct strbuf msg;
-	char *xfrm_msg;
 	struct diff_filespec *one = p->one;
 	struct diff_filespec *two = p->two;
 	const char *name;
 	const char *other;
 	const char *attr_path;
-	int complete_rewrite = 0;
 
 	name  = p->one->path;
 	other = (strcmp(name, p->two->path) ? p->two->path : NULL);
@@ -2082,83 +2145,34 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 
 	if (DIFF_PAIR_UNMERGED(p)) {
 		run_diff_cmd(pgm, name, NULL, attr_path,
-			     NULL, NULL, NULL, o, 0);
+			     NULL, NULL, NULL, o, p);
 		return;
 	}
 
 	diff_fill_sha1_info(one);
 	diff_fill_sha1_info(two);
 
-	strbuf_init(&msg, PATH_MAX * 2 + 300);
-	switch (p->status) {
-	case DIFF_STATUS_COPIED:
-		strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
-		strbuf_addstr(&msg, "\ncopy from ");
-		quote_c_style(name, &msg, NULL, 0);
-		strbuf_addstr(&msg, "\ncopy to ");
-		quote_c_style(other, &msg, NULL, 0);
-		strbuf_addch(&msg, '\n');
-		break;
-	case DIFF_STATUS_RENAMED:
-		strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
-		strbuf_addstr(&msg, "\nrename from ");
-		quote_c_style(name, &msg, NULL, 0);
-		strbuf_addstr(&msg, "\nrename to ");
-		quote_c_style(other, &msg, NULL, 0);
-		strbuf_addch(&msg, '\n');
-		break;
-	case DIFF_STATUS_MODIFIED:
-		if (p->score) {
-			strbuf_addf(&msg, "dissimilarity index %d%%\n",
-					similarity_index(p));
-			complete_rewrite = 1;
-			break;
-		}
-		/* fallthru */
-	default:
-		/* nothing */
-		;
-	}
-
-	if (hashcmp(one->sha1, two->sha1)) {
-		int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
-
-		if (DIFF_OPT_TST(o, BINARY)) {
-			mmfile_t mf;
-			if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
-			    (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
-				abbrev = 40;
-		}
-		strbuf_addf(&msg, "index %.*s..%.*s",
-				abbrev, sha1_to_hex(one->sha1),
-				abbrev, sha1_to_hex(two->sha1));
-		if (one->mode == two->mode)
-			strbuf_addf(&msg, " %06o", one->mode);
-		strbuf_addch(&msg, '\n');
-	}
-
-	if (msg.len)
-		strbuf_setlen(&msg, msg.len - 1);
-	xfrm_msg = msg.len ? msg.buf : NULL;
-
 	if (!pgm &&
 	    DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
 	    (S_IFMT & one->mode) != (S_IFMT & two->mode)) {
-		/* a filepair that changes between file and symlink
+		/*
+		 * a filepair that changes between file and symlink
 		 * needs to be split into deletion and creation.
 		 */
 		struct diff_filespec *null = alloc_filespec(two->path);
 		run_diff_cmd(NULL, name, other, attr_path,
-			     one, null, xfrm_msg, o, 0);
+			     one, null, &msg, o, p);
 		free(null);
+		strbuf_release(&msg);
+
 		null = alloc_filespec(one->path);
 		run_diff_cmd(NULL, name, other, attr_path,
-			     null, two, xfrm_msg, o, 0);
+			     null, two, &msg, o, p);
 		free(null);
 	}
 	else
 		run_diff_cmd(pgm, name, other, attr_path,
-			     one, two, xfrm_msg, o, complete_rewrite);
+			     one, two, &msg, o, p);
 
 	strbuf_release(&msg);
 }
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 2f27a0ba9e..a3f0897a52 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -104,7 +104,7 @@ cat >expect.typechange <<'EOF'
 -1
 diff --git a/file b/file
 new file mode 120000
-index ad8b3d2..67be421
+index 0000000..67be421
 --- /dev/null
 +++ b/file
 @@ -0,0 +1 @@
diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh
index 55334927ab..0f185caa44 100755
--- a/t/t4114-apply-typechange.sh
+++ b/t/t4114-apply-typechange.sh
@@ -25,6 +25,10 @@ test_expect_success 'setup repository and commits' '
 	git update-index foo &&
 	git commit -m "foo back to file" &&
 	git branch foo-back-to-file &&
+	printf "\0" > foo &&
+	git update-index foo &&
+	git commit -m "foo becomes binary" &&
+	git branch foo-becomes-binary &&
 	rm -f foo &&
 	git update-index --remove foo &&
 	mkdir foo &&
@@ -85,6 +89,20 @@ test_expect_success 'symlink becomes file' '
 	'
 test_debug 'cat patch'
 
+test_expect_success 'binary file becomes symlink' '
+	git checkout -f foo-becomes-binary &&
+	git diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&
+	git apply --index < patch
+	'
+test_debug 'cat patch'
+
+test_expect_success 'symlink becomes binary file' '
+	git checkout -f foo-symlinked-to-bar &&
+	git diff-tree -p --binary HEAD foo-becomes-binary > patch &&
+	git apply --index < patch
+	'
+test_debug 'cat patch'
+
 
 test_expect_success 'symlink becomes directory' '
 	git checkout -f foo-symlinked-to-bar &&