threads / discuss / 16833

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

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

## tl;dr

5 messages between Dec 22, 2008 and Dec 23, 2008.

replies: 4people: 3as markdown or json

Constantine Plotnikov· Dec 22, 2008, 14:29 UTC · lore

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

Junio C Hamano· Dec 22, 2008, 20:48 UTC · re: Constantine Plotnikov · lore

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

"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.
Johannes Schindelin· Dec 22, 2008, 22:10 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Dec 22, 2008, 23:12 UTC · re: Johannes Schindelin · lore

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

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?

Johannes Schindelin· Dec 23, 2008, 16:20 UTC · re: Junio C Hamano · lore

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

Hi,
On Mon, 22 Dec 2008, Junio C Hamano wrote:
Show 8 quoted lines
> 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.

Show 5 quoted lines
> 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

← back to recent threads