narrow: keep bookmarks temporarily stripped for as long as commits are
authorMartin von Zweigbergk <>
Sat, 08 Dec 2018 23:41:54 -0800
changeset 53633 109a267acc1efb3b279b6b244d548a0cd4fe5878
parent 53632 0d50bfcd8f65f336aa21397cf58cc1d711b6ac2d
child 53634 f79659e1e50fdea654b5111285a4f816104d0b07
push id1079
push dateMon, 10 Dec 2018 19:44:59 +0000
narrow: keep bookmarks temporarily stripped for as long as commits are The narrow extension also has support for shallowness and for inserting older commits on pull. It works by temporarily stripping newer commits, adding the older commits, then re-applying the stripped bundle. The regular Mercurial server uses that when you widen, although it shouldn't be necessary there. Our Google-internal server does it when the user requests an older commit. Our Google-internal tests fail since 7caf632e30c3 (filecache: unimplement __set__() and __delete__() (API), 2018-10-20). I haven't quite understood the problem, but it's related to the way we temporarily hide bookmarks while the commits they point to are stripped. When a transaction is started, Mercurial tries to read various things from the repo for the transaction summary. That leads to computation of hidden commits, which leads to an attempt to find commits pinned by bookmarks. This is the reason we temporarily hide the bookmarks. I think the aforementioned commit makes the restored bookmarks visible earlier than before (which seems like an improvement), so we can no longer incorrectly rely on the repo._bookmarks field being cached too long (IIUC). This patch makes it so we restore the temporarily hidden bookmarks only after the temporary bundle has been re-applied. It also adapts the code to update the repo.__bookmarks field using the pattern described in the aforementioned commit instead of writing directly to the fiels. I have spent many hours trying to understand what was going on here, but I still don't know if this can also happen without our custom server. So this patch unfortunately does not add any tests; I have only been able to test the fix using our Google-internal tests. Differential Revision:
--- a/hgext/narrow/
+++ b/hgext/narrow/
@@ -15,16 +15,17 @@ from mercurial.node import (
 from mercurial import (
+    localrepo,
 from mercurial.utils import (
@@ -174,24 +175,23 @@ def _handlechangespec(op, inpart):
             raise error.Abort(
                 _('unexpected changespec node chunk type: %s') % chunksignal)
         chunksignal = changegroup.readexactly(inpart, 4)
     if clkills:
         # preserve bookmarks that repair.strip() would otherwise strip
-        bmstore = repo._bookmarks
+        op._bookmarksbackup = repo._bookmarks
         class dummybmstore(dict):
             def applychanges(self, repo, tr, changes):
-        repo._bookmarks = dummybmstore()
+        localrepo.localrepository._bookmarks.set(repo, dummybmstore())
         chgrpfile = repair.strip(op.ui, repo, list(clkills), backup=True,
-        repo._bookmarks = bmstore
         if chgrpfile:
             op._widen_uninterr = repo.ui.uninterruptable()
             # presence of _widen_bundle attribute activates widen handler later
             op._widen_bundle = chgrpfile
     # Set the new narrowspec if we're widening. The setnewnarrowpats() method
     # will currently always be there when using the core+narrowhg server, but
     # other servers may include a changespec part even when not widening (e.g.
@@ -265,10 +265,15 @@ def setup():
     exchange.getbundle2partsmapping['changegroup'] = wrappedcgfn
     # Extend changegroup receiver so client can fixup after widen requests.
     origcghandler = bundle2.parthandlermapping['changegroup']
     def wrappedcghandler(op, inpart):
         origcghandler(op, inpart)
         if util.safehasattr(op, '_widen_bundle'):
             handlechangegroup_widen(op, inpart)
+        if util.safehasattr(op, '_bookmarksbackup'):
+            localrepo.localrepository._bookmarks.set(op.repo,
+                                                     op._bookmarksbackup)
+            del op._bookmarksbackup
     wrappedcghandler.params = origcghandler.params
     bundle2.parthandlermapping['changegroup'] = wrappedcghandler