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

[Jason Harris]

On Jan 2, 2011, at 11:14 AM, Adrian Buehlmann wrote:

> On 2011-01-02 06:03, Jason Harris wrote:
>> # HG changeset patch
>> # User jfh <jason at jasonfharris.com>
>> # Date 1293195683 -3600
>> # Node ID 4d97c3e51f4f05f4f93d646b2d0ac16120f82892
>> # 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 convenient 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.
>> 
>> 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"))
> 
> There's a space missing after the comma. See contrib/check-code.py for
> the rules.
> 
>>             lines = f.read().split('\n')
>>             f.close()
>>         except (IOError, OSError):
>> @@ -467,7 +467,7 @@
>> 
>>     def _writebranchcache(self, branches, tip, tiprev):
>>         try:
>> -            f = self.opener("branchheads.cache", "w", atomictemp=True)
>> +            f = self.opener(os.path.join("caches","branchheads.cache"), "w", atomictemp=True)
>>             f.write("%s %s\n" % (hex(tip), tiprev))
>>             for label, nodes in branches.iteritems():
>>                 for node in nodes:
>> diff --git a/mercurial/tags.py b/mercurial/tags.py
>> --- a/mercurial/tags.py
>> +++ b/mercurial/tags.py
>> @@ -12,6 +12,7 @@
>> 
>> from node import nullid, bin, hex, short
>> from i18n import _
>> +import os.path
>> import encoding
>> import error
>> 
>> @@ -154,7 +155,7 @@
>>     set, caller is responsible for reading tag info from each head.'''
>> 
>>     try:
>> -        cachefile = repo.opener('tags.cache', 'r')
>> +        cachefile = repo.opener(os.path.join('caches','tags.cache'), 'r')
>>         # force reading the file for static-http
>>         cachelines = iter(cachefile)
>>     except IOError:
>> @@ -189,7 +190,7 @@
>>                     cachefnode[headnode] = fnode
>>         except (ValueError, TypeError):
>>             # corruption of tags.cache, just recompute it
>> -            ui.warn(_('.hg/tags.cache is corrupt, rebuilding it\n'))
>> +            ui.warn(_('.hg/caches/tags.cache is corrupt, rebuilding it\n'))
>>             cacheheads = []
>>             cacherevs = []
>>             cachefnode = {}
>> @@ -251,7 +252,7 @@
>> def _writetagcache(ui, repo, heads, tagfnode, cachetags):
>> 
>>     try:
>> -        cachefile = repo.opener('tags.cache', 'w', atomictemp=True)
>> +        cachefile = repo.opener(os.path.join('caches','tags.cache'), 'w', atomictemp=True)
>>     except (OSError, IOError):
>>         return
> 
> There's really not much point duplicating
> 
>   os.path.join("caches", ...)
> 
> again and again.
> 
> I showed you a way how to avoid that (copener).
> 
> It's very unlikely that this patch will be accepted in this form.


Having the patches split into 3 parts its pretty clear whats going on, and this way Matt or the crew member can simply take the patches I and II if they choose, or take all 3 parts if they want and call hg collapse on them etc... Or I could submit V3 of the patch with the changes collapsed...

The patches have three parts to it:
The first part fixes the actual problem.
The second updates the test suite.
The third is the refactor you have asked for which I don't actually agree with for the reasons I give in the intro.

(ie copener will always have to be synchronized with opener. Sooner or later that synchronization will be busted somewhere, and it will be hard to track down. The fewer code synchronization issues one introduces the better... If you use the inline approach four calls in the whole sources to os.path.join is really very low impact, also it happens lazily, Ie we don't pay for these functions / calls unless we use them... etc, etc.) But frankly, I don't really much care.

But can someone who has the power to push these changes weigh in here and say which form they actually want the patch in?  Ie do they want the copener approach, or the inline approach.

Thanks,
  Jas