[PATCH 1 of 7] localrepo: factor _findtags() out of tags() (affects mq and bookmarks)

[Greg Ward]

# HG changeset patch
# User Greg Ward <greg at gerg.ca>
# Date 1246632310 14400
# Node ID bc27cc5162749383dbf6852aeb2d8b426e1bf6fd
# Parent  f46265d17271dc20e5547d26d86de7ac3975dc6c
localrepo: factor _findtags() out of tags() (affects mq and bookmarks).

This means caching is mainly the responsibility of localrepo, reducing
how much mqrepo and bookmarks have to know about the innards of
localrepo.

The bad news (currently) is that _findtags() is confused about whether
it should have a side effect or return a value.  If you only look at
localrepo, this looks easy to resolve... but it's less clear when you
bring mqrepo and bookmarkrepo into the picture.

diff --git a/hgext/bookmarks.py b/hgext/bookmarks.py
--- a/hgext/bookmarks.py
+++ b/hgext/bookmarks.py
@@ -296,12 +296,9 @@
                 write(self, marks)
             return result
 
-        def tags(self):
+        def _findtags(self):
             """Merge bookmarks with normal tags"""
-            if self.tagscache:
-                return self.tagscache
-
-            tagscache = super(bookmark_repo, self).tags()
+            tagscache = super(bookmark_repo, self)._findtags()
             tagscache.update(parse(self))
             return tagscache
 
diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -2427,11 +2427,9 @@
                 raise util.Abort(_('source has mq patches applied'))
             return super(mqrepo, self).push(remote, force, revs)
 
-        def tags(self):
-            if self.tagscache:
-                return self.tagscache
-
-            tagscache = super(mqrepo, self).tags()
+        def _findtags(self):
+            '''augment tags from base class with patch tags'''
+            tagscache = super(mqrepo, self)._findtags()
 
             q = self.mq
             if not q.applied:
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -235,6 +235,17 @@
         '''return a mapping of tag to node'''
         if self.tagscache:
             return self.tagscache
+        else:
+            return self._findtags()
+
+    def _findtags(self):
+        '''do the hard work of finding tags and populating self.tagscache'''
+
+        # XXX bogus: this method has both side effect (update
+        # self.tagscache) and return value (self.tagscache itself).
+        # Worse, tags() uses it for side effect, but mqrepo and
+        # bookmarkrepo both use it for its return value.  Make up your
+        # mind: return value or side effect -- not both!
 
         globaltags = {}
         tagtypes = {}