git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date
Mar 18, 2011, 01:42 UTC
Message-ID
<1300412548-1724-1-git-send-email-martin.von.zweigbergk@gmail.com>

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@gmail.com>
---

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
Next: Junio C Hamano
Message 1 of 6 in “diffcore-rename: don't consider unmerged path as source”
  1. diffcore-rename: don't consider unmerged path as sourceMartin von Zweigbergk, Mar 18, 2011
  2. Junio C HamanoMar 18, 2011
  3. Martin von ZweigbergkMar 18, 2011
  4. Junio C HamanoMar 21, 2011
  5. Martin von ZweigbergkMar 24, 2011
  6. diffcore-rename: don't consider unmerged path as sourceMartin von Zweigbergk, Mar 24, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.