[PATCH] rerere: demonstrate a weakness with identical conflicts in different files

[PATCH] rerere: demonstrate a weakness with identical conflicts in different files

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

When code sections are duplicated in different files and identical
conflicts arise simultaneously in these sections, then rerere resolves only
the first one and leaves the others unresolved.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 I encounter this situation several times a year, and every now and then
 it is particularly annoying because the conflicts are extensive.

 I would appreciate if you could have a look whether this is fixed
 easily. I tried myself, but I can't find the point where the
 subsequent conflicts are ignored.

 -- Hannes

 t/t4208-rerere-dup.sh |   77 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100755 t/t4208-rerere-dup.sh

diff --git a/t/t4208-rerere-dup.sh b/t/t4208-rerere-dup.sh
new file mode 100755
index 0000000..34c182a
--- /dev/null
+++ b/t/t4208-rerere-dup.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Johannes Sixt
+#
+
+test_description='test git rerere when sections of files are duplicated,
+and identical conflicts arise in these sections simultaneously.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	cat > a1 <<- EOF &&
+	alpha
+	beta
+	gamma
+	EOF
+	sed -e s/alpha/ALPHA/ <a1 >a2 &&
+
+	git add a1 a2 &&
+	git commit -q -a -m initial &&
+
+	git checkout -b first &&
+	cat > a1 <<- EOF &&
+	alpha
+	BETA
+	gamma
+	EOF
+	sed -e s/alpha/ALPHA/ <a1 >a2 &&
+	git commit -q -a -m first &&
+
+	git checkout master &&
+	cat > a1 <<- EOF &&
+	alpha
+	----
+	gamma
+	EOF
+	sed -e s/alpha/ALPHA/ <a1 >a2 &&
+	git commit -q -a -m master &&
+	git config rerere.enabled true
+'
+
+test_expect_success 'merge records only one conflict' '
+	test_must_fail git merge first &&
+	sha1=$(ls .git/rr-cache) &&
+	test $(echo $sha1 | wc -w) = 1
+'
+
+test_expect_success 'record a resolution' '
+	cat > a1 <<- EOF &&
+	alpha
+	--beta--
+	gamma
+	EOF
+	sed -e s/alpha/ALPHA/ <a1 >a2 &&
+	git rerere 2>actual
+'
+
+test_expect_success 'postimage must exist' '
+	test -f .git/rr-cache/$sha1/postimage
+'
+
+test_expect_success 'same resolution recorded twice' '
+	test $(grep "Recorded resolution" actual | wc -l) = 2 &&
+	test $(ls .git/rr-cache | wc -w) = 1
+'
+
+test_expect_success 'repeat merge' '
+	git reset --hard &&
+	test_must_fail git merge first
+'
+
+test_expect_failure 'both files are resolved' '
+	! grep ===== a1 a2
+'
+
+test_done
-- 
1.7.2.63.g079547

Re: [PATCH] rerere: demonstrate a weakness with identical conflicts in different files

[Junio C Hamano]

Hmm, my knee-jerk reaction was that something may be keying off of the
conflict ids to keep track of which ones are dealt with and which ones are
yet to be resolved, but I don't recall any part of the implementation that
would do something like that offhand.  Sorry.

Will try to find/make time to take a look at it later.

Re: [PATCH] rerere: demonstrate a weakness with identical conflicts in different files

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Hmm, my knee-jerk reaction was that something may be keying off of the
> conflict ids to keep track of which ones are dealt with and which ones are
> yet to be resolved, but I don't recall any part of the implementation that
> would do something like that offhand.  Sorry.

Heh, what was I thinking.  Yes, rr-cache/ database keys off of the
conflict id, so if your repository has more than one contents that produce
exactly the same conflict, say F and G, then, most likely:

 * You see one of them first, say F, record preimage.F and record its
   resolution as postimage.F

 * You encounter conflict G; record it in thisimage, try three-way merge
   between postimage.F and that using preimage.F as the common ancestor.
   If this doesn't work (and it likely doesn't), rerere punts.

Note that this issue can happen even when the trees you are currently
merging have only content derived from G and nothing related to F, as
their resolutions share the same conflict id.

I vaguely recall discussing this with you here and bringing up a possibile
solution for a situation like this; keep sets of <preimage, postimage>
(starting from the current {pre,post}image adding {pre,post}image.1,
{pre,post}image.2 and so on).  When we see a new conflict (after a mergy
operation before "rerere" resolves it), try to run the three-way merge
with it and each pair, and if we don't find anything that successfully
merges, record it as a new half of the pre-post pair, incrementing the
counter, and record a new resolution to complete the new pair for later
use.

We also need to split "thisimage" into multiple ones in case this happens
in a single tree (i.e. contents derived from F and from G are both present
and produce the conflict in the same merge).

Re: [PATCH] rerere: demonstrate a weakness with identical conflicts in different files

[Johannes Sixt]

Am 8/12/2010 4:50, schrieb Junio C Hamano:
> Yes, rr-cache/ database keys off of the
> conflict id, so if your repository has more than one contents that produce
> exactly the same conflict, say F and G, then, most likely:
> 
>  * You see one of them first, say F, record preimage.F and record its
>    resolution as postimage.F
> 
>  * You encounter conflict G; record it in thisimage, try three-way merge
>    between postimage.F and that using preimage.F as the common ancestor.
>    If this doesn't work (and it likely doesn't), rerere punts.

Aha! Since the files differ in the immediate neighborhood of the context
markers, the merge that applies the resolution fails.

Squash in this and the test passes:

diff --git a/t/t4208-rerere-dup.sh b/t/t4208-rerere-dup.sh
index 34c182a..2afa0ef 100755
--- a/t/t4208-rerere-dup.sh
+++ b/t/t4208-rerere-dup.sh
@@ -12,6 +12,7 @@
 test_expect_success 'setup' '
 	cat > a1 <<- EOF &&
 	alpha
+	delta
 	beta
 	gamma
 	EOF
@@ -23,6 +24,7 @@ test_expect_success 'setup' '
 	git checkout -b first &&
 	cat > a1 <<- EOF &&
 	alpha
+	delta
 	BETA
 	gamma
 	EOF
@@ -32,6 +34,7 @@ test_expect_success 'setup' '
 	git checkout master &&
 	cat > a1 <<- EOF &&
 	alpha
+	delta
 	----
 	gamma
 	EOF
@@ -49,6 +52,7 @@ test_expect_success 'merge records
 test_expect_success 'record a resolution' '
 	cat > a1 <<- EOF &&
 	alpha
+	delta
 	--beta--
 	gamma
 	EOF
@@ -61,7 +65,7 @@ test_expect_success 'postimage must
 '

 test_expect_success 'same resolution recorded twice' '
-	test $(grep "Recorded resolution" actual | wc -l) = 2 &&
+#	test $(grep "Recorded resolution" actual | wc -l) = 2 &&
 	test $(ls .git/rr-cache | wc -w) = 1
 '


The last hunk is necessary because the output of rerere is now:

Recorded resolution for 'a1'.
Resolved 'a2' using previous resolution.

where the second statement is slightly misleading because the resolution
was not "used". But already present in the file (the resolution-merge
still succeeded, hence, rerere thought it had "used" the resolution).

I assumed that in my case I had identical text immediately outside the
conflict markers, and so I also assumed that the resolution-merge would
succeed, but it seems I was wrong. I'll go back and investigate closer as
time permits.

Thanks for your help so far.

-- Hannes