Move-delete merge conflict is not displayed using git ls-files --unmerged

Move-delete merge conflict is not displayed using git ls-files --unmerged

[Constantine Plotnikov]

Lets consider repository state created with the following script (git
version 1.6.0.4).

mkdir test-move-delete
cd test-move-delete
git init
echo test1 >file.txt
echo test2 >>file.txt
echo test3 >>file.txt
echo test4 >>file.txt
git add file.txt
git commit -m "start"
git checkout -b moved master
git mv file.txt copy.txt
git commit -m "moved"
git checkout -b deleted master
git rm file.txt
git commit -m "deleted"
git merge moved

The last merge command fails with move-delete conflict.

CONFLICT (rename/delete): Renamed file.txt->copy.txt in moved and
deleted in HEAD
Automatic merge failed; fix conflicts and then commit the result.

However git ls-files --unmerged does not list any conflict in that
case and it is possible to execute git commit command right away
without doing anything with the repository.

I think that if git merge reports the conflicts, such conflicts should
be discoverable using git ls-files and prevent commit with resolving
the conflict like it is done with modify-delete conflicts.

Regards,
Constantine

Re: Move-delete merge conflict is not displayed using git ls-files --unmerged

[Junio C Hamano]

"Constantine Plotnikov" <constantine.plotnikov@gmail.com> writes:

> I think that if git merge reports the conflicts, such conflicts should
> be discoverable using git ls-files and prevent commit with resolving
> the conflict like it is done with modify-delete conflicts.

Yeah, I think so, too.

[PATCH] merge-recursive: mark rename/delete conflict as unmerged

[Johannes Schindelin]

When a file was renamed in one branch, but deleted in the other, one
should expect the index to contain an unmerged entry, namely the
target of the rename.  Make it so.

Noticed by Constantine Plotnikov.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 22 Dec 2008, Junio C Hamano wrote:

	> "Constantine Plotnikov" <constantine.plotnikov@gmail.com> writes:
	> 
	> > I think that if git merge reports the conflicts, such 
	> > conflicts should be discoverable using git ls-files and prevent 
	> > commit with resolving the conflict like it is done with
	> > modify-delete conflicts.
	> 
	> Yeah, I think so, too.

	A test case would have been nice.

 merge-recursive.c          |    5 +++++
 t/t6024-recursive-merge.sh |   23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index a0c804c..69e7152 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -902,6 +902,11 @@ static int process_renames(struct merge_options *o,
 				       ren1_src, ren1_dst, branch1,
 				       branch2);
 				update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
+				update_stages(ren1_dst, NULL,
+						branch1 == o->branch1 ?
+						ren1->pair->two : NULL,
+						branch1 == o->branch1 ?
+						NULL : ren1->pair->two, 1);
 			} else if (!sha_eq(dst_other.sha1, null_sha1)) {
 				const char *new_path;
 				clean_merge = 0;
diff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh
index 802d0d0..129fa30 100755
--- a/t/t6024-recursive-merge.sh
+++ b/t/t6024-recursive-merge.sh
@@ -97,4 +97,27 @@ test_expect_success 'refuse to merge binary files' '
 		merge.err
 '
 
+test_expect_success 'mark rename/delete as unmerged' '
+
+	git reset --hard &&
+	git checkout -b delete &&
+	git rm a1 &&
+	test_tick &&
+	git commit -m delete &&
+	git checkout -b rename HEAD^ &&
+	git mv a1 a2
+	test_tick &&
+	git commit -m rename &&
+	test_must_fail git merge delete &&
+	test 1 = $(git ls-files --unmerged | wc -l) &&
+	git rev-parse --verify :2:a2 &&
+	test_must_fail git rev-parse --verify :3:a2 &&
+	git checkout -f delete &&
+	test_must_fail git merge rename &&
+	test 1 = $(git ls-files --unmerged | wc -l) &&
+	test_must_fail git rev-parse --verify :2:a2 &&
+	git rev-parse --verify :3:a2
+
+'
+
 test_done
-- 
1.6.1.rc3.412.ga72b

Re: [PATCH] merge-recursive: mark rename/delete conflict as unmerged

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When a file was renamed in one branch, but deleted in the other, one
> should expect the index to contain an unmerged entry, namely the
> target of the rename.  Make it so.

That was quick, but the surrounding code makes me wonder if other
if/elseif branches also need similar handling.

For example, rename/add comes up with a new name that does not exist
anywhere, and adds both to the index; it is understandable that you need
to do this when processing a merge with non-zero depth because you need to
have a tree as the result, but shouldn't the final zero depth merge just
use the original names and leave the results in higher stages?

Re: [PATCH] merge-recursive: mark rename/delete conflict as unmerged

[Johannes Schindelin]

Hi,

On Mon, 22 Dec 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When a file was renamed in one branch, but deleted in the other, one 
> > should expect the index to contain an unmerged entry, namely the 
> > target of the rename.  Make it so.
> 
> That was quick, but the surrounding code makes me wonder if other
> if/elseif branches also need similar handling.

I tried to make sure that the surrounding code also adds unmerged entries, 
but I have to admit that my focus lay with the bug report at hand.

> For example, rename/add comes up with a new name that does not exist 
> anywhere, and adds both to the index; it is understandable that you need 
> to do this when processing a merge with non-zero depth because you need 
> to have a tree as the result, but shouldn't the final zero depth merge 
> just use the original names and leave the results in higher stages?

Hmm.  I think this is a different issue (if you mean the issue that the 
result might be named differently than expected).

As for the unmerged entries, no, I do not think we need to do anything 
else besides calling update_file() and set try_merge = 1.

After all, if a file was renamed in one branch, but added directly in 
another, does it need to conflict?  I'd say no.

Ciao,
Dscho