bookmarks: use repo.localvfs and repo.sharedvfs instead of repo.vfs

Summary:
Update bookmarks to work correctly when shared.  Rather than copying the
bookmarks file over after transaction handling, update the file in both
locations when bookmarks are shared.

Reviewed By: quark-zju

Differential Revision: D9699158

fbshipit-source-id: 2f75aaac364ffe02e59441ac0f39fb7d8e5d5d2e

mercurial/bookmarks.py

@@ -40,31 +40,11 @@ def _getbkfile(repo):
     """
 
     if repo._hassharedbookmarks:
-        srcrepo = repo._sharedprimaryrepo
-        if srcrepo is not None:
-            # just orig(srcrepo) doesn't work as expected, because
-            # HG_PENDING refers repo.root.
-            try:
-                fp, pending = txnutil.trypending(repo.root, repo.vfs, "bookmarks")
-                if pending:
-                    # only in this case, bookmark information in repo
-                    # is up-to-date.
-                    return fp
-                fp.close()
-            except IOError as inst:
-                if inst.errno != errno.ENOENT:
-                    raise
-
-            # otherwise, we should read bookmarks from srcrepo,
-            # because .hg/bookmarks in srcrepo might be already
-            # changed via another sharing repo
-            repo = srcrepo
-
-            # TODO: Pending changes in repo are still invisible in
-            # srcrepo, because bookmarks.pending is written only into repo.
-            # See also https://www.mercurial-scm.org/wiki/SharedRepository
-
-    fp, pending = txnutil.trypending(repo.root, repo.vfs, "bookmarks")
+        fp, pending = txnutil.trysharedpending(
+            repo.root, repo.sharedroot, repo.sharedvfs, "bookmarks"
+        )
+    else:
+        fp, pending = txnutil.trypending(repo.root, repo.localvfs, "bookmarks")
     return fp
 
 
@@ -172,29 +152,14 @@ class bmstore(dict):
 
         The transaction is then responsible for updating the file content."""
         tr.addfilegenerator("bookmarks", ("bookmarks",), self._write, location="local")
+        if (
+            self._repo._hassharedbookmarks
+            and self._repo.localvfs is not self._repo.sharedvfs
+        ):
+            tr.addfilegenerator(
+                "shared-bookmarks", ("bookmarks",), self._write, location="shared"
+            )
         tr.hookargs["bookmark_moved"] = "1"
-        if self._repo._hassharedbookmarks:
-            srcrepo = self._repo._sharedprimaryrepo
-            if srcrepo is not None:
-                category = "share-bookmarks"
-                tr.addpostclose(category, lambda tr: self._writeprimaryrepo(srcrepo))
-
-    def _writeprimaryrepo(self, repo):
-        """Write bookmarks to the primary shared repo"""
-        rbm = repo._bookmarks
-        if rbm.active not in self:
-            rbm.active = None
-            rbm._writeactive()
-
-        with repo.wlock():
-            file_ = repo.vfs("bookmarks", "w", atomictemp=True, checkambig=True)
-            try:
-                self._write(file_)
-            except:  # re-raises
-                file_.discard()
-                raise
-            finally:
-                file_.close()
 
     def _writeactive(self):
         if self._aclean:

mercurial/localrepo.py

@@ -833,7 +833,9 @@ class localrepository(object):
         self._primaryrepo = primaryrepo
         return primaryrepo
 
-    @repofilecache(localpaths=["bookmarks", "bookmarks.current"])
+    @repofilecache(
+        sharedpaths=["bookmarks"], localpaths=["bookmarks", "bookmarks.current"]
+    )
     def _bookmarks(self):
         return bookmarks.bmstore(self)
 

tests/test-share.t

@@ -171,7 +171,7 @@ test sharing bookmarks
    * bm1                       2:c2e0ac586386
      bm3                       2:c2e0ac586386
 
-check whether HG_PENDING makes pending changes only in relatd
+check whether HG_PENDING makes pending changes only in related
 repositories visible to an external hook.
 
 In "hg share" case, another transaction can't run in other
@@ -179,7 +179,7 @@ repositories sharing same source repository, because starting
 transaction requires locking store of source repository.
 
 Therefore, this test scenario ignores checking visibility of
-.hg/bookmakrs.pending in repo2, which shares repo1 without bookmarks.
+.hg/bookmarks.pending in repo2, which shares repo1 without bookmarks.
 
   $ cat > $TESTTMP/checkbookmarks.sh <<EOF
   > echo "@repo1"
@@ -209,15 +209,34 @@ Therefore, this test scenario ignores checking visibility of
   [255]
   $ hg book bm1
 
-FYI, in contrast to above test, bmX is invisible in repo1 (= shared
-src), because (1) HG_PENDING refers only repo3 and (2)
-"bookmarks.pending" is written only into repo3.
+In the unshared case, a bookmark being added in repo2 is not visible in repo1.
+
+  $ cd ../repo2
+  $ hg --config hooks.pretxnclose="sh $TESTTMP/checkbookmarks.sh" -q book bmX
+  @repo1
+   * bm1                       2:c2e0ac586386
+     bm3                       2:c2e0ac586386
+  @repo2
+     bm2                       3:0e6e70d1d5f1
+   * bmX                       3:0e6e70d1d5f1
+  @repo3
+     bm1                       2:c2e0ac586386
+   * bm3                       2:c2e0ac586386
+  transaction abort!
+  rollback completed
+  abort: pretxnclose hook exited with status 1
+  [255]
+  $ hg book bm2
+
+In symmetry with the first case, bmX is visible in repo1 (= shared rc)
+because HG_SHAREDPENDING refers to repo1.
 
   $ cd ../repo3
   $ hg --config hooks.pretxnclose="sh $TESTTMP/checkbookmarks.sh" -q book bmX
   @repo1
    * bm1                       2:c2e0ac586386
      bm3                       2:c2e0ac586386
+     bmX                       2:c2e0ac586386
   @repo2
    * bm2                       3:0e6e70d1d5f1
   @repo3