Patchwork [6,of,6] pull: prevent race condition in bookmark update when using -B (issue4689)

login
register
mail settings
Submitter Pierre-Yves David
Date June 5, 2015, 5:20 a.m.
Message ID <3a0a6d08cbb071ddf9fc.1433481603@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/9515/
State Accepted
Headers show

Comments

Pierre-Yves David - June 5, 2015, 5:20 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1433223241 25200
#      Mon Jun 01 22:34:01 2015 -0700
# Node ID 3a0a6d08cbb071ddf9fc5da462a66813367c5ac9
# Parent  4f36fdb8dc6586c41a09f9509df3c1b5dbeaf343
pull: prevent race condition in bookmark update when using -B (issue4689)

We are already fetching remote bookmarks to honor the -B option, we now pass
that data to the pull process so it can reuse it. This prevent race condition
between the initial looking and the actual pulling of changesets and bookmarks.
Tests are updated to handle this fact.
Matt Mackall - June 5, 2015, 5:45 p.m.
On Thu, 2015-06-04 at 22:20 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1433223241 25200
> #      Mon Jun 01 22:34:01 2015 -0700
> # Node ID 3a0a6d08cbb071ddf9fc5da462a66813367c5ac9
> # Parent  4f36fdb8dc6586c41a09f9509df3c1b5dbeaf343
> pull: prevent race condition in bookmark update when using -B (issue4689)

These are queued for default, thanks.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5127,10 +5127,11 @@  def pull(ui, repo, source="default", **o
             # The list of bookmark used here is not the one used to actually
             # update the bookmark name. This can result in the revision pulled
             # not ending up with the name of the bookmark because of a race
             # condition on the server. (See issue 4689 for details)
             remotebookmarks = other.listkeys('bookmarks')
+            pullopargs['remotebookmarks'] = remotebookmarks
             for b in opts['bookmark']:
                 if b not in remotebookmarks:
                     raise util.Abort(_('remote bookmark %s not found!') % b)
                 revs.append(remotebookmarks[b])
 
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -806,11 +806,12 @@  class pulloperation(object):
 
     A new should be created at the beginning of each pull and discarded
     afterward.
     """
 
-    def __init__(self, repo, remote, heads=None, force=False, bookmarks=()):
+    def __init__(self, repo, remote, heads=None, force=False, bookmarks=(),
+                 remotebookmarks=None):
         # repo we pull into
         self.repo = repo
         # repo we pull from
         self.remote = remote
         # revision we try to pull (None is "all")
@@ -826,11 +827,11 @@  class pulloperation(object):
         # set of pulled head
         self.rheads = None
         # list of missing changeset to fetch remotely
         self.fetch = None
         # remote bookmarks data
-        self.remotebookmarks = None
+        self.remotebookmarks = remotebookmarks
         # result of changegroup pulling (used as return code by pull)
         self.cgresult = None
         # list of step already done
         self.stepsdone = set()
 
diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -305,10 +305,47 @@  update a bookmark in the middle of a cli
    * @                         1:0d2164f0ce0d
      X                         1:0d2164f0ce0d
      Y                         4:b0a5eff05604
      Z                         1:0d2164f0ce0d
 
+Update a bookmark right after the initial lookup -B (issue4689)
+
+  $ echo c6 > ../pull-race/f3 # to be committed during the race
+  $ cat <<EOF > ../pull-race/.hg/hgrc
+  > [hooks]
+  > # if anything to commit, commit it right after the first key listing used #
+  > # during lookup. This make the commit appears before the actual getbundle
+  > # call.
+  > listkeys.makecommit= ((hg st | grep -q M) && (hg commit -m race; echo commited in pull-race)) || exit 0
+  > EOF
+
+(new config need server restart)
+
+  $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS
+  $ hg -R ../pull-race serve -p $HGPORT -d --pid-file=../pull-race.pid -E main-error.log
+  $ cat ../pull-race.pid >> $DAEMON_PIDS
+
+  $ hg -R $TESTTMP/pull-race book
+     @                         1:0d2164f0ce0d
+     X                         1:0d2164f0ce0d
+   * Y                         5:35d1ef0a8d1b
+     Z                         1:0d2164f0ce0d
+  $ hg pull -B Y
+  pulling from http://localhost:$HGPORT/
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  updating bookmark Y
+  (run 'hg update' to get a working copy)
+  $ hg book
+   * @                         1:0d2164f0ce0d
+     X                         1:0d2164f0ce0d
+     Y                         5:35d1ef0a8d1b
+     Z                         1:0d2164f0ce0d
+
 (done with this section of the test)
 
   $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS
   $ cd ../b
 
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -251,11 +251,10 @@  listkeys hook
   $ hg pull -B bar ../a
   pulling from ../a
   listkeys hook: HG_NAMESPACE=bookmarks HG_VALUES={'bar': '0000000000000000000000000000000000000000', 'foo': '0000000000000000000000000000000000000000'}
   no changes found
   listkeys hook: HG_NAMESPACE=phase HG_VALUES={}
-  listkeys hook: HG_NAMESPACE=bookmarks HG_VALUES={'bar': '0000000000000000000000000000000000000000', 'foo': '0000000000000000000000000000000000000000'}
   adding remote bookmark bar
   listkeys hook: HG_NAMESPACE=phases HG_VALUES={'cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b': '1', 'publishing': 'True'}
   $ cd ../a
 
 test that prepushkey can prevent incoming keys