evolve: lock the working copy early in next and prev (issue5244) stable
authorSimon Farnsworth <simonfar@fb.com>
Sun, 16 Oct 2016 09:52:53 -0700
branchstable
changeset 1742 970a4c13ebc3
parent 1741 8c05184672ae
child 1743 299cdaa24fa5
child 1744 db0a54205221
child 1745 6c922448ccec
evolve: lock the working copy early in next and prev (issue5244) Both next and prev depend on a consistent working copy, but were waiting to take the lock until they were ready to alter the working copy. Take the lock before reading the working copy state, and do not release it until we're definitely not going to change the working copy.
README
hgext/evolve.py
tests/fake-editor.sh
tests/test-prev-next.t
--- a/README	Sat Oct 08 08:14:21 2016 -0700
+++ b/README	Sun Oct 16 09:52:53 2016 -0700
@@ -60,6 +60,8 @@
 
  - Fix erroneous manifest computation when solving 'bumped' changeset.
  - split: avoid crash on empty commit (issue5191),
+ - next: improve locking to avoid issue with working copy parent (issue5244)
+ - prev: improve locking to avoid issue with working copy parent (issue5244)
 
 5.4.1 -- 2016-08-01
 
--- a/hgext/evolve.py	Sat Oct 08 08:14:21 2016 -0700
+++ b/hgext/evolve.py	Sun Oct 16 09:52:53 2016 -0700
@@ -2221,10 +2221,13 @@
     """update to parent revision
 
     Displays the summary line of the destination for clarity."""
-    if True:
+    wlock = None
+    dryrunopt = opts['dry_run']
+    if not dryrunopt:
+        wlock = repo.wlock()
+    try:
         wkctx = repo[None]
         wparents = wkctx.parents()
-        dryrunopt = opts['dry_run']
         if len(wparents) != 1:
             raise error.Abort('merge in progress')
         if not opts['merge']:
@@ -2254,7 +2257,6 @@
                 ret = hg.update(repo, p.rev())
                 if not ret:
                     tr = lock = None
-                    wlock = repo.wlock()
                     try:
                         lock = repo.lock()
                         tr = repo.transaction('previous')
@@ -2265,7 +2267,8 @@
                             bmdeactivate(repo)
                         tr.close()
                     finally:
-                        lockmod.release(tr, lock, wlock)
+                        lockmod.release(tr, lock)
+
             displayer.show(p)
             return 0
         else:
@@ -2273,6 +2276,8 @@
                 displayer.show(p)
             ui.warn(_('multiple parents, explicitly update to one\n'))
             return 1
+    finally:
+        lockmod.release(wlock)
 
 @command('^next',
          [('B', 'move-bookmark', False,
@@ -2290,10 +2295,13 @@
 
     Displays the summary line of the destination for clarity.
     """
-    if True:
+    wlock = None
+    dryrunopt = opts['dry_run']
+    if not dryrunopt:
+        wlock = repo.wlock()
+    try:
         wkctx = repo[None]
         wparents = wkctx.parents()
-        dryrunopt = opts['dry_run']
         if len(wparents) != 1:
             raise error.Abort('merge in progress')
         if not opts['merge']:
@@ -2323,7 +2331,6 @@
                 ret = hg.update(repo, c.rev())
                 if not ret:
                     lock = tr = None
-                    wlock = repo.wlock()
                     try:
                         lock = repo.lock()
                         tr = repo.transaction('next')
@@ -2334,7 +2341,7 @@
                             bmdeactivate(repo)
                         tr.close()
                     finally:
-                        lockmod.release(tr, lock, wlock)
+                        lockmod.release(tr, lock)
             displayer.show(c)
             result = 0
         elif children:
@@ -2376,6 +2383,8 @@
                 return result
             return 1
         return result
+    finally:
+        lockmod.release(wlock)
 
 def _reachablefrombookmark(repo, revs, bookmarks):
     """filter revisions and bookmarks reachable from the given bookmark
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/fake-editor.sh	Sun Oct 16 09:52:53 2016 -0700
@@ -0,0 +1,3 @@
+#!/bin/sh
+sleep 5
+echo "new desc" >> $1
--- a/tests/test-prev-next.t	Sat Oct 08 08:14:21 2016 -0700
+++ b/tests/test-prev-next.t	Sun Oct 16 09:52:53 2016 -0700
@@ -206,3 +206,34 @@
   move:[5] added d
   atop:[6] added b (3)
   working directory is now at 47ea25be8aea
+
+prev and next should lock properly against other commands
+
+  $ hg init repo
+  $ cd repo
+  $ HGEDITOR=${TESTDIR}/fake-editor.sh
+  $ echo hi > foo
+  $ hg ci -Am 'one'
+  adding foo
+  $ echo bye > foo
+  $ hg ci -Am 'two'
+
+  $ hg amend --edit &
+  $ sleep 1
+  $ hg prev
+  waiting for lock on working directory of $TESTTMP/repo held by '*' (glob)
+  got lock after [4-6] seconds (re)
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  [0] one
+  $ wait
+
+  $ hg amend --edit &
+  $ sleep 1
+  $ hg next --evolve
+  waiting for lock on working directory of $TESTTMP/repo held by '*' (glob)
+  1 new unstable changesets
+  got lock after [4-6] seconds (re)
+  move:[2] two
+  atop:[3] one
+  working directory now at a7d885c75614
+  $ wait