[PATCH v2] shelve: add a shelve extension to save/restore working changes

[Pierre-Yves David]

On 30 juin 2013, at 00:09, Kevin Bullock wrote:

> On 27 Jun 2013, at 5:54 PM, Bryan O'Sullivan wrote:
> 
>> On Thu, Jun 13, 2013 at 6:06 AM, Pierre-Yves David <pierre-yves.david at logilab.fr> wrote:
>>> This is not supposed to happen. The repoview object should just proxy
>>> everything to the original localrepo object.
>> 
>> Every access to repo.mq results in a new mq.queue object being created and destroyed. I don't know quite why this is happening, but I can see it clearly.
>> 
>>> I'm not saying that repoview can't be bugged. But it should not prevent you
>>> from doing what you are trying to.
>> 
>> Since I can't maintain any state on the repo object (because mq is deleted and recreated every time I access it), it is very definitely preventing me from doing what I am trying to do :-)
>> 
>> Here is what I can reconstruct so far.
>> 
>> When I ask for repo.mq, this ends up invoking propertycache.__get__ on a localrepo.proxycls object.
>> 
>> That tries to setattr the attribute, which (via inheritance) calls repoview.repoview.__setattr__. This seems to set the field on the mq.mqrepo object correctly, but next time the attribute is retrieved, the propertycache.__get__ method is called again, a new object is created as a result, and the previous one is deleted once the attribute is set again.
> 
> This is the crux of the issue: repo is a filtered repo object (a repoview). When you ask for repo.mq, Python sees that the mq property isn't set on repo. So it calls through dunder-and-decorator-land down to repo.unfiltered().mq() (the _method_ in mqrepo). Then propertycache assigns the result to the 'mq' attribute on the _unfiltered_ repo.
> 
> Then the next time you ask for repo.mq, Python sees that the mq property isn't set on repo. So it calls down through __getattr__ and propertycache... and of course ends up creating a new mq.queue, assigning it to repo.unfiltered().mq, and throwing away the old one.
> 
> So that's the bizarreness. Not quite sure yet how to fix it, but we clearly need to. Perhaps we could set up a time to talk it through via IRC or mumble?
> 
> This is exactly the sort of weirdness I feared when we introduced repoview. Le sigh.

(just spoted that I replied to Kevin only in my last few email :-( reposting )

The propertycache code looks like this

class propertycache(object):
   def __init__(self, func):
       self.func = func
       self.name = func.__name__
   def __get__(self, obj, type=None):
       result = self.func(obj)
       self.cachevalue(obj, result)
       return result

   def cachevalue(self, obj, value):
       setattr(obj, self.name, value)

Replacing the ``setattr(obj, self.name, value)`` call with ``obj.__dict__[self.name] = value`` would bypass repoview magic.

I'm actually a bit surprise that propertycache on repoview are so broken for two versions. As far as I understand the current code:
(1) All propertycache computed repoview side are actually store on the unfiltered repo. That would expect the unfiltered repo be confused.
(2) All repoview seems to not use the cached value anymore. I would have expected some performance impact there.


To go back to our main topic, the mq issue, in ``mercurial.localrepo`` there is an ``unfilteredpropertycache`` decorator that ensure a property cache is called at unfiltered repo level. It should be used by property cache on repo that does not care about filtering and could/should be shared between all repoview instance. I think the ``mq`` property fall in this category.

-- 
Pierre-Yves