[PATCH] diffcore-rename: don't consider unmerged path as source

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The output from 'git status' and 'git diff-index --cached -M' is
broken when there are unmerged files as well as new files similar to
the unmerged file (in stage 1).

When two copies have been created the output is:

$ git status -s
C  original -> exact-copy
R  original -> modified-copy
 U original

There are several errors here: 1) "original" has invalid status " U",
2) "modified-copy" is listed as a rename, even though its source
("original") still exists, and 3) "exact-copy" is detected as a copy,
even though 'git status' is only supposed to show renames

In the verbose 'git status' output, the unmerged path is completely
missing. 'git diff-index --cached -M' show similar output.

Fix these problems by making diffcore-rename not consider unmerged
paths as source for rename detection. For simplicty, don't consider
the path independent of the type of merge conflict, even if the path
is deleted on at least one side. Still consider unmerged paths as
source for copy detection.

While at it, update the users of diff_options to use
DIFF_DETECT_RENAME instead of directly using the value 1 to enable
rename detection.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@xxxxxxxxx>
---

This is the first time I look at the diff code, so please review
carefully. I think the changes make sense, but I really don't know the
code enough to be sure.

Also not sure about the "while at it" stuff...

The test cases assume that the paths will be printed in a certain
order. Can I rely on that?

 builtin/commit.c                    |    2 +-
 diffcore-rename.c                   |    7 +++-
 t/t7510-status-merge-rename-copy.sh |   68 +++++++++++++++++++++++++++++++++++
 wt-status.c                         |    6 ++--
 4 files changed, 77 insertions(+), 6 deletions(-)
 create mode 100755 t/t7510-status-merge-rename-copy.sh

diff --git a/builtin/commit.c b/builtin/commit.c
index 82092e5..1f9f314 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1284,7 +1284,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 	rev.show_root_diff = 1;
 	get_commit_format(format.buf, &rev);
 	rev.always_show_header = 0;
-	rev.diffopt.detect_rename = 1;
+	rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
 	rev.diffopt.rename_limit = 100;
 	rev.diffopt.break_opt = 0;
 	diff_setup_done(&rev.diffopt);
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 0cd4c13..c53ca36 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -464,7 +464,7 @@ void diffcore_rename(struct diff_options *options)
 			else
 				locate_rename_dst(p->two, 1);
 		}
-		else if (!DIFF_FILE_VALID(p->two)) {
+		else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {
 			/*
 			 * If the source is a broken "delete", and
 			 * they did not really want to get broken,
@@ -575,7 +575,10 @@ void diffcore_rename(struct diff_options *options)
 		struct diff_filepair *p = q->queue[i];
 		struct diff_filepair *pair_to_free = NULL;
 
-		if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
+		if (DIFF_PAIR_UNMERGED(p)) {
+			diff_q(&outq, p);
+		}
+		else if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
 			/*
 			 * Creation
 			 *
diff --git a/t/t7510-status-merge-rename-copy.sh b/t/t7510-status-merge-rename-copy.sh
new file mode 100755
index 0000000..8c7a4d2
--- /dev/null
+++ b/t/t7510-status-merge-rename-copy.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+#
+# Copyright (c) 2011 Martin von Zweigbergk
+#
+
+test_description='git status during merge with copies'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo a b c | xargs -n1 >original &&
+	git add original &&
+	git commit -m initial &&
+
+	cp original exact-copy &&
+	cp original modified-copy &&
+	echo x >>original &&
+	echo y >>modified-copy &&
+	git add original exact-copy modified-copy &&
+	git commit -m "create copies and modify original on master" &&
+
+	git checkout -b topic HEAD^ &&
+	echo z >>original &&
+	git add original &&
+	git commit -m "modify original on topic" &&
+	test_must_fail git merge master
+'
+
+cat >expected <<\EOF
+A  exact-copy
+A  modified-copy
+UU original
+EOF
+
+test_expect_success 'git status -s shows 2 added + 1 unmerged' '
+	git status -s -uno >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+A	exact-copy
+A	modified-copy
+U	original
+EOF
+
+test_expect_success 'git diff-index --cached shows 2 added + 1 unmerged' '
+	git diff-index --cached --name-status HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git diff-index --cached -M shows 2 added + 1 unmerged' '
+	git diff-index --cached --name-status HEAD >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+C	original	exact-copy
+C	original	modified-copy
+U	original
+EOF
+
+test_expect_success 'git diff-index --cached -C shows 2 copies + 1 unmerged' '
+	git diff-index --cached -C --name-status HEAD |
+		sed "s/^C[0-9]*/C/g" >actual &&
+	test_cmp expected actual
+'
+
+test_done
diff --git a/wt-status.c b/wt-status.c
index 4daa8bb..532bbcc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -320,7 +320,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	if (s->ignore_submodule_arg) {
 		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
-    }
+	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
 	init_pathspec(&rev.prune_data, s->pathspec);
@@ -345,7 +345,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = wt_status_collect_updated_cb;
 	rev.diffopt.format_callback_data = s;
-	rev.diffopt.detect_rename = 1;
+	rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
 	rev.diffopt.rename_limit = 200;
 	rev.diffopt.break_opt = 0;
 	init_pathspec(&rev.prune_data, s->pathspec);
@@ -597,7 +597,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, &opt);
 
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
-	rev.diffopt.detect_rename = 1;
+	rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
 	rev.diffopt.file = s->fp;
 	rev.diffopt.close_file = 0;
 	/*
-- 
1.7.4.79.gcbe20

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]