[PATCH] move tags.cache and branchheads.cache to a collected cache folder .hg/caches/

[Adrian Buehlmann]

On 2010-12-26 13:13, Jason Harris wrote:
> 
> On Dec 25, 2010, at 11:46 AM, Adrian Buehlmann wrote:
> 
>> On 2010-12-24 23:22, Jason Harris wrote:
>>> # HG changeset patch
>>> # User jfh <jason at jasonfharris.com>
>>> # Date 1293195683 -3600
>>> # Node ID 53a4b25e7165c3842c8cd9e376a4240738070aab
>>> # Parent  5314cbb775f6e2034be96bc64a54a0ec7c553a19
>>> move tags.cache and branchheads.cache to a collected cache folder .hg/caches/
>>>
>>> The generation of cache files like tags.cache and branchheads.cache is not an
>>> actual reflection of things changing in the whole of the .hg directory (like eg
>>> a commit or a rebase or something) but instead these cache files are just part
>>> of bookkeeping. As such its convienant to allow various clients to ignore file
>>> events to do with these cache files which would otherwise cause a double
>>> refresh. Eg one refresh might occur after a commit, but the act of refreshing
>>> after the commit would cause Mercurial to generate a new branchheads.cache which
>>> would then cause a second refresh, for clients.
>>>
>>> However if these cache files are moved into a directory like eg .hg/caches/ then
>>> GUI clients on OSX (and possibly other platforms) can happily ignore file events
>>> in this cache directory.
>>
>> Sounds reasonable.
> 
> Cool...
> 
> 
>> Perhaps you can implement it a bit differently though. See inline
>> comment below
>>
>>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>>> --- a/mercurial/localrepo.py
>>> +++ b/mercurial/localrepo.py
>>> @@ -439,7 +439,7 @@
>>>     def _readbranchcache(self):
>>>         partial = {}
>>>         try:
>>> -            f = self.opener("branchheads.cache")
>>> +            f = self.opener(os.path.join("caches","branchheads.cache"))
>>
>> What about setting up an new cache opener ('copener') and using it like this
>>
>>               f = self.copener("branchheads")
>>
>> I envision having the cacheopener's base directory in '.hg/cache'. Also,
>> we can drop the '.cache' suffix on the files in there while we're at it.
> 
> What about the line in:
> 
> http://mercurial.selenic.com/wiki/CodingStyle?action=show&redirect=BasicCodingStyle
> 
> which says:
> Think twice about using classes, functions are almost always smaller and simpler.

Absolutely right, but doesn't apply here.

> Thus, do we really want a class here copener which can be confused with opener (its one letter different), when the other code as I have it inlined is easily readable, just as simple to maintain as the current code, and totally obvious as to what its doing?  Admittedly if there were lots of cache files running around one might want such a class, but currently there are only two such files right?
> 
> If Matt and other crew members say they want a class here, I'll put it in, but to me for the moment it would seem like needless abstraction... Still I am not a guru in the Mercurial code base... so I could well be wrong here. But thanks for the comments...

I think an instance of the existing util.opener class should work just fine.

Compare with self.wopener in localrepo.py (quote):

    def __init__(self, baseui, path=None, create=0):
        repo.repository.__init__(self)
        self.root = os.path.realpath(util.expandpath(path))
        self.path = os.path.join(self.root, ".hg")
        self.origroot = path
        self.auditor = util.path_auditor(self.root, self._checknested)
        self.opener = util.opener(self.path)
        self.wopener = util.opener(self.root)           <---- example
        self.baseui = baseui
        self.ui = baseui.copy()

For the name: localrepo already has self.wopener and self.sopener. I
think copener could be a third, new member.

But I for one would be fine with self.cacheopener as well (I think Matt
prefers shorter names in general).