bookmarks: introduce a bmstore to manage bookmark persistence
authorAugie Fackler <raf@durin42.com>
Wed, 07 Nov 2012 16:21:39 -0600
changeset 17922 7f5dab94e48c
parent 17921 4ac9cf3d810c
child 17923 1e6b5faf9d4e
bookmarks: introduce a bmstore to manage bookmark persistence Bookmarks persistence still showed a fair amount of its legacy as a monkeypatching extension. This encapsulates all bookmarks serialization and parsing in a single class, and offers a single location where other bookmarks storage engines can be substituted in. As a result, many files no longer import the bookmarks module, which strikes me as an encapsulation win. This doesn't do anything to the current bookmark state yet, but I'm hoping put that in the bmstore class as well.
hgext/convert/hg.py
hgext/histedit.py
hgext/mq.py
hgext/rebase.py
mercurial/bookmarks.py
mercurial/cmdutil.py
mercurial/commands.py
mercurial/hg.py
mercurial/localrepo.py
mercurial/repair.py
--- a/hgext/convert/hg.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/hgext/convert/hg.py	Wed Nov 07 16:21:39 2012 -0600
@@ -219,9 +219,10 @@
             return
 
         self.ui.status(_("updating bookmarks\n"))
+        destmarks = self.repo._bookmarks
         for bookmark in updatedbookmark:
-            self.repo._bookmarks[bookmark] = bin(updatedbookmark[bookmark])
-            bookmarks.write(self.repo)
+            destmarks[bookmark] = bin(updatedbookmark[bookmark])
+        destmarks.write()
 
     def hascommit(self, rev):
         if rev not in self.repo and self.clonebranches:
--- a/hgext/histedit.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/hgext/histedit.py	Wed Nov 07 16:21:39 2012 -0600
@@ -144,7 +144,6 @@
     import pickle
 import os
 
-from mercurial import bookmarks
 from mercurial import cmdutil
 from mercurial import discovery
 from mercurial import error
@@ -740,12 +739,13 @@
             # nothing to move
         moves.append((bk, new[-1]))
     if moves:
+        marks = repo._bookmarks
         for mark, new in moves:
-            old = repo._bookmarks[mark]
+            old = marks[mark]
             ui.note(_('histedit: moving bookmarks %s from %s to %s\n')
                     % (mark, node.short(old), node.short(new)))
-            repo._bookmarks[mark] = new
-        bookmarks.write(repo)
+            marks[mark] = new
+        marks.write()
 
 def cleanupnode(ui, repo, name, nodes):
     """strip a group of nodes from the repository
--- a/hgext/mq.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/hgext/mq.py	Wed Nov 07 16:21:39 2012 -0600
@@ -63,7 +63,7 @@
 from mercurial.node import bin, hex, short, nullid, nullrev
 from mercurial.lock import release
 from mercurial import commands, cmdutil, hg, scmutil, util, revset
-from mercurial import repair, extensions, error, phases, bookmarks
+from mercurial import repair, extensions, error, phases
 from mercurial import patch as patchmod
 import os, re, errno, shutil
 
@@ -1675,9 +1675,10 @@
                     patchf.write(chunk)
                 patchf.close()
 
+                marks = repo._bookmarks
                 for bm in bmlist:
-                    repo._bookmarks[bm] = n
-                bookmarks.write(repo)
+                    marks[bm] = n
+                marks.write()
 
                 self.applied.append(statusentry(n, patchfn))
             except: # re-raises
@@ -2999,7 +3000,7 @@
             revs.update(set(rsrevs))
         if not revs:
             del marks[mark]
-            repo._writebookmarks(mark)
+            marks.write()
             ui.write(_("bookmark '%s' deleted\n") % mark)
 
     if not revs:
@@ -3049,7 +3050,7 @@
 
     if opts.get('bookmark'):
         del marks[mark]
-        repo._writebookmarks(marks)
+        marks.write()
         ui.write(_("bookmark '%s' deleted\n") % mark)
 
     repo.mq.strip(repo, revs, backup=backup, update=update,
--- a/hgext/rebase.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/hgext/rebase.py	Wed Nov 07 16:21:39 2012 -0600
@@ -479,13 +479,14 @@
 
 def updatebookmarks(repo, nstate, originalbookmarks, **opts):
     'Move bookmarks to their correct changesets'
+    marks = repo._bookmarks
     for k, v in originalbookmarks.iteritems():
         if v in nstate:
             if nstate[v] != nullmerge:
                 # update the bookmarks for revs that have moved
-                repo._bookmarks[k] = nstate[v]
+                marks[k] = nstate[v]
 
-    bookmarks.write(repo)
+    marks.write()
 
 def storestatus(repo, originalwd, target, state, collapse, keep, keepbranches,
                                                                 external):
--- a/mercurial/bookmarks.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/mercurial/bookmarks.py	Wed Nov 07 16:21:39 2012 -0600
@@ -10,32 +10,72 @@
 from mercurial import encoding, error, util, obsolete
 import errno, os
 
-def read(repo):
-    '''Parse .hg/bookmarks file and return a dictionary
+class bmstore(dict):
+    """Storage for bookmarks.
+
+    This object should do all bookmark reads and writes, so that it's
+    fairly simple to replace the storage underlying bookmarks without
+    having to clone the logic surrounding bookmarks.
+
+    This particular bmstore implementation stores bookmarks as
+    {hash}\s{name}\n (the same format as localtags) in
+    .hg/bookmarks. The mapping is stored as {name: nodeid}.
+
+    This class does NOT handle the "current" bookmark state at this
+    time.
+    """
 
-    Bookmarks are stored as {HASH}\\s{NAME}\\n (localtags format) values
-    in the .hg/bookmarks file.
-    Read the file and return a (name=>nodeid) dictionary
-    '''
-    bookmarks = {}
-    try:
-        for line in repo.opener('bookmarks'):
-            line = line.strip()
-            if not line:
-                continue
-            if ' ' not in line:
-                repo.ui.warn(_('malformed line in .hg/bookmarks: %r\n') % line)
-                continue
-            sha, refspec = line.split(' ', 1)
-            refspec = encoding.tolocal(refspec)
+    def __init__(self, repo):
+        dict.__init__(self)
+        self._repo = repo
+        try:
+            for line in repo.vfs('bookmarks'):
+                line = line.strip()
+                if not line:
+                    continue
+                if ' ' not in line:
+                    repo.ui.warn(_('malformed line in .hg/bookmarks: %r\n')
+                                 % line)
+                    continue
+                sha, refspec = line.split(' ', 1)
+                refspec = encoding.tolocal(refspec)
+                try:
+                    self[refspec] = repo.changelog.lookup(sha)
+                except LookupError:
+                    pass
+        except IOError, inst:
+            if inst.errno != errno.ENOENT:
+                raise
+
+    def write(self):
+        '''Write bookmarks
+
+        Write the given bookmark => hash dictionary to the .hg/bookmarks file
+        in a format equal to those of localtags.
+
+        We also store a backup of the previous state in undo.bookmarks that
+        can be copied back on rollback.
+        '''
+        repo = self._repo
+        if repo._bookmarkcurrent not in self:
+            setcurrent(repo, None)
+
+        wlock = repo.wlock()
+        try:
+
+            file = repo.vfs('bookmarks', 'w', atomictemp=True)
+            for name, node in self.iteritems():
+                file.write("%s %s\n" % (hex(node), encoding.fromlocal(name)))
+            file.close()
+
+            # touch 00changelog.i so hgweb reloads bookmarks (no lock needed)
             try:
-                bookmarks[refspec] = repo.changelog.lookup(sha)
-            except LookupError:
+                os.utime(repo.sjoin('00changelog.i'), None)
+            except OSError:
                 pass
-    except IOError, inst:
-        if inst.errno != errno.ENOENT:
-            raise
-    return bookmarks
+
+        finally:
+            wlock.release()
 
 def readcurrent(repo):
     '''Get the current bookmark
@@ -60,37 +100,6 @@
         file.close()
     return mark
 
-def write(repo):
-    '''Write bookmarks
-
-    Write the given bookmark => hash dictionary to the .hg/bookmarks file
-    in a format equal to those of localtags.
-
-    We also store a backup of the previous state in undo.bookmarks that
-    can be copied back on rollback.
-    '''
-    refs = repo._bookmarks
-
-    if repo._bookmarkcurrent not in refs:
-        setcurrent(repo, None)
-
-    wlock = repo.wlock()
-    try:
-
-        file = repo.opener('bookmarks', 'w', atomictemp=True)
-        for refspec, node in refs.iteritems():
-            file.write("%s %s\n" % (hex(node), encoding.fromlocal(refspec)))
-        file.close()
-
-        # touch 00changelog.i so hgweb reloads bookmarks (no lock needed)
-        try:
-            os.utime(repo.sjoin('00changelog.i'), None)
-        except OSError:
-            pass
-
-    finally:
-        wlock.release()
-
 def setcurrent(repo, mark):
     '''Set the name of the bookmark that we are currently on
 
@@ -152,7 +161,7 @@
             if mark != cur:
                 del marks[mark]
     if update:
-        repo._writebookmarks(marks)
+        marks.write()
     return update
 
 def listbookmarks(repo):
@@ -179,7 +188,7 @@
             if new not in repo:
                 return False
             marks[key] = repo[new].node()
-        write(repo)
+        marks.write()
         return True
     finally:
         w.release()
@@ -188,16 +197,17 @@
     ui.debug("checking for updated bookmarks\n")
     rb = remote.listkeys('bookmarks')
     changed = False
+    localmarks = repo._bookmarks
     for k in rb.keys():
-        if k in repo._bookmarks:
-            nr, nl = rb[k], repo._bookmarks[k]
+        if k in localmarks:
+            nr, nl = rb[k], localmarks[k]
             if nr in repo:
                 cr = repo[nr]
                 cl = repo[nl]
                 if cl.rev() >= cr.rev():
                     continue
                 if validdest(repo, cl, cr):
-                    repo._bookmarks[k] = cr.node()
+                    localmarks[k] = cr.node()
                     changed = True
                     ui.status(_("updating bookmark %s\n") % k)
                 else:
@@ -208,7 +218,7 @@
                     # find a unique @ suffix
                     for x in range(1, 100):
                         n = '%s@%d' % (kd, x)
-                        if n not in repo._bookmarks:
+                        if n not in localmarks:
                             break
                     # try to use an @pathalias suffix
                     # if an @pathalias already exists, we overwrite (update) it
@@ -216,17 +226,17 @@
                         if path == u:
                             n = '%s@%s' % (kd, p)
 
-                    repo._bookmarks[n] = cr.node()
+                    localmarks[n] = cr.node()
                     changed = True
                     ui.warn(_("divergent bookmark %s stored as %s\n") % (k, n))
         elif rb[k] in repo:
             # add remote bookmarks for changes we already have
-            repo._bookmarks[k] = repo[rb[k]].node()
+            localmarks[k] = repo[rb[k]].node()
             changed = True
             ui.status(_("adding remote bookmark %s\n") % k)
 
     if changed:
-        write(repo)
+        localmarks.write()
 
 def diff(ui, dst, src):
     ui.status(_("searching for changed bookmarks\n"))
--- a/mercurial/cmdutil.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/mercurial/cmdutil.py	Wed Nov 07 16:21:39 2012 -0600
@@ -10,7 +10,7 @@
 import os, sys, errno, re, tempfile
 import util, scmutil, templater, patch, error, templatekw, revlog, copies
 import match as matchmod
-import subrepo, context, repair, bookmarks, graphmod, revset, phases, obsolete
+import subrepo, context, repair, graphmod, revset, phases, obsolete
 import changelog
 import lock as lockmod
 
@@ -1756,9 +1756,10 @@
                 # Move bookmarks from old parent to amend commit
                 bms = repo.nodebookmarks(old.node())
                 if bms:
+                    marks = repo._bookmarks
                     for bm in bms:
-                        repo._bookmarks[bm] = newid
-                    bookmarks.write(repo)
+                        marks[bm] = newid
+                    marks.write()
             #commit the whole amend process
             if obsolete._enabled and newid != old.node():
                 # mark the new changeset as successor of the rewritten one
--- a/mercurial/commands.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/mercurial/commands.py	Wed Nov 07 16:21:39 2012 -0600
@@ -821,7 +821,7 @@
         if mark == repo._bookmarkcurrent:
             bookmarks.setcurrent(repo, None)
         del marks[mark]
-        bookmarks.write(repo)
+        marks.write()
 
     elif rename:
         if mark is None:
@@ -834,7 +834,7 @@
         if repo._bookmarkcurrent == rename and not inactive:
             bookmarks.setcurrent(repo, mark)
         del marks[rename]
-        bookmarks.write(repo)
+        marks.write()
 
     elif mark is not None:
         mark = checkformat(mark)
@@ -848,7 +848,7 @@
             marks[mark] = cur
         if not inactive and cur == marks[mark]:
             bookmarks.setcurrent(repo, mark)
-        bookmarks.write(repo)
+        marks.write()
 
     # Same message whether trying to deactivate the current bookmark (-i
     # with no NAME) or listing bookmarks
@@ -1321,11 +1321,12 @@
         elif marks:
             ui.debug('moving bookmarks %r from %s to %s\n' %
                      (marks, old.hex(), hex(node)))
+            newmarks = repo._bookmarks
             for bm in marks:
-                repo._bookmarks[bm] = node
+                newmarks[bm] = node
                 if bm == current:
                     bookmarks.setcurrent(repo, bm)
-            bookmarks.write(repo)
+            newmarks.write()
     else:
         e = cmdutil.commiteditor
         if opts.get('force_editor'):
@@ -4673,11 +4674,12 @@
 
     # update specified bookmarks
     if opts.get('bookmark'):
+        marks = repo._bookmarks
         for b in opts['bookmark']:
             # explicit pull overrides local bookmark if any
             ui.status(_("importing bookmark %s\n") % b)
-            repo._bookmarks[b] = repo[rb[b]].node()
-        bookmarks.write(repo)
+            marks[b] = repo[rb[b]].node()
+        marks.write()
 
     return ret
 
--- a/mercurial/hg.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/mercurial/hg.py	Wed Nov 07 16:21:39 2012 -0600
@@ -391,14 +391,15 @@
         destrepo = destpeer.local()
         if destrepo and srcpeer.capable("pushkey"):
             rb = srcpeer.listkeys('bookmarks')
+            marks = destrepo._bookmarks
             for k, n in rb.iteritems():
                 try:
                     m = destrepo.lookup(n)
-                    destrepo._bookmarks[k] = m
+                    marks[k] = m
                 except error.RepoLookupError:
                     pass
             if rb:
-                bookmarks.write(destrepo)
+                marks.write()
         elif srcrepo and destpeer.capable("pushkey"):
             for k, n in srcrepo._bookmarks.iteritems():
                 destpeer.pushkey('bookmarks', k, '', hex(n))
--- a/mercurial/localrepo.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/mercurial/localrepo.py	Wed Nov 07 16:21:39 2012 -0600
@@ -265,15 +265,12 @@
 
     @filecache('bookmarks')
     def _bookmarks(self):
-        return bookmarks.read(self)
+        return bookmarks.bmstore(self)
 
     @filecache('bookmarks.current')
     def _bookmarkcurrent(self):
         return bookmarks.readcurrent(self)
 
-    def _writebookmarks(self, marks):
-        bookmarks.write(self)
-
     def bookmarkheads(self, bookmark):
         name = bookmark.split('@', 1)[0]
         heads = []
--- a/mercurial/repair.py	Fri Nov 09 14:49:30 2012 -0800
+++ b/mercurial/repair.py	Wed Nov 07 16:21:39 2012 -0600
@@ -6,7 +6,7 @@
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
-from mercurial import changegroup, bookmarks
+from mercurial import changegroup
 from mercurial.node import short
 from mercurial.i18n import _
 import os
@@ -181,7 +181,7 @@
 
         for m in updatebm:
             bm[m] = repo[newbmtarget].node()
-        bookmarks.write(repo)
+        bm.write()
     except: # re-raises
         if backupfile:
             ui.warn(_("strip failed, full bundle stored in '%s'\n")