D5449: pull: fix inconsistent view of bookmarks during pull (issue4700)

[valentin.gatienbaron (Valentin Gatien-Baron)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGbad05a6afdc8: pull: fix inconsistent view of bookmarks during pull (issue4700) (authored by valentin.gatienbaron, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D5449?vs=12938&id=12965#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5449?vs=12938&id=12965

REVISION DETAIL
  https://phab.mercurial-scm.org/D5449

AFFECTED FILES
  mercurial/commands.py
  tests/test-bookmarks-pushpull.t

CHANGE DETAILS

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
@@ -673,12 +673,13 @@
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
+  updating bookmark Y
   new changesets 0d60821d2197 (1 drafts)
   (run 'hg update' to get a working copy)
   $ hg book
      @                         1:0d2164f0ce0d
      X                         1:0d2164f0ce0d
-   * Y                         5:35d1ef0a8d1b
+   * Y                         6:0d60821d2197
      Z                         1:0d2164f0ce0d
   $ hg -R $TESTTMP/pull-race book
      @                         1:0d2164f0ce0d
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4414,49 +4414,47 @@
         revs, checkout = hg.addbranchrevs(repo, other, branches,
                                           opts.get('rev'))
 
-
         pullopargs = {}
-        if opts.get('bookmark'):
-            if not revs:
-                revs = []
-            # 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')
+
+        nodes = None
+        if opts['bookmark'] or revs:
+            # The list of bookmark used here is the same used to actually update
+            # the bookmark names, to avoid the race from issue 4689 and we do
+            # all lookup and bookmark queries in one go so they see the same
+            # version of the server state (issue 4700).
+            nodes = []
+            fnodes = []
+            revs = revs or []
+            if revs and not other.capable('lookup'):
+                err = _("other repository doesn't support revision lookup, "
+                        "so a rev cannot be specified.")
+                raise error.Abort(err)
+            with other.commandexecutor() as e:
+                fremotebookmarks = e.callcommand('listkeys', {
+                    'namespace': 'bookmarks'
+                })
+                for r in revs:
+                    fnodes.append(e.callcommand('lookup', {'key': r}))
+            remotebookmarks = fremotebookmarks.result()
             remotebookmarks = bookmarks.unhexlifybookmarks(remotebookmarks)
             pullopargs['remotebookmarks'] = remotebookmarks
             for b in opts['bookmark']:
                 b = repo._bookmarks.expandname(b)
                 if b not in remotebookmarks:
                     raise error.Abort(_('remote bookmark %s not found!') % b)
-                revs.append(hex(remotebookmarks[b]))
-
-        if revs:
-            try:
-                # When 'rev' is a bookmark name, we cannot guarantee that it
-                # will be updated with that name because of a race condition
-                # server side. (See issue 4689 for details)
-                oldrevs = revs
-                revs = [] # actually, nodes
-                for r in oldrevs:
-                    with other.commandexecutor() as e:
-                        node = e.callcommand('lookup', {'key': r}).result()
-
-                    revs.append(node)
-                    if r == checkout:
-                        checkout = node
-            except error.CapabilityError:
-                err = _("other repository doesn't support revision lookup, "
-                        "so a rev cannot be specified.")
-                raise error.Abort(err)
+                nodes.append(remotebookmarks[b])
+            for i, rev in enumerate(revs):
+                node = fnodes[i].result()
+                nodes.append(node)
+                if rev == checkout:
+                    checkout = node
 
         wlock = util.nullcontextmanager()
         if opts.get('update'):
             wlock = repo.wlock()
         with wlock:
             pullopargs.update(opts.get('opargs', {}))
-            modheads = exchange.pull(repo, other, heads=revs,
+            modheads = exchange.pull(repo, other, heads=nodes,
                                      force=opts.get('force'),
                                      bookmarks=opts.get('bookmark', ()),
                                      opargs=pullopargs).cgresult



To: valentin.gatienbaron, #hg-reviewers
Cc: yuja, indygreg, mercurial-devel