Patchwork [1,of,3,STABLE] update: wlock the repo for the whole 'hg update' command

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 12, 2015, 12:06 a.m.
Message ID <6070fb278da8f8e618f8.1439338003@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/10191/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 12, 2015, 12:06 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1439335572 25200
#      Tue Aug 11 16:26:12 2015 -0700
# Branch stable
# Node ID 6070fb278da8f8e618f826a29feb5cb78ca038a0
# Parent  d4e1e947444b81a9e0a7d5dc7bfa282b38d00b94
update: wlock the repo for the whole 'hg update' command

The update command is touching the repository and should lock it for the length
of its operations. Equally importantly it should lock the repository when it is
writing bookmarks. It wasn't doing so until now, leaving doors open for all
kinds of drunk beavers party.

This result in some minor tests changes, and the fixing of a couple of bug from
race conditions.

Code is does not receives any changes beside it's extra indendation.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -6449,55 +6449,59 @@  def update(ui, repo, node=None, rev=None
         raise util.Abort(_("please specify just one revision"))
 
     if rev is None or rev == '':
         rev = node
 
-    cmdutil.clearunfinished(repo)
-
-    # with no argument, we also move the active bookmark, if any
-    rev, movemarkfrom = bookmarks.calculateupdate(ui, repo, rev)
-
-    # if we defined a bookmark, we have to remember the original bookmark name
-    brev = rev
-    rev = scmutil.revsingle(repo, rev, rev).rev()
-
-    if check and clean:
-        raise util.Abort(_("cannot specify both -c/--check and -C/--clean"))
-
-    if date:
-        if rev is not None:
-            raise util.Abort(_("you can't specify a revision and a date"))
-        rev = cmdutil.finddate(ui, repo, date)
-
-    if check:
-        cmdutil.bailifchanged(repo, merge=False)
-        if rev is None:
-            rev = repo[repo[None].branch()].rev()
-
-    repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
-
-    if clean:
-        ret = hg.clean(repo, rev)
-    else:
-        ret = hg.update(repo, rev)
-
-    if not ret and movemarkfrom:
-        if bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
-            ui.status(_("updating bookmark %s\n") % repo._activebookmark)
+    wlock = repo.wlock()
+    try:
+        cmdutil.clearunfinished(repo)
+
+        # with no argument, we also move the active bookmark, if any
+        rev, movemarkfrom = bookmarks.calculateupdate(ui, repo, rev)
+
+        # if we defined a bookmark, we have to remember the original name
+        brev = rev
+        rev = scmutil.revsingle(repo, rev, rev).rev()
+
+        if check and clean:
+            raise util.Abort(_("cannot specify both -c/--check and -C/--clean"))
+
+        if date:
+            if rev is not None:
+                raise util.Abort(_("you can't specify a revision and a date"))
+            rev = cmdutil.finddate(ui, repo, date)
+
+        if check:
+            cmdutil.bailifchanged(repo, merge=False)
+            if rev is None:
+                rev = repo[repo[None].branch()].rev()
+
+        repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
+
+        if clean:
+            ret = hg.clean(repo, rev)
         else:
-            # this can happen with a non-linear update
-            ui.status(_("(leaving bookmark %s)\n") %
-                      repo._activebookmark)
+            ret = hg.update(repo, rev)
+
+        if not ret and movemarkfrom:
+            if bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
+                ui.status(_("updating bookmark %s\n") % repo._activebookmark)
+            else:
+                # this can happen with a non-linear update
+                ui.status(_("(leaving bookmark %s)\n") %
+                          repo._activebookmark)
+                bookmarks.deactivate(repo)
+        elif brev in repo._bookmarks:
+            bookmarks.activate(repo, brev)
+            ui.status(_("(activating bookmark %s)\n") % brev)
+        elif brev:
+            if repo._activebookmark:
+                ui.status(_("(leaving bookmark %s)\n") %
+                          repo._activebookmark)
             bookmarks.deactivate(repo)
-    elif brev in repo._bookmarks:
-        bookmarks.activate(repo, brev)
-        ui.status(_("(activating bookmark %s)\n") % brev)
-    elif brev:
-        if repo._activebookmark:
-            ui.status(_("(leaving bookmark %s)\n") %
-                      repo._activebookmark)
-        bookmarks.deactivate(repo)
+    finally:
+        wlock.release()
 
     return ret
 
 @command('verify', [])
 def verify(ui, repo):
diff --git a/tests/test-blackbox.t b/tests/test-blackbox.t
--- a/tests/test-blackbox.t
+++ b/tests/test-blackbox.t
@@ -117,12 +117,12 @@  extension and python hooks - use the eol
   $ echo '[extensions]' >> .hg/hgrc
   $ echo 'eol=' >> .hg/hgrc
   $ echo '[hooks]' >> .hg/hgrc
   $ echo 'update = echo hooked' >> .hg/hgrc
   $ hg update
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   hooked
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg blackbox -l 5
   1970/01/01 00:00:00 bob> update
   1970/01/01 00:00:00 bob> writing .hg/cache/tags2-visible with 0 tags
   1970/01/01 00:00:00 bob> pythonhook-preupdate: hgext.eol.preupdate finished in * seconds (glob)
   1970/01/01 00:00:00 bob> exthook-update: echo hooked finished in * seconds (glob)
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -221,12 +221,12 @@  preupdate hook can prevent update
 update hook
 
   $ echo "update = printenv.py update" >> .hg/hgrc
   $ hg update
   preupdate hook: HG_PARENT1=539e4b31b6dc
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   update hook: HG_ERROR=0 HG_PARENT1=539e4b31b6dc
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
 pushkey hook
 
   $ echo "pushkey = printenv.py pushkey" >> .hg/hgrc
   $ cd ../b
@@ -642,12 +642,12 @@  final release (and dirstate flush).
   $ echo 'update = hg id' >> .hg/hgrc
   $ echo bb > a
   $ hg ci -ma
   223eafe2750c tip
   $ hg up 0 --config extensions.largefiles=
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   cb9a9f314b8b
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
 make sure --verbose (and --quiet/--debug etc.) are propagated to the local ui
 that is passed to pre/post hooks
 
   $ echo '[hooks]' > .hg/hgrc
diff --git a/tests/test-lock-badness.t b/tests/test-lock-badness.t
--- a/tests/test-lock-badness.t
+++ b/tests/test-lock-badness.t
@@ -57,11 +57,10 @@  One process waiting for another
   $ echo b > b/b
   $ hg -R b ci -A -m b --config hooks.precommit="python:`pwd`/hooks.py:sleepone" > stdout &
   $ hg -R b up -q --config hooks.pre-update="python:`pwd`/hooks.py:sleephalf"
   waiting for lock on working directory of b held by '*:*' (glob)
   got lock after ? seconds (glob)
-  warning: ignoring unknown working parent d2ae7f538514!
   $ wait
   $ cat stdout
   adding b
 
 Pushing to a local read-only repo that can't be locked