rerere affects handling of git-stash-pop merge conflicts

rerere affects handling of git-stash-pop merge conflicts

[Phil Hord]

I was investigating a git-stash-pop anomaly when I ran across this one.
I think this is residual breakage from [1] bb0a484 (mergetool: Skip
autoresolved paths, 2010-08-17).  A similar problem aimed at 'git merge'
was ostensibly fixed by [2] 2f59c9 (mergetool: don't skip modify/remove
conflicts, 2011-02-16).

[1] http://thread.gmane.org/gmane.comp.version-control.git/153420
[2] http://thread.gmane.org/gmane.comp.version-control.git/164622


Summary:
After a 'git stash pop' with conflicts, 'git mergetool' fails to notice
the conflicted files if 'rerere.enabled=true'.  git mergetool
erroneously reports 'no files need merging'.

After a 'git merge' with conflicts, 'git mergetool' seems to work ok in
either case.

Here's my test script:

------8<------ /tmp/stash.sh
#!/bin/bash

#enable/disable rerere for this test
git config --global rerere.enabled ${1:false}

# run the test
rm -rf foo
mkdir foo && cd foo && git init &&
echo A>file && git add file && git commit --quiet -m "A" &&
echo B>file && git stash &&
echo C>file && git add file && git commit --quiet -m "C" &&
git stash pop

git mergetool
------8<------ /tmp/stash.sh

git --version
    git version 1.7.6.178.g55272

# With rerere.enabled=false, it works ok (mergetool offers to
# merge the conflicting file):
./stash.sh false

    Initialized empty Git repository in /tmp/foo/.git/
    Saved working directory and index state WIP on master: ac67a86 A
    HEAD is now at ac67a86 A
    Auto-merging file
    CONFLICT (content): Merge conflict in file
    merge tool candidates: meld opendiff kdiff3 tkdiff xxdiff
    tortoisemerge gvimdiff diffuse ecmerge p4merge araxis bc3 vimdiff
    emerge
    Merging:
    file

    Normal merge conflict for 'file':
      {local}: modified
      {remote}: modified
    Hit return to start merge resolution tool (xxdiff): ^C

# But with rerere.enabled=true, mergetool thinks there is no conflict:
./stash.sh true

    Initialized empty Git repository in /tmp/foo/.git/
    Saved working directory and index state WIP on master: d40e77b A
    HEAD is now at d40e77b A
    Auto-merging file
    CONFLICT (content): Merge conflict in file
    merge tool candidates: meld opendiff kdiff3 tkdiff xxdiff
    tortoisemerge gvimdiff diffuse ecmerge p4merge araxis bc3 vimdiff
    emerge
    No files need merging

Phil

Re: rerere affects handling of git-stash-pop merge conflicts

[Phil Hord]

git rerere is misleading us after a conflicted 'git stash pop'.  There 
_is_ an unresolved merge conflict, but .git/MERGE_RR exists and is 
empty, even though there is no previously recorded resolution.
This causes git mergetool to skip over our conflicted file since he now 
uses 'git rerere remaining' to find the unresolved conflicts.

I think one of these is true, but I'm not sure which one.

   A. rerere _should not_ be triggered at all by "stash pop" conflicts
or
   B. rerere _should_ be correctly and completely triggered by "stash 
pop" conflicts
or
   C. 'git mergetool' should not be expected to work after a failed "git 
stash pop"

I'm hoping the right answer is B, but I would also accept A.

fwiw - 'git stash pop' calls 'merge-recursive' at the end, and this is 
where the problem appears.  I have not decoded git-merge-recursive's use 
there enough to figure out where it's gone wrong.

Phil


On 07/14/2011 01:09 PM, Phil Hord wrote:
> I was investigating a git-stash-pop anomaly when I ran across this one.
> I think this is residual breakage from [1] bb0a484 (mergetool: Skip
> autoresolved paths, 2010-08-17).  A similar problem aimed at 'git merge'
> was ostensibly fixed by [2] 2f59c9 (mergetool: don't skip modify/remove
> conflicts, 2011-02-16).
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/153420
> [2] http://thread.gmane.org/gmane.comp.version-control.git/164622
>
>
> Summary:
> After a 'git stash pop' with conflicts, 'git mergetool' fails to notice
> the conflicted files if 'rerere.enabled=true'.  git mergetool
> erroneously reports 'no files need merging'.
>
> After a 'git merge' with conflicts, 'git mergetool' seems to work ok in
> either case.
>
> Here's my test script:
>
> ------8<------ /tmp/stash.sh
> #!/bin/bash
>
> #enable/disable rerere for this test
> git config --global rerere.enabled ${1:false}
>
> # run the test
> rm -rf foo
> mkdir foo&&  cd foo&&  git init&&
> echo A>file&&  git add file&&  git commit --quiet -m "A"&&
> echo B>file&&  git stash&&
> echo C>file&&  git add file&&  git commit --quiet -m "C"&&
> git stash pop
>
> git mergetool
> ------8<------ /tmp/stash.sh
>
> git --version
>      git version 1.7.6.178.g55272
>
> # With rerere.enabled=false, it works ok (mergetool offers to
> # merge the conflicting file):
> ./stash.sh false
>
>      Initialized empty Git repository in /tmp/foo/.git/
>      Saved working directory and index state WIP on master: ac67a86 A
>      HEAD is now at ac67a86 A
>      Auto-merging file
>      CONFLICT (content): Merge conflict in file
>      merge tool candidates: meld opendiff kdiff3 tkdiff xxdiff
>      tortoisemerge gvimdiff diffuse ecmerge p4merge araxis bc3 vimdiff
>      emerge
>      Merging:
>      file
>
>      Normal merge conflict for 'file':
>        {local}: modified
>        {remote}: modified
>      Hit return to start merge resolution tool (xxdiff): ^C
>
> # But with rerere.enabled=true, mergetool thinks there is no conflict:
> ./stash.sh true
>
>      Initialized empty Git repository in /tmp/foo/.git/
>      Saved working directory and index state WIP on master: d40e77b A
>      HEAD is now at d40e77b A
>      Auto-merging file
>      CONFLICT (content): Merge conflict in file
>      merge tool candidates: meld opendiff kdiff3 tkdiff xxdiff
>      tortoisemerge gvimdiff diffuse ecmerge p4merge araxis bc3 vimdiff
>      emerge
>      No files need merging
>
> Phil
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC] Add two basic tests for "rerere remaining"

[Phil Hord]

After a 'git stash apply' which results in a conflicted file,
git mergetool can be used to work on the conflicts.  However,
when rerere.enabled=true, git mergetool determines the list
of unresolved file conflicts by asking 'git rerere remaining'.

There is a problem with this because rerere does not record
the 'git stash apply' conflicts and so 'git rerere remaining'
does not report them.

Demonstrate this failing so it can be studied and addressed.

Also add a very basic test for 'git rerere remaining' after
a normal merge conflict.

Signed-off-by: Phil Hord <hordp@cisco.com>
---

This patch adds one failing test and one working test.

The working test is very limited and just repeats the
same test as the 'rerere status' one for 'rerere remaining'.
A better test should probably be added, one that demonstrates
and confirms the differences between 'rerere status' and
'rerere remaining'.  I don't understand this difference
well enough to code that test, though.
 
 t/t4200-rerere.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 36255d6..a96ebad 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -96,10 +96,42 @@ test_expect_success 'rerere.enabled works, too' '
 	grep ^=======$ $rr/preimage
 '
 
+test_expect_success 'set up conflicted stash apply' '
+	rm -rf .git/rr-cache &&
+	git config rerere.enabled true &&
+
+	git reset --hard &&
+	git checkout master &&
+	git show second:a1 >a1 &&
+
+	# should be 1 modified file and no unmerged files
+	test $(git ls-files -m |wc -l) = 1 &&
+	test $(git ls-files -u |wc -l) = 0 &&
+
+	git stash save &&
+	git checkout first &&
+
+	test_must_fail git stash apply &&
+
+	# there should be three states for one unresolved file
+	cnt=$(git ls-files -u | wc -l) &&
+	echo $cnt &&
+	test $cnt = 3
+'
+
+test_expect_failure 'rerere-remaining correctly reports stash-apply conflicts' '
+	echo a1 >expect &&
+	git rerere status >out &&
+	test_cmp expect out &&
+	git rerere remaining >out &&
+	test_cmp expect out
+'
+
 test_expect_success 'set up rr-cache' '
 	rm -rf .git/rr-cache &&
 	git config rerere.enabled true &&
 	git reset --hard &&
+	git checkout second &&
 	test_must_fail git merge first &&
 	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1
@@ -160,6 +192,12 @@ test_expect_success 'rerere status' '
 	test_cmp expect out
 '
 
+test_expect_success 'rerere remaining' '
+	echo a1 >expect &&
+	git rerere remaining >out &&
+	test_cmp expect out
+'
+
 test_expect_success 'first postimage wins' '
 	git show first:a1 | sed "s/To die: t/To die! T/" >expect &&
 
-- 
1.7.6.8.gd2879

Re: rerere affects handling of git-stash-pop merge conflicts

[Martin von Zweigbergk]

On Thu, 14 Jul 2011, Phil Hord wrote:

> Summary:
> After a 'git stash pop' with conflicts, 'git mergetool' fails to notice
> the conflicted files if 'rerere.enabled=true'.  git mergetool
> erroneously reports 'no files need merging'.

It seems to be because git-stash uses git merge-recursive directly
instead of calling git merge. I don't know why git merge-recursive is
used directly. It has been like that ever since git-stash was
introduced in f2c66ed (Add git-stash script, 2007-06-30).

Re: rerere affects handling of git-stash-pop merge conflicts

[Junio C Hamano]

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

> On Thu, 14 Jul 2011, Phil Hord wrote:
>
>> Summary:
>> After a 'git stash pop' with conflicts, 'git mergetool' fails to notice
>> the conflicted files if 'rerere.enabled=true'.  git mergetool
>> erroneously reports 'no files need merging'.
>
> It seems to be because git-stash uses git merge-recursive directly
> instead of calling git merge. I don't know why git merge-recursive is
> used directly. It has been like that ever since git-stash was
> introduced in f2c66ed (Add git-stash script, 2007-06-30).

But "stash pop/apply" should never call "merge" directly, as it is not
interested in creating (nor preparing to create, with "merge --no-commit")
a new merge commit, nor doing the history level three-way merge.

Calling merge-recursive backend is used as a way to run a three-way tree
merge with explicitly specified three trees.

[PATCH] Add two basic tests for "rerere remaining"

[Phil Hord]

After a 'git stash apply' which results in a conflicted file,
git mergetool can be used to work on the conflicts.  However,
when rerere.enabled=true, git mergetool determines the list
of unresolved file conflicts by asking 'git rerere remaining'.

There is a problem with this because rerere does not record
the 'git stash apply' conflicts and so 'git rerere remaining'
does not report them.

Demonstrate this failing so it can be studied and addressed.

Also add a very basic test for 'git rerere remaining' after
a normal merge conflict.

Signed-off-by: Phil Hord <hordp@cisco.com>
---

This patch adds one failing test and one working test.

The working test is very limited and just repeats the
same test as the 'rerere status' one for 'rerere remaining'.
A better test should probably be added, one that demonstrates
and confirms the differences between 'rerere status' and
'rerere remaining'.  I don't understand this difference
well enough to code that test, though.

This patch is the same as before, but dropping the /RFC. 

 t/t4200-rerere.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 36255d6..a96ebad 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -96,10 +96,42 @@ test_expect_success 'rerere.enabled works, too' '
 	grep ^=======$ $rr/preimage
 '
 
+test_expect_success 'set up conflicted stash apply' '
+	rm -rf .git/rr-cache &&
+	git config rerere.enabled true &&
+
+	git reset --hard &&
+	git checkout master &&
+	git show second:a1 >a1 &&
+
+	# should be 1 modified file and no unmerged files
+	test $(git ls-files -m |wc -l) = 1 &&
+	test $(git ls-files -u |wc -l) = 0 &&
+
+	git stash save &&
+	git checkout first &&
+
+	test_must_fail git stash apply &&
+
+	# there should be three states for one unresolved file
+	cnt=$(git ls-files -u | wc -l) &&
+	echo $cnt &&
+	test $cnt = 3
+'
+
+test_expect_failure 'rerere-remaining correctly reports stash-apply conflicts' '
+	echo a1 >expect &&
+	git rerere status >out &&
+	test_cmp expect out &&
+	git rerere remaining >out &&
+	test_cmp expect out
+'
+
 test_expect_success 'set up rr-cache' '
 	rm -rf .git/rr-cache &&
 	git config rerere.enabled true &&
 	git reset --hard &&
+	git checkout second &&
 	test_must_fail git merge first &&
 	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1
@@ -160,6 +192,12 @@ test_expect_success 'rerere status' '
 	test_cmp expect out
 '
 
+test_expect_success 'rerere remaining' '
+	echo a1 >expect &&
+	git rerere remaining >out &&
+	test_cmp expect out
+'
+
 test_expect_success 'first postimage wins' '
 	git show first:a1 | sed "s/To die: t/To die! T/" >expect &&
 
-- 1.7.6.8.gd2879