[PATCH STABLE] resolve: restore .orig only after merge is fully complete (issue4952)

[Pierre-Yves David]

On 11/22/2015 11:24 AM, Matt Harbison wrote:
> On Fri, 13 Nov 2015 18:56:44 -0500, Siddharth Agarwal <sid0 at fb.com> wrote:
>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0 at fb.com>
>> # Date 1447458962 28800
>> #      Fri Nov 13 15:56:02 2015 -0800
>> # Branch stable
>> # Node ID 08e5dcae034817491d156cec7934139c469a47eb
>> # Parent  8a256cee72c890c761918ec1d244b286694ac51b
>> resolve: restore .orig only after merge is fully complete (issue4952)
>>
>> Previously, we'd restore the .orig file after the premerge is complete
>> but
>> before the merge was complete. This would lead to the .orig file
>> potentially
>> containing merge conflict markers in it, as a leftover from the last
>> merge
>> attempt.
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -5670,10 +5670,10 @@ def resolve(ui, repo, *pats, **opts):
>>                      ui.setconfig('ui', 'forcemerge', '', 'resolve')
>>                      ms.commit()
>> -                # replace filemerge's .orig file with our resolve file
>> -                # for files in tocomplete, ms.resolve will not overwrite
>> -                # .orig -- only preresolve does
>> -                util.rename(a + ".resolve", a + ".orig")
>> +                # replace filemerge's .orig file with our resolve
>> file, but only
>> +                # for merges that are complete
>> +                if complete:
>> +                    util.rename(a + ".resolve", a + ".orig")
>>         for f in tocomplete:
>>              try:
>> @@ -5687,6 +5687,10 @@ def resolve(ui, repo, *pats, **opts):
>>                  ui.setconfig('ui', 'forcemerge', '', 'resolve')
>>                  ms.commit()
>> +            # replace filemerge's .orig file with our resolve file
>> +            a = repo.wjoin(f)
>> +            util.rename(a + ".resolve", a + ".orig")
>> +
>>          ms.commit()
>>         if not didwork and pats:
>> diff --git a/tests/test-resolve.t b/tests/test-resolve.t
>> --- a/tests/test-resolve.t
>> +++ b/tests/test-resolve.t
>> @@ -207,6 +207,30 @@ resolve <file> should re-merge file
>>    [1]
>>    $ grep '<<<' file1 > /dev/null
>> +test .orig behavior with resolve
>> +
>> +  $ echo resolve > file
>> +  $ hg resolve -q file1 --tool 'f --dump $TESTTMP/repo/file1.orig'
>> +  */file1~base*: (glob)
>> +  >>>
>> +  foo
>> +  <<<
>> +  */file1~other*: (glob)
>> +  >>>
>> +  foo
>> +  bar
>> +  <<<
>> +  $TESTTMP/repo/file1: (glob)
>> +  >>>
>> +  foo
>> +  baz
>> +  <<<
>> +  $TESTTMP/repo/file1.orig: (glob)
>> +  >>>
>> +  foo
>> +  baz
>> +  <<<
>> +
>>  resolve <file> should do nothing if 'file' was marked resolved
>>    $ echo resolved > file1
>>    $ hg resolve -m file1
>
> I had to adjust the command line for Windows (it can't execute f directly):
>
>    $ hg resolve -q file1 --tool "sh -c 'f --dump
> \"$TESTTMP/repo/file1.orig\"'"
>
> But then I get the following diff:
>
> --- c:/Users/Matt/Projects/hg/tests/test-resolve.t
> +++ c:/Users/Matt/Projects/hg/tests/test-resolve.t.err
> @@ -211,25 +211,11 @@
>
>     $ echo resolve > file
>     $ hg resolve -q file1 --tool "sh -c 'f --dump
> \"$TESTTMP/repo/file1.orig\"'"
> -  */file1~base*: (glob)
> +  $TESTTMP/repo/file1.orig:\r (esc)
>     >>>
>     foo
> -  <<<
> -  */file1~other*: (glob)
> -  >>>
> -  foo
> -  bar
> -  <<<
> -  $TESTTMP/repo/file1: (glob)
> -  >>>
> -  foo
> -  baz
> -  <<<
> -  $TESTTMP/repo/file1.orig: (glob)
> -  >>>
> -  foo
> -  baz
> -  <<<
> +  baz\r (esc)
> +  <<<\r (esc)
>
>   resolve <file> should do nothing if 'file' was marked resolved
>     $ echo resolved > file1
>
> Basically, file1~base, file1~other and file1 are missing from the file.
> I don't see any conditional tests to explain the diff, and I'm not
> familiar with the resolve code.  Any thoughts on what this could be?

Please create a bug so that we can track this issue.

-- 
Pierre-Yves David