[PATCH 3 of 4 v2] rebase: avoid redundant repo[rev].rev() - just keep working in rev space

[Pierre-Yves David]

On 12/05/2014 07:16 AM, Mads Kiilerich wrote:
> On 12/05/2014 05:39 AM, Matt Harbison wrote:
>>
>> On Mon, 01 Dec 2014 23:13:48 -0500, Mads Kiilerich
>> <mads at kiilerich.com> wrote:
>>
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski at unity3d.com>
>>> # Date 1417493579 -3600
>>> #      Tue Dec 02 05:12:59 2014 +0100
>>> # Node ID 63e92a66482bb5af8b9bdd95dfb0fff500eef582
>>> # Parent  9a8d248290a4e634a6bdc0a1d07514fe8e971ad9
>>> rebase: avoid redundant repo[rev].rev() - just keep working in rev space
>>>
>>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>>> --- a/hgext/rebase.py
>>> +++ b/hgext/rebase.py
>>> @@ -540,20 +540,20 @@ def rebasenode(repo, rev, p1, state, col
>>>      'Rebase a single revision'
>>>      # Merge phase
>>>      # Update to target and merge it with local
>>> -    if repo['.'].rev() != repo[p1].rev():
>>> -        repo.ui.debug(" update to %d:%s\n" % (repo[p1].rev(),
>>> repo[p1]))
>>> +    if repo['.'].rev() != p1:
>>> +        repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1]))
>>
>> This crashed when running evolve 8d28bb4fc127, but the Windows test
>> suite runs clean.  Maybe it needs to be "%s:%s", but I don't have time
>> to chase down if p1 is always a string right now, and didn't want to
>> throw out a patch that guesses.  There are a couple of debug lines
>> below this change too.
>>
>> $ ../hg evolve
>> move:[23950] addremove: restore the relative path printing when files
>> are named
>> atop:[23951] match: introduce uipath() to properly style a file path
>> ** Unknown exception encountered with possibly-broken third-party
>> extension evolve
>> ** which supports versions 3.2 of Mercurial.
>> ** Please disable evolve and try your action again.
>> ** If that fixes the bug please report it to http://bz.selenic.com/
>> ** Python 2.7.2 (default, Jun 12 2011, 15:08:59) [MSC v.1500 32 bit
>> (Intel)]
>> ** Mercurial Distributed SCM (version 3.2.2+44-c237499a7fba+20141205)
>> ** Extensions loaded: eol, convert, graphlog, patchbomb, progress,
>> extdiff, strip, mq, evolve, rebase
>> Traceback (most recent call last):
>>   File "../hg", line 43, in <module>
>>     mercurial.dispatch.run()
>> ...
>>   File "c:\Users\Matt\Projects\hg\mercurial\util.py", line 679, in check
>>     return func(*args, **kwargs)
>>   File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1225,
>> in evolve
>>     progresscb=progresscb)
>>   File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1251,
>> in _evolveany
>>     return _solveunstable(ui, repo, tro, dryrunopt, confirmopt,
>> progresscb)
>>   File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1355,
>> in _solveunstable
>>     relocate(repo, orig, target, keepbranch)
>>   File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 811,
>> in relocate
>>     orig.p1().node())
>>   File "c:\Users\Matt\Projects\hg\hgext\rebase.py", line 547, in
>> rebasenode
>>     repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1]))
>> TypeError: %d format: a number is required, not str
>
> So evolve calls rebasenode directly, passing nodeids where Mercurial
> itself only passes revs. Fortunately it is caught early -
> http://selenic.com/hg/rev/ffef6d503ab2 removed the unused support for
> nodeids.

Yes, I need to migrate it to the new API matt made for 3.2 but I did not 
come to it yet

>
> Calling rebasenode directly hints that evolve duplicates some parts of
> rebase internals - that seems unfortunate. If rebasenode has to stay a
> stable API then it should be documented as such ... but I doubt that is
> how it is.

Feel free to break evolve.


-- 
Pierre-Yves David