[PATCH v3] shelve: make unshelve be able to abort in any case

[Kostia Balytskyi]

On 7/13/16, 3:02 PM, "Mercurial-devel on behalf of Yuya Nishihara" <mercurial-devel-bounces at mercurial-scm.org on behalf of yuya at tcha.org> wrote:

>On Tue, 12 Jul 2016 17:04:34 +0000, Kostia Balytskyi wrote:
>> # HG changeset patch
>> # User Kostia Balytskyi <ikostia at fb.com>
>> # Date 1468340243 -3600
>> #      Tue Jul 12 17:17:23 2016 +0100
>> # Node ID 5dbbeae9e7657cec9775db60977570e7efb5ed18
>> # Parent  2550604f5ec736d4e603b04f6fe746468c0efd3b
>> shelve: make unshelve be able to abort in any case
>> 
>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -170,16 +170,21 @@ class shelvedstate(object):
>>              parents = [nodemod.bin(h) for h in fp.readline().split()]
>>              stripnodes = [nodemod.bin(h) for h in fp.readline().split()]
>>              branchtorestore = fp.readline().strip()
>> +        except (IOError, ValueError, TypeError) as err:
>> +            raise error.CorruptedState(err.message)
>
>IOError doesn't mean the file is corrupted.
Yes, but the exception means that “state is corrupted” which in my mind includes “can’t read file” thing and so on. That is why the message below is “could not read the state” instead of “state is corrupted”.
>
>> -        obj = cls()
>> -        obj.name = name
>> -        obj.wctx = repo[nodemod.bin(wctx)]
>> -        obj.pendingctx = repo[nodemod.bin(pendingctx)]
>> -        obj.parents = parents
>> -        obj.stripnodes = stripnodes
>> -        obj.branchtorestore = branchtorestore
>> +        try:
>> +            obj = cls()
>> +            obj.name = name
>> +            obj.wctx = repo[nodemod.bin(wctx)]
>> +            obj.pendingctx = repo[nodemod.bin(pendingctx)]
>> +            obj.parents = parents
>> +            obj.stripnodes = stripnodes
>> +            obj.branchtorestore = branchtorestore
>> +        except (TypeError, error.RepoLookupError) as err:
>> +            raise error.CorruptedState(err.message)
>
>My two cents, we can move nodemod.bin() to the first block so that this block
>will never catch TypeError that could be raised deeply from repo.__getitem__().
>
>>          except IOError as err:
>> -            if err.errno != errno.ENOENT:
>> +            if err.errno == errno.ENOENT:
>> +                cmdutil.wrongtooltocontinue(repo, _('unshelve'))
>> +            else:
>>                  raise
>> -            cmdutil.wrongtooltocontinue(repo, _('unshelve'))
>
>Unneeded change?
Why is it unneeded?
>
>> +        except error.CorruptedState as err:
>> +            ui.debug(err.message + '\n')
>
>BaseException.message is deprecated, Python will emit DeprecationWarning.
Will fix.
>
>> +            if continuef:
>> +                msg = _('could not read shelved state file')
>
>It sounds like a trivial error. I think "shelved state file is corrupted" is
>clearer.
>
>> +                hint = _('please run hg unshelve --abort to abort unshelve '
>> +                         'operation')
>> +                raise error.Abort(msg, hint=hint)
>> +            elif abortf:
>> +                msg = _('could not read shelved state file, your working copy '
>> +                        'may be in an unexpected state\nplease update to some '
>> +                        'commit\n')
>> +                ui.warn(msg)
>> +                shelvedstate.clear(repo)
>> +            return
>>  
>>          if abortf:
>>              return unshelveabort(ui, repo, state, opts)
>> diff --git a/mercurial/error.py b/mercurial/error.py
>> --- a/mercurial/error.py
>> +++ b/mercurial/error.py
>> @@ -235,3 +235,6 @@ class InvalidBundleSpecification(Excepti
>>  
>>  class UnsupportedBundleSpecification(Exception):
>>      """error raised when a bundle specification is not supported."""
>> +
>> +class CorruptedState(HintException):
>> +    """error raised when a command is not able to read its state from file"""
>
>If this is an exception locally used in shelve.py, move it to shelve.py.

It can be reused for other stateful commands.
>_______________________________________________
>Mercurial-devel mailing list
>Mercurial-devel at mercurial-scm.org
>https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel