cmdstate: introduce a "saver" contextmanager and use it in place of save()
authorKyle Lippincott <spectral@google.com>
Mon, 16 Sep 2019 12:44:38 -0700
changeset 4848 535ab2609e45
parent 4847 0fecff9ac36d
child 4851 e804d5a7c193
cmdstate: introduce a "saver" contextmanager and use it in place of save() Previously, the state was only saved in some paths out of these functions. This can be problematic, if the user ctrl-c's (or `kill -9`'s) the process, or we exit out of `relocate` for anything besides the "expected" reason, we won't record that we were in the middle of an evolve. One of our users has discovered that this leaves hg in a weird state; the user did something like this: ``` $ hg evolve <something goes wrong with the merge tool, hits ctrl-c> <deals with the merge conflicts> $ hg evolve --continue abort: no interrupted evolve to continue $ hg evolve abort: uncommitted changes # Note: commands.status.verbose=True is set. $ hg status M foo # The repository is in an unfinished *update* state. # No unresolved merge conflicts # To continue: hg update ``` The user did an `hg update`, but it didn't actually do anything besides take it out of the unfinished update state (the files were still dirty in the working directory).
hgext3rd/evolve/evolvecmd.py
hgext3rd/evolve/state.py
tests/test-evolve-interrupted.t
--- a/hgext3rd/evolve/evolvecmd.py	Mon Sep 16 12:42:50 2019 -0700
+++ b/hgext3rd/evolve/evolvecmd.py	Mon Sep 16 12:44:38 2019 -0700
@@ -178,15 +178,10 @@
         repo.ui.note(todo)
         if progresscb:
             progresscb()
-        try:
+        with state.saver(evolvestate, {b'current': orig.node()}):
             newid = relocate(repo, orig, target, evolvestate, pctx,
                              keepbranch, b'orphan')
             return (True, newid)
-        except error.InterventionRequired:
-            ops = {b'current': orig.node()}
-            evolvestate.addopts(ops)
-            evolvestate.save()
-            raise
 
 def _solvephasedivergence(ui, repo, bumped, evolvestate, displayer,
                           dryrun=False, confirm=False, progresscb=None):
@@ -241,7 +236,8 @@
         # Need to rebase the changeset at the right place
         repo.ui.status(
             _(b'rebasing to destination parent: %s\n') % prec.p1())
-        try:
+        with state.saver(evolvestate, {b'current': bumped.hex(),
+                                       b'precursor': prec.hex()}):
             newnode = relocate(repo, bumped, prec.p1(), evolvestate,
                                category=b'phasedivergent')
             if newnode is not None:
@@ -250,11 +246,6 @@
                                        operation=b'evolve')
                 bumped = new
                 evolvestate[b'temprevs'].append(newnode)
-        except error.InterventionRequired:
-            evolvestate[b'current'] = bumped.hex()
-            evolvestate[b'precursor'] = prec.hex()
-            evolvestate.save()
-            raise
 
     return _resolvephasedivergent(ui, repo, prec, bumped)
 
@@ -511,13 +502,9 @@
         evolvestate[b'relocating'] = True
         ui.status(_(b'rebasing "other" content-divergent changeset %s on'
                     b' %s\n' % (other, divergent.p1())))
-        try:
+        with state.saver(evolvestate, {b'current': other.node()}):
             newother = relocate(repo, other, divergent.p1(), evolvestate,
                                 keepbranch=True)
-        except error.InterventionRequired:
-            evolvestate[b'current'] = other.node()
-            evolvestate.save()
-            raise
         evolvestate[b'old-other'] = other.node()
         other = repo[newother]
         evolvestate[b'relocating'] = False
@@ -567,21 +554,22 @@
                  (TROUBLES['CONTENTDIVERGENT'], other.hex()[:12]))
     if progresscb:
         progresscb()
-    mergeancestor = repo.changelog.isancestor(divergent.node(), other.node())
-    stats = merge.update(repo,
-                         other.node(),
-                         branchmerge=True,
-                         force=False,
-                         ancestor=base.node(),
-                         mergeancestor=mergeancestor)
-    hg._showstats(repo, stats)
+    with state.saver(evolvestate):
+        mergeancestor = repo.changelog.isancestor(divergent.node(),
+                                                  other.node())
+        stats = merge.update(repo,
+                             other.node(),
+                             branchmerge=True,
+                             force=False,
+                             ancestor=base.node(),
+                             mergeancestor=mergeancestor)
+        hg._showstats(repo, stats)
 
-    # conflicts while merging content-divergent changesets
-    if compat.hasconflict(stats):
-        evolvestate.save()
-        hint = _(b"see 'hg help evolve.interrupted'")
-        raise error.InterventionRequired(_(b"unresolved merge conflicts"),
-                                         hint=hint)
+        # conflicts while merging content-divergent changesets
+        if compat.hasconflict(stats):
+            hint = _(b"see 'hg help evolve.interrupted'")
+            raise error.InterventionRequired(_(b"unresolved merge conflicts"),
+                                             hint=hint)
 
 def _completecontentdivergent(ui, repo, progresscb, divergent, other,
                               base, evolvestate):
--- a/hgext3rd/evolve/state.py	Mon Sep 16 12:42:50 2019 -0700
+++ b/hgext3rd/evolve/state.py	Mon Sep 16 12:44:38 2019 -0700
@@ -15,6 +15,7 @@
 
 from __future__ import absolute_import
 
+import contextlib
 import errno
 import struct
 
@@ -143,3 +144,16 @@
         return state
     finally:
         f.close()
+
+@contextlib.contextmanager
+def saver(state, opts=None):
+    """ensure the state is saved on disk during the duration of the context
+
+    The state is preserved if the context is exited through an exception.
+    """
+    if opts:
+        state.addopts(opts)
+    state.save()
+    yield
+    # delete only if no exception where raised
+    state.delete()
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-evolve-interrupted.t	Mon Sep 16 12:44:38 2019 -0700
@@ -0,0 +1,149 @@
+Quitting an evolve in the middle (via ctrl-c or something) can leave things in a
+weird intermediate state where hg thinks we're in the middle of an update
+operation (or even just leave the 'merge' directory around without actually
+indicating we're in the middle of *any* operation).
+
+  $ . $TESTDIR/testlib/common.sh
+
+  $ cat << EOF >> $HGRCPATH
+  > [extensions]
+  > rebase =
+  > evolve =
+  > [alias]
+  > l = log -G -T'{rev} {desc}'
+  > EOF
+
+  $ hg init interrupted-orphan
+  $ cd interrupted-orphan
+
+  $ echo apricot > a
+  $ hg ci -qAm apricot
+
+  $ echo banana > b
+  $ hg ci -qAm banana
+
+Let's go back to amend 0 and make an orphan out of 1 (and a merge conflict to
+test with)
+
+  $ hg up -q 0
+  $ echo blueberry > b
+  $ hg l
+  o  1 banana
+  |
+  @  0 apricot
+  
+  $ hg ci --amend -qAm 'apricot and blueberry'
+  1 new orphan changesets
+  $ hg l
+  @  2 apricot and blueberry
+  
+  *  1 banana
+  |
+  x  0 apricot
+  
+
+  $ hg evolve --update --config hooks.precommit=false --config ui.merge=:other
+  move:[1] banana
+  atop:[2] apricot and blueberry
+  transaction abort!
+  rollback completed
+  abort: precommit hook exited with status 1
+  [255]
+  $ hg l
+  @  2 apricot and blueberry
+  
+  *  1 banana
+  |
+  x  0 apricot
+  
+  $ cat b
+  banana
+
+  $ hg status --config commands.status.verbose=True
+  M b
+  # The repository is in an unfinished *evolve* state.
+  
+  # No unresolved merge conflicts.
+  
+  # To continue:    hg evolve --continue
+  # To abort:       hg evolve --abort
+  # To stop:        hg evolve --stop
+  # (also see `hg help evolve.interrupted`)
+  
+
+  $ ls .hg/evolvestate
+  .hg/evolvestate
+
+  $ cat b
+  banana
+
+  $ hg l
+  @  2 apricot and blueberry
+  
+  *  1 banana
+  |
+  x  0 apricot
+  
+
+Test various methods of handling that unfinished state
+  $ hg evolve --abort
+  evolve aborted
+  working directory is now at e1989e4b1526
+  $ ls .hg/evolvestate
+  ls: cannot access '?.hg/evolvestate'?: No such file or directory (re)
+  [2]
+  $ cat b
+  blueberry
+  $ hg l
+  @  2 apricot and blueberry
+  
+  *  1 banana
+  |
+  x  0 apricot
+  
+
+  $ hg evolve --update --config hooks.precommit=false --config ui.merge=:other
+  move:[1] banana
+  atop:[2] apricot and blueberry
+  transaction abort!
+  rollback completed
+  abort: precommit hook exited with status 1
+  [255]
+  $ cat b
+  banana
+  $ hg evolve --stop
+  stopped the interrupted evolve
+  working directory is now at e1989e4b1526
+  $ cat .hg/evolvestate
+  cat: .hg/evolvestate: No such file or directory
+  [1]
+  $ cat b
+  blueberry
+  $ hg l
+  @  2 apricot and blueberry
+  
+  *  1 banana
+  |
+  x  0 apricot
+  
+
+  $ hg evolve --update --config hooks.precommit=false --config ui.merge=:other
+  move:[1] banana
+  atop:[2] apricot and blueberry
+  transaction abort!
+  rollback completed
+  abort: precommit hook exited with status 1
+  [255]
+  $ hg evolve --continue
+  evolving 1:e0486f65907d "banana"
+  working directory is now at bd5ec7dfc2af
+  $ cat .hg/evolvestate
+  cat: .hg/evolvestate: No such file or directory
+  [1]
+  $ cat b
+  banana
+  $ hg l
+  @  3 banana
+  |
+  o  2 apricot and blueberry
+