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

[Siddharth Agarwal]

On 11/22/15 17:17, Pierre-Yves David wrote:
>
>
> 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.

The problem is that the 'f' inside sh isn't getting passed all the 
arguments that sh itself is passed. Since the rest aren't essential to 
the test, feel free to update the test output.