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

login
register
mail settings
Submitter Kostia Balytskyi
Date July 12, 2016, 5:04 p.m.
Message ID <AM3PR06MB147547256D08220114DEED2D3300@AM3PR06MB147.eurprd06.prod.outlook.com>
Download mbox | patch
Permalink /patch/15802/
State Changes Requested
Headers show

Comments

Kostia Balytskyi - July 12, 2016, 5:04 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@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
Kostia Balytskyi - July 12, 2016, 5:13 p.m.
If this one looks ok to people, I will send a v4 with CorruptedState(Hint, Exception) instead of CorruptedState(HintException).

On 7/12/16, 6:04 PM, "Mercurial-devel on behalf of Kostia Balytskyi" <mercurial-devel-bounces@mercurial-scm.org on behalf of ikostia@fb.com> wrote:

># HG changeset patch

># User Kostia Balytskyi <ikostia@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)

>         finally:

>             fp.close()

> 

>-        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)

> 

>         return obj

> 

>@@ -663,9 +668,24 @@ def _dounshelve(ui, repo, *shelved, **op

>         try:

>             state = shelvedstate.load(repo)

>         except IOError as err:

>-            if err.errno != errno.ENOENT:

>+            if err.errno == errno.ENOENT:

>+                cmdutil.wrongtooltocontinue(repo, _('unshelve'))

>+            else:

>                 raise

>-            cmdutil.wrongtooltocontinue(repo, _('unshelve'))

>+        except error.CorruptedState as err:

>+            ui.debug(err.message + '\n')

>+            if continuef:

>+                msg = _('could not read shelved state file')

>+                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"""

>diff --git a/tests/test-shelve.t b/tests/test-shelve.t

>--- a/tests/test-shelve.t

>+++ b/tests/test-shelve.t

>@@ -1585,3 +1585,40 @@ On non bare shelve the branch informatio

>   ? b

>   $ hg branch

>   default

>+  $ cd ..

>+

>+Prepare unshleve with a corrupted shelvedstate

>+  $ hg init r1 && cd r1

>+  $ echo text1 > file && hg add file

>+  $ hg shelve

>+  shelved as default

>+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved

>+  $ echo text2 > file && hg ci -Am text1

>+  adding file

>+  $ hg unshelve

>+  unshelving change 'default'

>+  rebasing shelved changes

>+  rebasing 1:396ea74229f9 "(changes in empty repository)" (tip)

>+  merging file

>+  warning: conflicts while merging file! (edit, then use 'hg resolve --mark')

>+  unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')

>+  [1]

>+  $ echo somethingsomething > .hg/shelvedstate

>+

>+Unshelve --continue fails with appropriate message if shelvedstate is corrupted

>+  $ hg unshelve --continue

>+  abort: could not read shelved state file

>+  (please run hg unshelve --abort to abort unshelve operation)

>+  [255]

>+

>+Unshelve --abort works with a corrupted shelvedstate

>+  $ hg unshelve --abort

>+  could not read shelved state file, your working copy may be in an unexpected state

>+  please update to some commit

>+

>+Unshelve --abort fails with appropriate message if there's no unshelve in

>+progress

>+  $ hg unshelve --abort

>+  abort: no unshelve in progress

>+  [255]

>+  $ cd ..

>_______________________________________________

>Mercurial-devel mailing list

>Mercurial-devel@mercurial-scm.org

>https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=WD6ZmwZ3Xb86_eIJu9dqh-ZZ7fXxIEft3Ny3wQjajkA&s=4ElAiQ2Cw-_O2llxeEeibtlcHcG8R8wXXqA6nTPxaN8&e=
Yuya Nishihara - July 13, 2016, 2:02 p.m.
On Tue, 12 Jul 2016 17:04:34 +0000, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@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.

> -        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?

> +        except error.CorruptedState as err:
> +            ui.debug(err.message + '\n')

BaseException.message is deprecated, Python will emit DeprecationWarning.

> +            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.
Kostia Balytskyi - July 13, 2016, 2:36 p.m.
On 7/13/16, 3:02 PM, "Mercurial-devel on behalf of Yuya Nishihara" <mercurial-devel-bounces@mercurial-scm.org on behalf of yuya@tcha.org> wrote:

>On Tue, 12 Jul 2016 17:04:34 +0000, Kostia Balytskyi wrote:

>> # HG changeset patch

>> # User Kostia Balytskyi <ikostia@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@mercurial-scm.org

>https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

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)
         finally:
             fp.close()
 
-        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)
 
         return obj
 
@@ -663,9 +668,24 @@  def _dounshelve(ui, repo, *shelved, **op
         try:
             state = shelvedstate.load(repo)
         except IOError as err:
-            if err.errno != errno.ENOENT:
+            if err.errno == errno.ENOENT:
+                cmdutil.wrongtooltocontinue(repo, _('unshelve'))
+            else:
                 raise
-            cmdutil.wrongtooltocontinue(repo, _('unshelve'))
+        except error.CorruptedState as err:
+            ui.debug(err.message + '\n')
+            if continuef:
+                msg = _('could not read shelved state file')
+                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"""
diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -1585,3 +1585,40 @@  On non bare shelve the branch informatio
   ? b
   $ hg branch
   default
+  $ cd ..
+
+Prepare unshleve with a corrupted shelvedstate
+  $ hg init r1 && cd r1
+  $ echo text1 > file && hg add file
+  $ hg shelve
+  shelved as default
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo text2 > file && hg ci -Am text1
+  adding file
+  $ hg unshelve
+  unshelving change 'default'
+  rebasing shelved changes
+  rebasing 1:396ea74229f9 "(changes in empty repository)" (tip)
+  merging file
+  warning: conflicts while merging file! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
+  [1]
+  $ echo somethingsomething > .hg/shelvedstate
+
+Unshelve --continue fails with appropriate message if shelvedstate is corrupted
+  $ hg unshelve --continue
+  abort: could not read shelved state file
+  (please run hg unshelve --abort to abort unshelve operation)
+  [255]
+
+Unshelve --abort works with a corrupted shelvedstate
+  $ hg unshelve --abort
+  could not read shelved state file, your working copy may be in an unexpected state
+  please update to some commit
+
+Unshelve --abort fails with appropriate message if there's no unshelve in
+progress
+  $ hg unshelve --abort
+  abort: no unshelve in progress
+  [255]
+  $ cd ..