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

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

[Martin von Zweigbergk]

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

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

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> 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

I think the above actually sort-of makes sense.  The first column of
status output is about comparing the HEAD and the index, and the second
column is about comparing the index and the working tree, but I'll begin
by explaining the latter.

When comparing an unmerged index with the working tree, the comparison
itself does not make much sense.  When producing textual diff,
"diff-index" tends to give the difference between stage #2 and the working
tree in order to show the difference from "our" version and the result of
the incomplete merge, but when we need to show the result concisely in the
"status -s" output, the fact that the index is unmerged is more important
than the incomplete merge result in the working tree is different from the
original, so we show "U".

So I think "U" is perfectly good there.

About the comparison between HEAD and index, "original" in HEAD is copied
to "exact-copy" in the index, and "modified-copy" in the index has a very
similar contents as "original" in HEAD. It may be a bug that the latter is
shown as "R" and I suspect that is because the code mistook the unmerged
entry in the index as missing.  Turning that "R" to "C" may be worth
doing. Change the code that says "ah, the index is unmerged at this path,
so treat it as if it is not there" to "if the unmerged path does not have
stage #2 entry, it is missing".

> 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

The prose is good but if you illustrated a bug with a command output,
please follow it up with another command output that you think is the
right output.  It becomes easier to point out potential flaws in the
design and the thought behind it, like I just did above about "U".

I don't think anybody said "only supposed to show renames", but I suspect
that the recent lt/diff-rename topic may affect this part of the 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.

I don't think the part after "Still..." is justified enough.

For that matter, everything after "For simplicity..." is not justified
enough. What are you sacrificing in return for that simplicity?  "Ideally
we should show X but because we don't consider these we end up getting Y
but that still is better than what we get today which is Z" is missing.

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

Because "while at it" is by definition stuff that is not essential, don't
do "white at it" if you are not sure, if it adds unnecessary noise and
burden on reviewers.

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

[Martin von Zweigbergk]

On Thu, 17 Mar 2011, Junio C Hamano wrote:

> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> 
> > 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
> 
> The first column of
> status output is about comparing the HEAD and the index, and the second
> column is about comparing the index and the working tree

According to git-status's man page, that is true for paths without
merge conflicts only. For paths with merge conflicts, the two letters
represent each side of the merge. That is why I thought " U" (note the
SP) was invalid. Reading the documentation again, I see that ' '
actually means 'unmodified'. I'm still not sure it is correct
though. First, "original" was modified on both sides in this case,
which is of course the reason it had a merge conflict. Second, the
meaning of " U" is not described in the table in the man page. I have
always assumed that the table is supposed to be complete.

Either way, the fact that "original" is completetly missing from the
verbose 'git status' output must surely be incorrect.

> When comparing an unmerged index with the working tree, the comparison
> itself does not make much sense.  When producing textual diff,
> "diff-index" tends to give the difference between stage #2 and the working
> tree in order to show the difference from "our" version and the result of
> the incomplete merge, but when we need to show the result concisely in the
> "status -s" output, the fact that the index is unmerged is more important
> than the incomplete merge result in the working tree is different from the
> original, so we show "U".
> 
> So I think "U" is perfectly good there.

For completeness, I should say that I think the output from 'git
diff-files' is correct (and unchanged by this patch); only 'git
status' and 'git diff-index' showed incorrect (as far as I can see)
output.

$ git diff-files --name-status
U	original
M	original

Same output with '-M' or '-C'.

> About the comparison between HEAD and index, "original" in HEAD is copied
> to "exact-copy" in the index, and "modified-copy" in the index has a very
> similar contents as "original" in HEAD. It may be a bug that the latter is
> shown as "R" and I suspect that is because the code mistook the unmerged
> entry in the index as missing.  Turning that "R" to "C" may be worth
> doing. Change the code that says "ah, the index is unmerged at this path,
> so treat it as if it is not there" to "if the unmerged path does not have
> stage #2 entry, it is missing".

Exactly. That is what this patch tries to do (although not for 'git
status', but for 'git diff-index --cached -C', see later answers).

Speaking of stage #2, I think my references to stage #1 in the commit
message should have been referring to stage #2.

> > 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
> 
> The prose is good but if you illustrated a bug with a command output,
> please follow it up with another command output that you think is the
> right output.

Makes sense. This is the output after this patch:

$ git status -s
A  exact-copy
A  modified-copy
UU original

> I don't think anybody said "only supposed to show renames", but I suspect
> that the recent lt/diff-rename topic may affect this part of the output.

wt-status.c sets rev.diffopt.detect_rename = 1 (==
DIFF_DETECT_RENAME). I think that means that it should only detect
rename, not copies. With the patch applies, the files show up as added
(as show in above). I tried changing it to DIFF_DETECT_COPY and then,
not surprisingly, 'git status' showed them as copies instead (even
with the patch applied).

While on the subject, I almost asked why 'git status' does not detect
copies, but I never did. I kind of suspected that it might be too
slow. Could there be some other reason?

> > 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.
> 
> I don't think the part after "Still..." is justified enough.
> 
> For that matter, everything after "For simplicity..." is not justified
> enough. What are you sacrificing in return for that simplicity?  "Ideally
> we should show X but because we don't consider these we end up getting Y
> but that still is better than what we get today which is Z" is missing.

I wasn't really sure how it should work, so I just punted and said
"For simplicity" :-). What I was not sure about was what stage to
compare to. For example, for "deleted by us" conflicts, the most
likely outcome is that the file will be deleted, so it could make some
sense to make it eligible for rename detection. This would mean that
if a similar file was added by "them", it would be described as a
rename in the git-status output. The same applies to "deleted by us",
but if I understand correctly, we don't have the stage #3 content
available to compare to at this point in the code, and even if we did,
it would be inconsistent with how the diff is done with stage #2
everywhere else.

I don't have time to think more about this right now, but I hope the
above explains what I simplfied from. I'll think a bit more about this
later and might get back with another mail.

> > Also not sure about the "while at it" stuff...
> 
> Because "while at it" is by definition stuff that is not essential, don't
> do "white at it" if you are not sure, if it adds unnecessary noise and
> burden on reviewers.

I changed it because I thought it is slightly more clear that
DIFF_DETECT_RENAME, as opposed to 1, does not detect renames. I could
either explain that reason in the commit message, do it in a separate
patch, or just drop it completely. I don't have a strong
opinion. Which would you prefer?

Thanks for reviewing. I will try to include much of the above in the
commit message in a re-roll.


/Martin

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

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> 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?

I re-read the patch and found that the core of the patch, i.e. the change
to diffcore-rename.c, is basically sound.

I'd like to see the commit log message to describe the cause of the
breakage better.  Perhaps something like this:

  Since e9c8409 (diff-index --cached --raw: show tree entry on the LHS for
  unmerged entries., 2007-01-05), an unmerged entry should be detected by
  using DIFF_PAIR_UNMERGED(p), not by noticing both one and two sides of
  the filepair records mode=0 entries. However, it forgot to update some
  parts of the rename detection logic.

  This only makes difference in the "diff --cached" codepath where an
  unmerged filepair carries information on the entries that came from the
  tree.  It probably hasn't been noticed for a long time because nobody
  would run "diff -M" during a conflict resolution, but "git status" uses
  rename detection when it internally runs "diff-index" and "diff-files"
  and gives nonsense results.

  In an unmerged pair, "one" side can have a valid filespec to record the
  tree entry (e.g. what's in HEAD) when running "diff --cached". This can
  be used as a rename source to other paths in the index that are not
  unmerged. The path that is unmerged by definition does not have the
  final content yet (i.e. "two" side cannot have a valid filespec), so it
  can never be a rename destination.

  Use the DIFF_PAIR_UNMERGED() to detect unmerged filepair correctly, and
  allow the valid "one" side of an unmerged filepair to be considered a
  potential rename source, but never to be considered a rename destination.

Please split changes to wt-status.c (indentation and symbolic constant)
and builtin/commit.c (symbolic constant) to a single commit that is
separate from this fix, as we would want to backport the fix to older
maintenance tracks.

Also, please don't add a new test script 7510 that conflicts what is
already in flight ('pu' has t/t7510-commit-notes.sh).  Instead, tack
something like the following (and your diff-index tests) at the end of
t/t7060-wtstatus.sh, as you should be able to do this without actually
running a merge.

Answering the last question in the comment part of your message, the paths
are supposed to be shown in the name order, so your comparison (and the
comparison below) should be the right thing to do.

Thanks.



test_expect_success 'rename & unmerged setup' '
	git rm -f -r . &&
	cat "$TEST_DIRECTORY/README" >ONE &&
	git add ONE &&
	test_tick &&
	git commit -m "One commit with ONE" &&

	echo Modified >TWO &&
	cat ONE >>TWO &&
	cat ONE >>THREE &&
	git add TWO THREE &&
	sha1=$(git rev-parse :ONE) &&
	git rm --cached ONE &&
	(
		echo "100644 $sha1 1	ONE" &&
		echo "100644 $sha1 2	ONE" &&
		echo "100644 $sha1 3	ONE"
	) | git update-index --index-info &&
	echo Further >>THREE
'

test_expect_success 'rename & unmerged status' '
	git status -suno >actual &&
	cat >expect <<-EOF &&
	UU ONE
	AM THREE
	A  TWO
	EOF
	test_cmp expect actual
'

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

[Martin von Zweigbergk]

On Mon, 21 Mar 2011, Junio C Hamano wrote:

> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> 
> > 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?
> 
> I re-read the patch and found that the core of the patch, i.e. the change
> to diffcore-rename.c, is basically sound.
> 
> I'd like to see the commit log message to describe the cause of the
> breakage better.  Perhaps something like this:
> 
>   Since e9c8409 (diff-index --cached --raw: show tree entry on the LHS for
>   unmerged entries., 2007-01-05), an unmerged entry should be detected by
>   using DIFF_PAIR_UNMERGED(p), not by noticing both one and two sides of
>   the filepair records mode=0 entries. However, it forgot to update some
>   parts of the rename detection logic.
> 
>   This only makes difference in the "diff --cached" codepath where an
>   unmerged filepair carries information on the entries that came from the
>   tree.  It probably hasn't been noticed for a long time because nobody
>   would run "diff -M" during a conflict resolution, but "git status" uses
>   rename detection when it internally runs "diff-index" and "diff-files"
>   and gives nonsense results.
> 
>   In an unmerged pair, "one" side can have a valid filespec to record the
>   tree entry (e.g. what's in HEAD) when running "diff --cached". This can
>   be used as a rename source to other paths in the index that are not
>   unmerged. The path that is unmerged by definition does not have the
>   final content yet (i.e. "two" side cannot have a valid filespec), so it
>   can never be a rename destination.
> 
>   Use the DIFF_PAIR_UNMERGED() to detect unmerged filepair correctly, and
>   allow the valid "one" side of an unmerged filepair to be considered a
>   potential rename source, but never to be considered a rename destination.

Thanks. I really appreciate it. My commit message did feel a little
lazy and I didn't know the history.

> Please split changes to wt-status.c (indentation and symbolic constant)
> and builtin/commit.c (symbolic constant) to a single commit that is
> separate from this fix, as we would want to backport the fix to older
> maintenance tracks.

Ah, of course. I did think this should go to maint, so it seems stupid
not to have split it. Will do.

> Also, please don't add a new test script 7510 that conflicts what is
> already in flight ('pu' has t/t7510-commit-notes.sh).

Sorry, I missed that and thanks for the help below. I have not had
much time lately, but I will do my best to send a re-roll in not too
long.

>  Instead, tack
> something like the following (and your diff-index tests) at the end of
> t/t7060-wtstatus.sh, as you should be able to do this without actually
> running a merge.
> 
> Answering the last question in the comment part of your message, the paths
> are supposed to be shown in the name order, so your comparison (and the
> comparison below) should be the right thing to do.
> 
> Thanks.
> 
> 
> 
> test_expect_success 'rename & unmerged setup' '
> 	git rm -f -r . &&
> 	cat "$TEST_DIRECTORY/README" >ONE &&
> 	git add ONE &&
> 	test_tick &&
> 	git commit -m "One commit with ONE" &&
> 
> 	echo Modified >TWO &&
> 	cat ONE >>TWO &&
> 	cat ONE >>THREE &&
> 	git add TWO THREE &&
> 	sha1=$(git rev-parse :ONE) &&
> 	git rm --cached ONE &&
> 	(
> 		echo "100644 $sha1 1	ONE" &&
> 		echo "100644 $sha1 2	ONE" &&
> 		echo "100644 $sha1 3	ONE"
> 	) | git update-index --index-info &&
> 	echo Further >>THREE
> '
> 
> test_expect_success 'rename & unmerged status' '
> 	git status -suno >actual &&
> 	cat >expect <<-EOF &&
> 	UU ONE
> 	AM THREE
> 	A  TWO
> 	EOF
> 	test_cmp expect actual
> '
> 

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

[Martin von Zweigbergk]

Since e9c8409 (diff-index --cached --raw: show tree entry on the LHS for
unmerged entries., 2007-01-05), an unmerged entry should be detected by
using DIFF_PAIR_UNMERGED(p), not by noticing both one and two sides of
the filepair records mode=0 entries. However, it forgot to update some
parts of the rename detection logic.

This only makes difference in the "diff --cached" codepath where an
unmerged filepair carries information on the entries that came from the
tree.  It probably hasn't been noticed for a long time because nobody
would run "diff -M" during a conflict resolution, but "git status" uses
rename detection when it internally runs "diff-index" and "diff-files"
and gives nonsense results.

In an unmerged pair, "one" side can have a valid filespec to record the
tree entry (e.g. what's in HEAD) when running "diff --cached". This can
be used as a rename source to other paths in the index that are not
unmerged. The path that is unmerged by definition does not have the
final content yet (i.e. "two" side cannot have a valid filespec), so it
can never be a rename destination.

Use the DIFF_PAIR_UNMERGED() to detect unmerged filepair correctly, and
allow the valid "one" side of an unmerged filepair to be considered a
potential rename source, but never to be considered a rename destination.

Commit message and first two test cases by Junio, the rest by Martin.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
I think I mean something slightly different when I say "don't consider
unmerged path as source" than you do when you say "allow the valid
"one" side of an unmerged filepair to be considered a potential rename
source". I mean that they shouldn't show up on the LHS of a "copied: A
-> B" line in git-status output. I think you mean that they should be
registered as rename sources, even though the destination will always
be the file itself, so other files will only end up having status
COPIED. I didn't know how phrase the subject line in a better way, but
feel free to change it as you like.


 diffcore-rename.c   |    7 ++++-
 t/t7060-wtstatus.sh |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

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/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index fcac472..48b751e 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -56,4 +56,63 @@ test_expect_success 'M/D conflict does not segfault' '
 	)
 '
 
+test_expect_success 'rename & unmerged setup' '
+	git rm -f -r . &&
+	cat "$TEST_DIRECTORY/README" >ONE &&
+	git add ONE &&
+	test_tick &&
+	git commit -m "One commit with ONE" &&
+
+	echo Modified >TWO &&
+	cat ONE >>TWO &&
+	cat ONE >>THREE &&
+	git add TWO THREE &&
+	sha1=$(git rev-parse :ONE) &&
+	git rm --cached ONE &&
+	(
+		echo "100644 $sha1 1	ONE" &&
+		echo "100644 $sha1 2	ONE" &&
+		echo "100644 $sha1 3	ONE"
+	) | git update-index --index-info &&
+	echo Further >>THREE
+'
+
+test_expect_success 'rename & unmerged status' '
+	git status -suno >actual &&
+	cat >expect <<-EOF &&
+	UU ONE
+	AM THREE
+	A  TWO
+	EOF
+	test_cmp expect actual
+'
+
+cat >expected <<\EOF
+U	ONE
+A	THREE
+A	TWO
+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
+U	ONE
+C	ONE	THREE
+C	ONE	TWO
+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
-- 
1.7.4.79.gcbe20