pull: list bookmarks before pulling changesets (issue3873)

Consider a bookmark B that exists both locally and remotely. If B is updated
remotely, and then a pull is performed where the pull set contains the new
location of B, the bookmark is updated locally. However, if remote B is
updated in the middle of a pull to a location not in the pull set, the
bookmark won't be updated locally at all.

To fix this, list bookmarks before pulling in changesets, not after. This
still leaves a race open if B gets moved in between listing bookmarks and
pulling in changesets, but the race window is much smaller. Fixing the race
properly would require a bundle format upgrade.

test-hook.t's output changes because we no longer do two listkeys calls during
pull, just one.

test-pull-http.t's output changes because we now search for bookmarks before
searching for changes.

mercurial/bookmarks.py

@@ -221,9 +221,8 def pushbookmark(repo, key, old, new):
     finally:
         w.release()
 
-def updatefromremote(ui, repo, remote, path):
+def updatefromremote(ui, repo, remotemarks, path):
     ui.debug("checking for updated bookmarks\n")
-    remotemarks = remote.listkeys('bookmarks')
     changed = False
     localmarks = repo._bookmarks
     for k in sorted(remotemarks):

mercurial/commands.py

@@ -4496,10 +4496,11 def pull(ui, repo, source="default", **o
     ui.status(_('pulling from %s\n') % util.hidepassword(source))
     revs, checkout = hg.addbranchrevs(repo, other, branches, opts.get('rev'))
 
+    remotebookmarks = other.listkeys('bookmarks')
+
     if opts.get('bookmark'):
         if not revs:
             revs = []
-        remotebookmarks = other.listkeys('bookmarks')
         for b in opts['bookmark']:
             if b not in remotebookmarks:
                 raise util.Abort(_('remote bookmark %s not found!') % b)
@@ -4514,7 +4515,7 def pull(ui, repo, source="default", **o
             raise util.Abort(err)
 
     modheads = repo.pull(other, heads=revs, force=opts.get('force'))
-    bookmarks.updatefromremote(ui, repo, other, source)
+    bookmarks.updatefromremote(ui, repo, remotebookmarks, source)
     if checkout:
         checkout = str(repo.changelog.rev(other.lookup(checkout)))
     repo._subtoppath = source

mercurial/subrepo.py

@@ -547,9 +547,10 class hgsubrepo(abstractsubrepo):
             else:
                 self._repo.ui.status(_('pulling subrepo %s from %s\n')
                                      % (subrelpath(self), srcurl))
+                remotebookmarks = other.listkeys('bookmarks')
                 self._repo.pull(other)
-                bookmarks.updatefromremote(self._repo.ui, self._repo, other,
-                                           srcurl)
+                bookmarks.updatefromremote(self._repo.ui, self._repo,
+                                           remotebookmarks, srcurl)
 
     @annotatesubrepoerror
     def get(self, state, overwrite=False):

tests/test-bookmarks-pushpull.t

@@ -204,6 +204,39 update a remote bookmark from a non-head
      Y                         3:f6fc62dde3c0
      Z                         1:0d2164f0ce0d
 
+update a bookmark in the middle of a client pulling changes
+
+  $ cd ..
+  $ hg clone -q a pull-race
+  $ hg clone -q pull-race pull-race2
+  $ cd pull-race
+  $ hg up -q Y
+  $ echo c4 > f2
+  $ hg ci -Am4
+  $ echo c5 > f3
+  $ cat <<EOF > .hg/hgrc
+  > [hooks]
+  > outgoing.makecommit = hg ci -Am5; echo committed in pull-race
+  > EOF
+  $ cd ../pull-race2
+  $ hg pull
+  pulling from $TESTTMP/pull-race (glob)
+  searching for changes
+  adding changesets
+  adding f3
+  committed in pull-race
+  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                         4:b0a5eff05604
+     Z                         1:0d2164f0ce0d
+  $ cd ../b
+
 diverging a remote bookmark fails
 
   $ hg up -q 4e3505fd9583

tests/test-hook.t

@@ -198,7 +198,6 listkeys hook
   listkeys hook: HG_NAMESPACE=bookmarks HG_VALUES={'bar': '0000000000000000000000000000000000000000', 'foo': '0000000000000000000000000000000000000000'}
   no changes found
   listkeys hook: HG_NAMESPACE=phases HG_VALUES={'cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b': '1', 'publishing': 'True'}
-  listkeys hook: HG_NAMESPACE=bookmarks HG_VALUES={'bar': '0000000000000000000000000000000000000000', 'foo': '0000000000000000000000000000000000000000'}
   adding remote bookmark bar
   importing bookmark bar
   $ cd ../a

tests/test-pull-http.t

@@ -58,7 +58,6 expect error, pulling not allowed
 
   $ req
   pulling from http://localhost:$HGPORT/
-  searching for changes
   abort: authorization failed
   % serve errors