[PATCH 1 of 5 STABLE] filecache: pass func name that constructs the full path of the given file

[Idan Kamara]

On Wed, Feb 29, 2012 at 12:03 AM, Matt Mackall <mpm at selenic.com> wrote:

> On Tue, 2012-02-28 at 17:53 +0200, Idan Kamara wrote:
> > # HG changeset patch
> > # User Idan Kamara <idankk86 at gmail.com>
> > # Date 1330444187 -7200
> > # Branch stable
> > # Node ID 94da74854f8123af5f81c1a996352fdaeffb23d5
> > # Parent  de7aec5079bdaaae8509ec74a57fc2cb2c160371
> > filecache: pass func name that constructs the full path of the given file
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -176,7 +176,7 @@
> >      def _writebookmarks(self, marks):
> >        bookmarks.write(self)
> >
> > -    @filecache('phaseroots', True)
> > +    @filecache('phaseroots', 'sjoin')
>
> Err.. yuck. We're going to pass a string and then look it up with
> getattr?
>
> Ok, so now I'm at "yuck", but I have no idea what the motivation for
> this is without reading all the patches. But all the patches have
> one-line descriptions so I have no idea what the underlying bug is and
> why this is the right fix. So now I'm at "yuck" plus "huh?".
>

Yes, perhaps I should have included a summary for this
series.


>
> My take: the deeper problem here is that the filecache scheme is
> incestuous with the repo object, which makes it hard to repurpose for
> use with dirstate. So it seems the right fix is to have a base class
> (generic file caching) and derived classes (current class + one to make
> dirstate happy).


Hm, that sounds like overkill to the problem this was meant to fix.

Currently filecache is coupled with localrepo because it assumes
the files it monitors are either in .hg or .hg/store, this is controlled
by the instore flag which tells it which of (s)join methods to use when
figuring out the path of the monitored file at runtime.

So now dirstate comes along and that doesn't quite fit because it
uses different names and it also caches files in the repository root.

But all we really care about is how to compute the final path, so
passing the function seems like the right thing to do, rather than two
classes (or more when filecache is needed in another class) with hardcoded
function calls.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120229/3445c30d/attachment.html>