[PATCH 3 of 3 V2] rebase: handle revtodo as a special value when storing/restoring state

[Mads Kiilerich]

On 12/05/2014 01:44 AM, Pierre-Yves David wrote:
>
>
> On 12/04/2014 08:06 AM, Mads Kiilerich wrote:
>> On 12/04/2014 04:29 PM, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>>> # Date 1417551792 28800
>>> #      Tue Dec 02 12:23:12 2014 -0800
>>> # Node ID e3b289dd518b326d927f1ec6b61f025a6c74c12a
>>> # Parent  f1632814e8d3733acec372dcc17620ab0a8617bf
>>> rebase: handle revtodo as a special value when storing/restoring state
>>>
>>> Revtodo happens to share its value with nullrev but this is an
>>> implementation
>>> details. So we move away from it.
>>>
>>> After this changeset one can successfully change the values for all
>>> the constant
>>> and have the test passing. But doing so would require more refactoring
>>> if we
>>> want to avoid breaking backward compatibility on the state file.
>>>
>>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>>> --- a/hgext/rebase.py
>>> +++ b/hgext/rebase.py
>>> @@ -16,11 +16,11 @@ http://mercurial.selenic.com/wiki/Rebase
>>>   from mercurial import hg, util, repair, merge, cmdutil, commands,
>>> bookmarks
>>>   from mercurial import extensions, patch, scmutil, phases, obsolete,
>>> error
>>>   from mercurial import copies
>>>   from mercurial.commands import templateopts
>>> -from mercurial.node import nullrev
>>> +from mercurial.node import nullrev, nullid, hex
>>>   from mercurial.lock import release
>>>   from mercurial.i18n import _
>>>   import os, errno
>>>   revtodo = -1
>>> @@ -728,12 +728,16 @@ def storestatus(repo, originalwd, target
>>>       f.write('%d\n' % int(keep))
>>>       f.write('%d\n' % int(keepbranches))
>>>       f.write('%s\n' % (activebookmark or ''))
>>>       for d, v in state.iteritems():
>>>           oldrev = repo[d].hex()
>>> -        if v > nullmerge:
>>> +        if v >= 0:
>>>               newrev = repo[v].hex()
>>> +        elif v == revtodo:
>>> +            # To maintain format compatibility, we have to use nullid.
>>> +            # Please do remove this special case when upgrading the
>>> format.
>>> +            newrev = hex(nullid)
>>>           else:
>>>               newrev = v
>>>           f.write("%s:%s\n" % (oldrev, newrev))
>>>       f.close()
>>>       repo.ui.debug('rebase status stored\n')
>>> @@ -771,10 +775,13 @@ def restorestatus(repo):
>>>                   activebookmark = l
>>>               else:
>>>                   oldrev, newrev = l.split(':')
>>>                   if newrev in (str(nullmerge), str(revignored)):
>>>                       state[repo[oldrev].rev()] = int(newrev)
>>> +                elif newrev == nullid:
>>
>> I guess that should be newrev == hex(nullid)?
>
> Indeed. I think I need a "no uncommitted change before patchbomb" 
> check :-/.

We kind of have that - http://selenic.com/repo/hg/rev/dbff8c119cf6 .

> (Note that is keeps working because revtodo is still -1, so the 
> 'repo[hex(nullid)].rev()' call eventually return -1)

I agree. It is only relevant to get the full decoupling from nullrev 
that you wanted.

/Mads