# HG changeset patch # User Augie Fackler # Date 1447294682 18000 # Node ID dad6404ccddb236ef0f1b88c741eec71bf6a5563 # Parent 0ce0cfee497f07472f64d6f0a4f2152a264fafa2 bmstore: add handling of the active bookmark This further centralizes the handling of bookmark storage, and will help get some lingering bookmarks business out of localrepo. Right now, this change implies reading of the active bookmark to also imply reading all bookmarks from disk - for users with many many bookmarks this may be a measurable performance hit. In that case, we should migrate bmstore to be able to lazy-read its properties from disk rather than having to eagerly read them, but I decided to avoid doing that to try and avoid some potentially complicated filecache decorator issues. This doesn't move the logic for writing the active bookmark into a transaction, though that is probably the correct next step. Since the API probably needs to morph a little more, I didn't bother marking bookmarks.{activate,deactivate} as deprecated yet. diff -r 0ce0cfee497f -r dad6404ccddb mercurial/bookmarks.py --- a/mercurial/bookmarks.py Thu Jan 07 20:02:47 2016 -0800 +++ b/mercurial/bookmarks.py Wed Nov 11 21:18:02 2015 -0500 @@ -44,16 +44,15 @@ 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 object should do all bookmark-related reads and writes, so + that it's fairly simple to replace the storage underlying + bookmarks without having to clone the logic surrounding + bookmarks. This type also should manage the active bookmark, if + any. 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 "active" bookmark state at this - time. """ def __init__(self, repo): @@ -79,6 +78,20 @@ if inst.errno != errno.ENOENT: raise self._clean = True + self._active = _readactive(repo, self) + self._aclean = True + + @property + def active(self): + return self._active + + @active.setter + def active(self, mark): + if mark is not None and mark not in self: + raise AssertionError('bookmark %s does not exist!' % mark) + + self._active = mark + self._aclean = False def __setitem__(self, *args, **kwargs): self._clean = False @@ -107,6 +120,9 @@ ''' msg = 'bm.write() is deprecated, use bm.recordchange(transaction)' self._repo.ui.deprecwarn(msg, '3.7') + # TODO: writing the active bookmark should probably also use a + # transaction. + self._writeactive() if self._clean: return repo = self._repo @@ -128,8 +144,10 @@ def _writerepo(self, repo): """Factored out for extensibility""" - if repo._activebookmark not in self: - deactivate(repo) + rbm = repo._bookmarks + if rbm.active not in self: + rbm.active = None + rbm._writeactive() wlock = repo.wlock() try: @@ -146,12 +164,33 @@ finally: wlock.release() + def _writeactive(self): + if self._aclean: + return + wlock = self._repo.wlock() + try: + if self._active is not None: + f = self._repo.vfs('bookmarks.current', 'w', atomictemp=True) + try: + f.write(encoding.fromlocal(self._active)) + finally: + f.close() + else: + try: + self._repo.vfs.unlink('bookmarks.current') + except OSError as inst: + if inst.errno != errno.ENOENT: + raise + finally: + wlock.release() + self._aclean = True + def _write(self, fp): for name, node in self.iteritems(): fp.write("%s %s\n" % (hex(node), encoding.fromlocal(name))) self._clean = True -def readactive(repo): +def _readactive(repo, marks): """ Get the active bookmark. We can have an active bookmark that updates itself as we commit. This function returns the name of that bookmark. @@ -172,7 +211,7 @@ # static-http which only tries to load the file when we try # to read from it. mark = encoding.tolocal((file.readlines() or [''])[0]) - if mark == '' or mark not in repo._bookmarks: + if mark == '' or mark not in marks: mark = None except IOError as inst: if inst.errno != errno.ENOENT: @@ -188,35 +227,15 @@ follow new commits that are made. The name is recorded in .hg/bookmarks.current """ - if mark not in repo._bookmarks: - raise AssertionError('bookmark %s does not exist!' % mark) - - active = repo._activebookmark - if active == mark: - return - - wlock = repo.wlock() - try: - file = repo.vfs('bookmarks.current', 'w', atomictemp=True) - file.write(encoding.fromlocal(mark)) - file.close() - finally: - wlock.release() - repo._activebookmark = mark + repo._bookmarks.active = mark + repo._bookmarks._writeactive() def deactivate(repo): """ Unset the active bookmark in this repository. """ - wlock = repo.wlock() - try: - repo.vfs.unlink('bookmarks.current') - repo._activebookmark = None - except OSError as inst: - if inst.errno != errno.ENOENT: - raise - finally: - wlock.release() + repo._bookmarks.active = None + repo._bookmarks._writeactive() def isactivewdirparent(repo): """ @@ -266,7 +285,7 @@ deletefrom = parents marks = repo._bookmarks update = False - active = repo._activebookmark + active = marks.active if not active: return False diff -r 0ce0cfee497f -r dad6404ccddb mercurial/cmdutil.py --- a/mercurial/cmdutil.py Thu Jan 07 20:02:47 2016 -0800 +++ b/mercurial/cmdutil.py Wed Nov 11 21:18:02 2015 -0500 @@ -2530,13 +2530,14 @@ # First, do a regular commit to record all changes in the working # directory (if there are any) ui.callhooks = False - activebookmark = repo._activebookmark + activebookmark = repo._bookmarks.active try: - repo._activebookmark = None + repo._bookmarks.active = None opts['message'] = 'temporary amend commit for %s' % old node = commit(ui, repo, commitfunc, pats, opts) finally: - repo._activebookmark = activebookmark + repo._bookmarks.active = activebookmark + repo._bookmarks.recordchange(tr) ui.callhooks = True ctx = repo[node] diff -r 0ce0cfee497f -r dad6404ccddb mercurial/localrepo.py --- a/mercurial/localrepo.py Thu Jan 07 20:02:47 2016 -0800 +++ b/mercurial/localrepo.py Wed Nov 11 21:18:02 2015 -0500 @@ -459,13 +459,13 @@ pass return proxycls(self, name) - @repofilecache('bookmarks') + @repofilecache('bookmarks', 'bookmarks.current') def _bookmarks(self): return bookmarks.bmstore(self) - @repofilecache('bookmarks.current') + @property def _activebookmark(self): - return bookmarks.readactive(self) + return self._bookmarks.active def bookmarkheads(self, bookmark): name = bookmark.split('@', 1)[0]