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

[Jason Harris]

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

> On 2011-01-02 06:03, Jason Harris wrote:
>> # HG changeset patch
>> # User jfh <jason at jasonfharris.com>
> 
> I suggest using the canonical form
> 
>   'Jason Harris <....>'
> 
>> # Date 1293942436 -3600
>> # Node ID 41fd8c59cced164599f02011c5c7505546af0d4b
>> # Parent  d47017e900275807a63b8f653f714c43593d959f
>> move tags.cache and branchheads.cache to a collected cache folder .hg/caches/
>> 
> [snip]
>> 
>> Part III: Refactor requested by Adrian Buehlmann.
>> 
>> Adrian asked that these changes to move the caches into their own directory, be
>> wrapped up in a new copener.
> 
> Right, but there is no need to have a refactoring in your own patches at
> all. Do it right from the start.

The point is, if it was up to me I wouldn't actually include part III (ie the refactor), I would just include part I and II. This way I am letting Matt & co decide if they want all three parts including your suggested refactor or not. Once you go the refactor route you have the future synchronization problems as I pointed out, and it seems to me its a bit of abstraction for its own sake... but YMMV. Thats why I kept the patches separate.

> BTW: try to be a bit less wordy. I think not that many people are going
> to read through your intro of this series, due to its sheer length.
> 
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -33,6 +33,7 @@
>>         self.auditor = util.path_auditor(self.root, self._checknested)
>>         self.opener = util.opener(self.path)
>>         self.wopener = util.opener(self.root)
>> +        self.copener = util.opener(os.path.join(self.path, "caches"))
> 
> Minor point: My proposal was to use the name 'cache' for the directory.
> 
> But 'caches' will probably work too.

Yep, I liked caches better.

>>         self.baseui = baseui
>>         self.ui = baseui.copy()
>> 
>> @@ -93,6 +94,7 @@
>>         self.sopener = self.store.opener
>>         self.sjoin = self.store.join
>>         self.opener.createmode = self.store.createmode
>> +        self.copener.createmode = self.store.createmode
>>         self._applyrequirements(requirements)
>>         if create:
>>             self._writerequirements()
>> @@ -439,7 +441,7 @@
>>     def _readbranchcache(self):
>>         partial = {}
>>         try:
>> -            f = self.opener(os.path.join("caches","branchheads.cache"))
>> +            f = self.copener("branchheads.cache")
> 
> My proposal was to drop the '.cache' suffix on the filenames. Having a
> filename
> 
>  '.hg/caches/branchheads.cache'
> 
> seems redundant to me. There's no need to have the term 'cache' twice in
> the path.

I kind of preferred the suffix to make it clear.

>>             lines = f.read().split('\n')
>>             f.close()
>>         except (IOError, OSError):
>> @@ -467,7 +469,7 @@
>> 
>>     def _writebranchcache(self, branches, tip, tiprev):
>>         try:
>> -            f = self.opener(os.path.join("caches","branchheads.cache"), "w", atomictemp=True)
>> +            f = self.copener("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/statichttprepo.py b/mercurial/statichttprepo.py
>> --- a/mercurial/statichttprepo.py
>> +++ b/mercurial/statichttprepo.py
>> @@ -10,6 +10,7 @@
>> from i18n import _
>> import changelog, byterange, url, error
>> import localrepo, manifest, util, store
>> +import os.path
> 
> Unneeded.

Whoops... I had taken that out, and actually in my sources I look at now its not there...

I will wait until some others weigh in to say if they want the refactor in from the start or not... Still all this to-ing and fro-ing seems a little redundant. Its just easier to take part I & II and then someone else change them they way they want to...

>> import urllib, urllib2, errno
>> 
>> class httprangereader(object):
>> @@ -90,6 +91,7 @@
>> 
>>         opener = build_opener(ui, authinfo)
>>         self.opener = opener(self.path)
>> +        self.copener = opener(self.path + "/caches")
>> 
>>         # find requirements
>>         try:
>> diff --git a/mercurial/tags.py b/mercurial/tags.py
>> --- a/mercurial/tags.py
>> +++ b/mercurial/tags.py
>> @@ -12,7 +12,6 @@
>> 
>> from node import nullid, bin, hex, short
>> from i18n import _
>> -import os.path
>> import encoding
>> import error
>> 
>> @@ -155,7 +154,7 @@
>>     set, caller is responsible for reading tag info from each head.'''
>> 
>>     try:
>> -        cachefile = repo.opener(os.path.join('caches','tags.cache'), 'r')
>> +        cachefile = repo.copener('tags.cache', 'r')
>>         # force reading the file for static-http
>>         cachelines = iter(cachefile)
>>     except IOError:
>> @@ -252,7 +251,7 @@
>> def _writetagcache(ui, repo, heads, tagfnode, cachetags):
>> 
>>     try:
>> -        cachefile = repo.opener(os.path.join('caches','tags.cache'), 'w', atomictemp=True)
>> +        cachefile = repo.copener('tags.cache', 'w', atomictemp=True)
>>     except (OSError, IOError):
>>         return
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel