[PATCH 1 of 2] convert: temporarily disable hg p2 optimization

[Mads Kiilerich]

On 10/16/2015 11:14 PM, Durham Goode wrote:
>
>
> On 10/16/15 2:11 PM, Durham Goode wrote:
>>
>>
>> On 10/15/15 2:46 PM, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski at unity3d.com>
>>> # Date 1444945384 -7200
>>> #      Thu Oct 15 23:43:04 2015 +0200
>>> # Node ID 8c7010f12530b69c86d2bcdd70098ced582ece49
>>> # Parent  07db7e95c464537aeb2dd7aba39de0813eaffd04
>>> convert: temporarily disable hg p2 optimization
>>>
>>> Disabling the optimization gives a test change. That proves that the 
>>> optimized
>>> code path is wrong. That seems to have been introduced in a75d24539aba.
>>>
>>> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
>>> --- a/hgext/convert/hg.py
>>> +++ b/hgext/convert/hg.py
>>> @@ -221,7 +221,7 @@ class mercurial_sink(converter_sink):
>>>           files = dict(files)
>>>             def getfilectx(repo, memctx, f):
>>> -            if p2ctx and f in p2files and f not in copies:
>>> +            if p2ctx and f in p2files and f not in copies and False:
>>>                   self.ui.debug('reusing %s from p2\n' % f)
>>>                   try:
>>>                       return p2ctx[f]
>>> diff --git a/tests/test-convert-filemap.t 
>>> b/tests/test-convert-filemap.t
>>> --- a/tests/test-convert-filemap.t
>>> +++ b/tests/test-convert-filemap.t
>>> @@ -723,11 +723,11 @@ test converting merges into a repo that
>>>     $ hg -R merge-test2 manifest -r tip
>>>     converted/a
>>>     converted/b
>>> -  x
>>>     $ hg -R merge-test2 log -G -T '{shortest(node)} {desc}\n{files % 
>>> "- {file}\n"}\n'
>>> -  o    6eaa merge a & b
>>> +  o    0390 merge a & b
>>>     |\   - converted/a
>>>     | |  - toberemoved
>>> +  | |  - x
>>>     | |
>>>     | o  2995 add b
>>>     | |  - converted/b
>> I don't understand what you're saying here.
>>
>> In the test, it takes merge-test1 (which adds file 'x' in rev 1), and 
>> it calls convert in such a way that it imports the contents of 
>> merge-test2 on top of the 'x' commit.  That means 'x' should not be 
>> affected by the convert, and that it should appear in the tip 
>> manifest, as it currently does.
>>
>> Your change makes the convert delete 'x', when 'x' isn't related to 
>> the incoming changes in any way.  So am I missing something?
> I got my repo-names reversed.  Here's the same text with the correct 
> repo names:
>
> In the test, it takes merge-test1 (which edits files 'a' and 'b') and 
> merge-test2 (which adds 'x'), and it calls convert in such a way that 
> it imports the contents of merge-test1 on top of the 'x' commit.  That 
> means 'x' should not be affected by the convert, and that it should 
> appear in the tip manifest, as it currently does.
>
> Your change makes the convert delete 'x', when 'x' isn't related to 
> the incoming changes in any way.  So am I missing something?

First, do you agree that the p2clean part just is an optimization? Or 
should we intentionally put different semantics in that?

/Mads