Patchwork [3,of,3] histedit: delete histedit statefile on any exception during abort

login
register
mail settings
Submitter Christian Delahousse
Date Oct. 8, 2015, 10 p.m.
Message ID <1035b07658b5d21c80ad.1444341606@dev4253.prn1.facebook.com>
Download mbox | patch
Permalink /patch/10899/
State Superseded
Commit e28102403d1b886237bf1340bbed4f826bcd6882
Headers show

Comments

Christian Delahousse - Oct. 8, 2015, 10 p.m.
# HG changeset patch
# User Christian Delahousse <cdelahousse@fb.com>
# Date 1444088685 25200
#      Mon Oct 05 16:44:45 2015 -0700
# Node ID 1035b07658b5d21c80ad1d26c88fe6c63ba81b19
# Parent  d7088586b95e2efd96f9e9d3a71a4346a211306d
histedit: delete histedit statefile on any exception during abort

When an user aborts a histedit, many things could go wrong. At a minimum, after
a histedit abort failure, their repository should be out of that state. We've
found situations where the user could not exit the histedit state without
manually deleting the histedit state file. This patch ensures that if any
exception happens during an abort, the histedit statefile will be deleted so
that users are out of the histedit state and can at least manually get the repo
back to a workable condition.
Augie Fackler - Oct. 9, 2015, 3:28 p.m.
On Thu, Oct 08, 2015 at 03:00:06PM -0700, Christian Delahousse wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1444088685 25200
> #      Mon Oct 05 16:44:45 2015 -0700
> # Node ID 1035b07658b5d21c80ad1d26c88fe6c63ba81b19
> # Parent  d7088586b95e2efd96f9e9d3a71a4346a211306d
> histedit: delete histedit statefile on any exception during abort

Queued this.

> When an user aborts a histedit, many things could go wrong. At a minimum, after
> a histedit abort failure, their repository should be out of that state. We've
> found situations where the user could not exit the histedit state without
> manually deleting the histedit state file. This patch ensures that if any
> exception happens during an abort, the histedit statefile will be deleted so
> that users are out of the histedit state and can at least manually get the repo
> back to a workable condition.

I guess, though in principle I'd like to see the bugs in this area get
fixed (manual corruption of the state file doesn't really count.)

>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -812,26 +812,35 @@
>          state.write()
>          return
>      elif goal == 'abort':
> -        state.read()
> -        tmpnodes, leafs = newnodestoabort(state)
> -        ui.debug('restore wc to old parent %s\n' % node.short(state.topmost))
> +        try:
> +            state.read()
> +            tmpnodes, leafs = newnodestoabort(state)
> +            ui.debug('restore wc to old parent %s\n'
> +                    % node.short(state.topmost))
>
> -        # Recover our old commits if necessary
> -        if not state.topmost in repo and state.backupfile:
> -            backupfile = repo.join(state.backupfile)
> -            f = hg.openpath(ui, backupfile)
> -            gen = exchange.readbundle(ui, f, backupfile)
> -            changegroup.addchangegroup(repo, gen, 'histedit',
> -                                       'bundle:' + backupfile)
> -            os.remove(backupfile)
> +            # Recover our old commits if necessary
> +            if not state.topmost in repo and state.backupfile:
> +                backupfile = repo.join(state.backupfile)
> +                f = hg.openpath(ui, backupfile)
> +                gen = exchange.readbundle(ui, f, backupfile)
> +                changegroup.addchangegroup(repo, gen, 'histedit',
> +                                        'bundle:' + backupfile)
> +                os.remove(backupfile)
>
> -        # check whether we should update away
> -        if repo.unfiltered().revs('parents() and (%n  or %ln::)',
> -                                  state.parentctxnode, leafs | tmpnodes):
> -            hg.clean(repo, state.topmost)
> -        cleanupnode(ui, repo, 'created', tmpnodes)
> -        cleanupnode(ui, repo, 'temp', leafs)
> -        state.clear()
> +            # check whether we should update away
> +            if repo.unfiltered().revs('parents() and (%n  or %ln::)',
> +                                    state.parentctxnode, leafs | tmpnodes):
> +                hg.clean(repo, state.topmost)
> +            cleanupnode(ui, repo, 'created', tmpnodes)
> +            cleanupnode(ui, repo, 'temp', leafs)
> +        except Exception:
> +            if state.inprogress():
> +                ui.warn(_('warning: encountered an exception during histedit '
> +                    '--abort; the repository may not have been completely '
> +                    'cleaned up\n'))
> +            raise
> +        finally:
> +                state.clear()
>          return
>      else:
>          cmdutil.checkunfinished(repo)
> diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
> --- a/tests/test-histedit-arguments.t
> +++ b/tests/test-histedit-arguments.t
> @@ -324,3 +324,24 @@
>    |
>    o  0:6058cbb6cfd7 one
>
> +
> +Test that abort fails gracefully on exception
> +----------------------------------------------
> +  $ hg histedit . -q --commands - << EOF
> +  > edit 8fda0c726bf2 6 x
> +  > EOF
> +  Make changes as needed, you may commit or record as needed now.
> +  When you are finished, run hg histedit --continue to resume.
> +  [1]
> +Corrupt histedit state file
> +  $ sed -i 's/8fda0c726bf2/123456789012/' .hg/histedit-state
> +  $ hg histedit --abort
> +  warning: encountered an exception during histedit --abort; the repository may not have been completely cleaned up
> +  abort: No such file or directory: * (glob)
> +  [255]
> +Histedit state has been exited
> +  $ hg summary -q
> +  parent: 5:63379946892c
> +  commit: 1 added, 1 unknown (new branch head)
> +  update: 4 new changesets (update)
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Oct. 9, 2015, 5:32 p.m.
On Thu, Oct 08, 2015 at 03:00:06PM -0700, Christian Delahousse wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1444088685 25200
> #      Mon Oct 05 16:44:45 2015 -0700
> # Node ID 1035b07658b5d21c80ad1d26c88fe6c63ba81b19
> # Parent  d7088586b95e2efd96f9e9d3a71a4346a211306d
> histedit: delete histedit statefile on any exception during abort

check-code sends festive decorative gourd season greetings

>
> When an user aborts a histedit, many things could go wrong. At a minimum, after
> a histedit abort failure, their repository should be out of that state. We've
> found situations where the user could not exit the histedit state without
> manually deleting the histedit state file. This patch ensures that if any
> exception happens during an abort, the histedit statefile will be deleted so
> that users are out of the histedit state and can at least manually get the repo
> back to a workable condition.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -812,26 +812,35 @@
>          state.write()
>          return
>      elif goal == 'abort':
> -        state.read()
> -        tmpnodes, leafs = newnodestoabort(state)
> -        ui.debug('restore wc to old parent %s\n' % node.short(state.topmost))
> +        try:
> +            state.read()
> +            tmpnodes, leafs = newnodestoabort(state)
> +            ui.debug('restore wc to old parent %s\n'
> +                    % node.short(state.topmost))
>
> -        # Recover our old commits if necessary
> -        if not state.topmost in repo and state.backupfile:
> -            backupfile = repo.join(state.backupfile)
> -            f = hg.openpath(ui, backupfile)
> -            gen = exchange.readbundle(ui, f, backupfile)
> -            changegroup.addchangegroup(repo, gen, 'histedit',
> -                                       'bundle:' + backupfile)
> -            os.remove(backupfile)
> +            # Recover our old commits if necessary
> +            if not state.topmost in repo and state.backupfile:
> +                backupfile = repo.join(state.backupfile)
> +                f = hg.openpath(ui, backupfile)
> +                gen = exchange.readbundle(ui, f, backupfile)
> +                changegroup.addchangegroup(repo, gen, 'histedit',
> +                                        'bundle:' + backupfile)
> +                os.remove(backupfile)
>
> -        # check whether we should update away
> -        if repo.unfiltered().revs('parents() and (%n  or %ln::)',
> -                                  state.parentctxnode, leafs | tmpnodes):
> -            hg.clean(repo, state.topmost)
> -        cleanupnode(ui, repo, 'created', tmpnodes)
> -        cleanupnode(ui, repo, 'temp', leafs)
> -        state.clear()
> +            # check whether we should update away
> +            if repo.unfiltered().revs('parents() and (%n  or %ln::)',
> +                                    state.parentctxnode, leafs | tmpnodes):
> +                hg.clean(repo, state.topmost)
> +            cleanupnode(ui, repo, 'created', tmpnodes)
> +            cleanupnode(ui, repo, 'temp', leafs)
> +        except Exception:
> +            if state.inprogress():
> +                ui.warn(_('warning: encountered an exception during histedit '
> +                    '--abort; the repository may not have been completely '
> +                    'cleaned up\n'))
> +            raise
> +        finally:
> +                state.clear()
>          return
>      else:
>          cmdutil.checkunfinished(repo)
> diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
> --- a/tests/test-histedit-arguments.t
> +++ b/tests/test-histedit-arguments.t
> @@ -324,3 +324,24 @@
>    |
>    o  0:6058cbb6cfd7 one
>
> +
> +Test that abort fails gracefully on exception
> +----------------------------------------------
> +  $ hg histedit . -q --commands - << EOF
> +  > edit 8fda0c726bf2 6 x
> +  > EOF
> +  Make changes as needed, you may commit or record as needed now.
> +  When you are finished, run hg histedit --continue to resume.
> +  [1]
> +Corrupt histedit state file
> +  $ sed -i 's/8fda0c726bf2/123456789012/' .hg/histedit-state
> +  $ hg histedit --abort
> +  warning: encountered an exception during histedit --abort; the repository may not have been completely cleaned up
> +  abort: No such file or directory: * (glob)
> +  [255]
> +Histedit state has been exited
> +  $ hg summary -q
> +  parent: 5:63379946892c
> +  commit: 1 added, 1 unknown (new branch head)
> +  update: 4 new changesets (update)
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -812,26 +812,35 @@ 
         state.write()
         return
     elif goal == 'abort':
-        state.read()
-        tmpnodes, leafs = newnodestoabort(state)
-        ui.debug('restore wc to old parent %s\n' % node.short(state.topmost))
+        try:
+            state.read()
+            tmpnodes, leafs = newnodestoabort(state)
+            ui.debug('restore wc to old parent %s\n'
+                    % node.short(state.topmost))
 
-        # Recover our old commits if necessary
-        if not state.topmost in repo and state.backupfile:
-            backupfile = repo.join(state.backupfile)
-            f = hg.openpath(ui, backupfile)
-            gen = exchange.readbundle(ui, f, backupfile)
-            changegroup.addchangegroup(repo, gen, 'histedit',
-                                       'bundle:' + backupfile)
-            os.remove(backupfile)
+            # Recover our old commits if necessary
+            if not state.topmost in repo and state.backupfile:
+                backupfile = repo.join(state.backupfile)
+                f = hg.openpath(ui, backupfile)
+                gen = exchange.readbundle(ui, f, backupfile)
+                changegroup.addchangegroup(repo, gen, 'histedit',
+                                        'bundle:' + backupfile)
+                os.remove(backupfile)
 
-        # check whether we should update away
-        if repo.unfiltered().revs('parents() and (%n  or %ln::)',
-                                  state.parentctxnode, leafs | tmpnodes):
-            hg.clean(repo, state.topmost)
-        cleanupnode(ui, repo, 'created', tmpnodes)
-        cleanupnode(ui, repo, 'temp', leafs)
-        state.clear()
+            # check whether we should update away
+            if repo.unfiltered().revs('parents() and (%n  or %ln::)',
+                                    state.parentctxnode, leafs | tmpnodes):
+                hg.clean(repo, state.topmost)
+            cleanupnode(ui, repo, 'created', tmpnodes)
+            cleanupnode(ui, repo, 'temp', leafs)
+        except Exception:
+            if state.inprogress():
+                ui.warn(_('warning: encountered an exception during histedit '
+                    '--abort; the repository may not have been completely '
+                    'cleaned up\n'))
+            raise
+        finally:
+                state.clear()
         return
     else:
         cmdutil.checkunfinished(repo)
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -324,3 +324,24 @@ 
   |
   o  0:6058cbb6cfd7 one
   
+
+Test that abort fails gracefully on exception
+----------------------------------------------
+  $ hg histedit . -q --commands - << EOF
+  > edit 8fda0c726bf2 6 x
+  > EOF
+  Make changes as needed, you may commit or record as needed now.
+  When you are finished, run hg histedit --continue to resume.
+  [1]
+Corrupt histedit state file
+  $ sed -i 's/8fda0c726bf2/123456789012/' .hg/histedit-state
+  $ hg histedit --abort
+  warning: encountered an exception during histedit --abort; the repository may not have been completely cleaned up
+  abort: No such file or directory: * (glob)
+  [255]
+Histedit state has been exited
+  $ hg summary -q
+  parent: 5:63379946892c 
+  commit: 1 added, 1 unknown (new branch head)
+  update: 4 new changesets (update)
+