From patchwork Sat Mar 30 02:58:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [4,of,4] pull: list bookmarks before pulling changesets (issue3873) From: Siddharth Agarwal X-Patchwork-Id: 1229 Message-Id: <720475f6e382359a2057.1364612307@sid0x220> To: mercurial-devel@selenic.com Date: Fri, 29 Mar 2013 19:58:27 -0700 # HG changeset patch # User Siddharth Agarwal # Date 1364612046 25200 # Fri Mar 29 19:54:06 2013 -0700 # Node ID 720475f6e382359a20579224b1756a8c532a47a4 # Parent 16a8db1339b1dc7734fefdaca99beb3a3fa14d49 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. diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py --- a/mercurial/bookmarks.py +++ b/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): diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/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 diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py --- a/mercurial/subrepo.py +++ b/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): 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 @@ -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 < .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 diff --git a/tests/test-hook.t b/tests/test-hook.t --- a/tests/test-hook.t +++ b/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 diff --git a/tests/test-pull-http.t b/tests/test-pull-http.t --- a/tests/test-pull-http.t +++ b/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