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

Matt Mackall mpm at selenic.com
Tue Feb 28 16:03:19 CST 2012


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?".

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).

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list