[PATCH 1 of 8 RFC] vfs: replace invocation of file APIs of os module by ones via vfs

[Matt Mackall]

On Sat, 2012-06-16 at 03:00 +0900, FUJIWARA Katsunori wrote:
> At Fri, 15 Jun 2012 10:31:45 -0500,
> Matt Mackall wrote:
> > 
> > On Fri, 2012-06-15 at 23:45 +0900, FUJIWARA Katsunori wrote:
> > > # HG changeset patch
> > > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > > # Date 1339768793 -32400
> > > # Node ID a14b63be9a04e7fac445fea69bbaf840ca3f4063
> > > # Parent  622aa57a90b1d1f09b3204458b087de12ce2de82
> > > vfs: replace invocation of file APIs of os module by ones via vfs
> > 
> > You seem to have missed the importance of step 1:
> > 
> > "Rename opener to vfs"
> > http://mercurial.selenic.com/wiki/WindowsUTF8Plan#Steps
> > 
> > The whole point of this exercise is to have one (or just a few) central
> > objects we route all our file operations through that are attached to
> > repository objects so that repository objects can easily switch their
> > modes as needed. Conveniently, we have something very much like that
> > already: it's called an opener. It's even beginning to grow some of
> > these sorts of methods.
> > 
> > In particular, we'll want to take one of these objects, wopener, and
> > switch it to UTF-8 mode.. while leaving the other two in native mode.
> > Which means we need to be calling methods at a high enough level that we
> > know which part of the repository we're operating on...
> 
> Sorry, I mis-understand, because I also think about "create a
> filesystem abstraction object in util.py" in "Abstracting filesystem
> API for UTF-8 support on Windows" which you posted to devel-ml.
> 
> # http://www.selenic.com/pipermail/mercurial-devel/2011-December/036385.html
> 
> > >     1. replace all invocations of above functions "os.XXXX()" by
> > >         "util.vfs().XXXX()" mechanically: or adding "vfs()." in
> > >         util.py itself
> > 
> > ...thus creating vfs objects on the fly with util.vfs() doesn't get us
> > any closer to having centralized control on them.  
> > 
> > Also, this patch is quite big. It took me a while to locate the only
> > real interesting bit: the definition of vfs. I'd rather see:
> > 
> > 1. rename the object
> > 2. add a couple methods
> > 3. convert some users
> > 4. add some methods
> > 5. convert some users
> > ...
> 
> I understand that both "add a couple methods" and "convert some users"
> mean "don't replace whole of code at once", am I right ?

It means I want to see the "small, interesting implementation bits"
separated from the "large, boring, trivial conversions".

> Would you like the steps below ?
> 
>     1. pick some code path up
>     2. check direct file API invocations in the path
>     3. add just "a couple of" methods to current "opener" to invoke
>        such APIs, if needed
>     4. make codes in the path use methods of "opener"
>     5. repeat from (3) to (4) for the path
>     6. repeat from (1) to (5) for all paths
>     5. create patch for each pairs of (3) and (4)

Please do a small amount of initial work, stop, and send a few patches.
Much smaller than the current batch.

> 
> BTW:
> 
> > "Rename opener to vfs"
> 
> Should I rename both name of class and referring variables of "opener"
> to "vfs"?  or only name of class ?

Well, we'll probably want to do both eventually. I think since we'll be
adding a bunch of new callers to repo.<foo>.<someop>, we want to do
rename the variables first, so we don't have to go back and change even
more things later.

-- 
Mathematics is the supreme nostalgia of our time.