Patchwork localrepo: get value from the unfiltered caches should check if the attribute existed

login
register
mail settings
Submitter elson.wei@gmail.com
Date Aug. 6, 2013, 3:45 a.m.
Message ID <06e23972c0d2d9420e96.1375760744@ElsonWei-NB.PrimeVOLT>
Download mbox | patch
Permalink /patch/2004/
State Superseded
Headers show

Comments

elson.wei@gmail.com - Aug. 6, 2013, 3:45 a.m.
# HG changeset patch
# User Wei, Elson <elson.wei@gmail.com>
# Date 1375760701 -28800
#      Tue Aug 06 11:45:01 2013 +0800
# Node ID 06e23972c0d2d9420e96dd657db1cc2d30b7fc11
# Parent  a58251c0568fc5e86089a803ca75f75cc8c01678
localrepo: get value from the unfiltered caches should check if the attribute existed.

If the attribute existed already, it should be returned by getattr().
Otherwise, it will be evaluated again.
Matt Mackall - Aug. 14, 2013, 6:30 p.m.
On Tue, 2013-08-06 at 11:45 +0800, elson.wei@gmail.com wrote:
> # HG changeset patch
> # User Wei, Elson <elson.wei@gmail.com>
> # Date 1375760701 -28800
> #      Tue Aug 06 11:45:01 2013 +0800
> # Node ID 06e23972c0d2d9420e96dd657db1cc2d30b7fc11
> # Parent  a58251c0568fc5e86089a803ca75f75cc8c01678
> localrepo: get value from the unfiltered caches should check if the attribute existed.
> 
> If the attribute existed already, it should be returned by getattr().
> Otherwise, it will be evaluated again.

So.. this sounds important. Can you explain how/where this causes a
problem?

> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -39,6 +39,8 @@
>      """propertycache that apply to unfiltered repo only"""
>  
>      def __get__(self, repo, type=None):
> +        if hasunfilteredcache(repo, self.name):
> +            return getattr(repo.unfiltered(), self.name)
>          return super(unfilteredpropertycache, self).__get__(repo.unfiltered())
>  
>  class filteredpropertycache(propertycache):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Aug. 15, 2013, 2:21 a.m.
[cc:ed back to the list and de-top-posted]

> 2013/8/15 Matt Mackall <mpm@selenic.com>
> 
> > On Tue, 2013-08-06 at 11:45 +0800, elson.wei@gmail.com wrote:
> > > # HG changeset patch
> > > # User Wei, Elson <elson.wei@gmail.com>
> > > # Date 1375760701 -28800
> > > #      Tue Aug 06 11:45:01 2013 +0800
> > > # Node ID 06e23972c0d2d9420e96dd657db1cc2d30b7fc11
> > > # Parent  a58251c0568fc5e86089a803ca75f75cc8c01678
> > > localrepo: get value from the unfiltered caches should check if the
> > attribute existed.
> > >
> > > If the attribute existed already, it should be returned by getattr().
> > > Otherwise, it will be evaluated again.
> >
> > So.. this sounds important. Can you explain how/where this causes a
> > problem?

On Thu, 2013-08-15 at 09:43 +0800, elson wrote:
> It might impact on performance only.
> But, if a function is evaluated every time. Why is it marked as a
> propertycache?

This is not the sort of answer I was hoping for.

I was hoping you'd say "when someone runs 'hg foo', Mercurial does
stupid thing X because of Y."

Then I can say "yes, that sounds like a serious bug and now I know
enough about what the patch is trying to do to review it and test it".

Otherwise you have to wait for the nonexistant person who is an expert
in both the caching system and the repo filtering to show up to
understand your patch.

Patch

# HG changeset patch
# User Wei, Elson <elson.wei@gmail.com>
# Date 1375760701 -28800
#      Tue Aug 06 11:45:01 2013 +0800
# Node ID 06e23972c0d2d9420e96dd657db1cc2d30b7fc11
# Parent  a58251c0568fc5e86089a803ca75f75cc8c01678
localrepo: get value from the unfiltered caches should check if the attribute existed.

If the attribute existed already, it should be returned by getattr().
Otherwise, it will be evaluated again.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -39,6 +39,8 @@ 
     """propertycache that apply to unfiltered repo only"""
 
     def __get__(self, repo, type=None):
+        if hasunfilteredcache(repo, self.name):
+            return getattr(repo.unfiltered(), self.name)
         return super(unfilteredpropertycache, self).__get__(repo.unfiltered())
 
 class filteredpropertycache(propertycache):