Patchwork subrepo: convert the os.path references in git to vfs

login
register
mail settings
Submitter Matt Harbison
Date April 10, 2015, 1:21 a.m.
Message ID <7b26d433ab78988a19ca.1428628871@Envy>
Download mbox | patch
Permalink /patch/8585/
State Accepted
Commit 419528cb05b6176c8685f1dba0de73aa78c833b0
Headers show

Comments

Matt Harbison - April 10, 2015, 1:21 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1423331860 18000
#      Sat Feb 07 12:57:40 2015 -0500
# Node ID 7b26d433ab78988a19cadcead645196999b59568
# Parent  e0e28e910fa3797fd0aa4f818e9b33c5bcbf0e53
subrepo: convert the os.path references in git to vfs

There are a handful of os.path references in the free functions at the top of
the module that will be trickier to remove.
Mathias De Maré - April 10, 2015, 5:57 a.m.
On Fri, Apr 10, 2015 at 3:21 AM, Matt Harbison <mharbison72@gmail.com>
wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1423331860 18000
> #      Sat Feb 07 12:57:40 2015 -0500
> # Node ID 7b26d433ab78988a19cadcead645196999b59568
> # Parent  e0e28e910fa3797fd0aa4f818e9b33c5bcbf0e53
> subrepo: convert the os.path references in git to vfs
>
> There are a handful of os.path references in the free functions at the top
> of
> the module that will be trickier to remove.
>
These look good to me.
I fear I have added some of the os.path usage myself.

>
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -1300,7 +1300,7 @@
>          return retdata, p.returncode
>
>      def _gitmissing(self):
> -        return not os.path.exists(os.path.join(self._abspath, '.git'))
> +        return not self.wvfs.exists('.git')
>
>      def _gitstate(self):
>          return self._gitcommand(['rev-parse', 'HEAD'])
> @@ -1632,14 +1632,13 @@
>          # local-only history
>          self.ui.note(_('removing subrepo %s\n') % self._relpath)
>          self._gitcommand(['config', 'core.bare', 'true'])
> -        for f in os.listdir(self._abspath):
> +        for f in self.wvfs.listdir():
>              if f == '.git':
>                  continue
> -            path = os.path.join(self._abspath, f)
> -            if os.path.isdir(path) and not os.path.islink(path):
> -                shutil.rmtree(path)
> +            if self.wvfs.isdir(f) and not self.wvfs.islink(f):
> +                shutil.rmtree(self.wvfs.join(f))
>              else:
> -                os.remove(path)
> +                self.wvfs.unlink(f)
>
>      def archive(self, archiver, prefix, match=None):
>          total = 0
> @@ -1815,8 +1814,7 @@
>                  bakname = "%s.orig" % name
>                  self.ui.note(_('saving current version of %s as %s\n') %
>                          (name, bakname))
> -                util.rename(os.path.join(self._abspath, name),
> -                            os.path.join(self._abspath, bakname))
> +                self.wvfs.rename(name, bakname)
>
>          if not opts.get('dry_run'):
>              self.get(substate, overwrite=True)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Katsunori FUJIWARA - April 10, 2015, 9:14 a.m.
At Thu, 09 Apr 2015 21:21:11 -0400,
Matt Harbison wrote:
> 
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1423331860 18000
> #      Sat Feb 07 12:57:40 2015 -0500
> # Node ID 7b26d433ab78988a19cadcead645196999b59568
> # Parent  e0e28e910fa3797fd0aa4f818e9b33c5bcbf0e53
> subrepo: convert the os.path references in git to vfs
> 
> There are a handful of os.path references in the free functions at the top of
> the module that will be trickier to remove.
> 
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -1300,7 +1300,7 @@
>          return retdata, p.returncode
>  
>      def _gitmissing(self):
> -        return not os.path.exists(os.path.join(self._abspath, '.git'))
> +        return not self.wvfs.exists('.git')
>  
>      def _gitstate(self):
>          return self._gitcommand(['rev-parse', 'HEAD'])
> @@ -1632,14 +1632,13 @@
>          # local-only history
>          self.ui.note(_('removing subrepo %s\n') % self._relpath)
>          self._gitcommand(['config', 'core.bare', 'true'])
> -        for f in os.listdir(self._abspath):
> +        for f in self.wvfs.listdir():
>              if f == '.git':
>                  continue
> -            path = os.path.join(self._abspath, f)
> -            if os.path.isdir(path) and not os.path.islink(path):
> -                shutil.rmtree(path)
> +            if self.wvfs.isdir(f) and not self.wvfs.islink(f):

Using "vfs.readdir()" instead of "os.listdir()" can avoid these
(expensive) file type examination.

> +                shutil.rmtree(self.wvfs.join(f))

I'm just working for replacing "shtuil.rmtree()" itself by vfs, and
will post that series in a day (even though this series will conflict
yours, unfortunately)

I have other patches to replace os.*/os.path.*/util.* file API
invocations in subrepo.py, too.

So, let's coordinate our vfs-migration patches, if you have also other
ones for subrepo.py.

>              else:
> -                os.remove(path)
> +                self.wvfs.unlink(f)
>  
>      def archive(self, archiver, prefix, match=None):
>          total = 0
>
> @@ -1815,8 +1814,7 @@
>                  bakname = "%s.orig" % name
>                  self.ui.note(_('saving current version of %s as %s\n') %
>                          (name, bakname))
> -                util.rename(os.path.join(self._abspath, name),
> -                            os.path.join(self._abspath, bakname))
> +                self.wvfs.rename(name, bakname)
>  
>          if not opts.get('dry_run'):
>              self.get(substate, overwrite=True)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Matt Mackall - April 10, 2015, 9:02 p.m.
On Thu, 2015-04-09 at 21:21 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1423331860 18000
> #      Sat Feb 07 12:57:40 2015 -0500
> # Node ID 7b26d433ab78988a19cadcead645196999b59568
> # Parent  e0e28e910fa3797fd0aa4f818e9b33c5bcbf0e53
> subrepo: convert the os.path references in git to vfs

I've queued the two parts of this patch the apply after foozy's, thanks.
Matt Harbison - April 11, 2015, 1:29 a.m.
On Fri, 10 Apr 2015 05:14:37 -0400, FUJIWARA Katsunori  
<foozy@lares.dti.ne.jp> wrote:

> At Thu, 09 Apr 2015 21:21:11 -0400,
> Matt Harbison wrote:
>>
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1423331860 18000
>> #      Sat Feb 07 12:57:40 2015 -0500
>> # Node ID 7b26d433ab78988a19cadcead645196999b59568
>> # Parent  e0e28e910fa3797fd0aa4f818e9b33c5bcbf0e53
>> subrepo: convert the os.path references in git to vfs
>>
>> There are a handful of os.path references in the free functions at the  
>> top of
>> the module that will be trickier to remove.
>>
>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>> --- a/mercurial/subrepo.py
>> +++ b/mercurial/subrepo.py
>> @@ -1300,7 +1300,7 @@
>>          return retdata, p.returncode
>>
>>      def _gitmissing(self):
>> -        return not os.path.exists(os.path.join(self._abspath, '.git'))
>> +        return not self.wvfs.exists('.git')
>>
>>      def _gitstate(self):
>>          return self._gitcommand(['rev-parse', 'HEAD'])
>> @@ -1632,14 +1632,13 @@
>>          # local-only history
>>          self.ui.note(_('removing subrepo %s\n') % self._relpath)
>>          self._gitcommand(['config', 'core.bare', 'true'])
>> -        for f in os.listdir(self._abspath):
>> +        for f in self.wvfs.listdir():
>>              if f == '.git':
>>                  continue
>> -            path = os.path.join(self._abspath, f)
>> -            if os.path.isdir(path) and not os.path.islink(path):
>> -                shutil.rmtree(path)
>> +            if self.wvfs.isdir(f) and not self.wvfs.islink(f):
>
> Using "vfs.readdir()" instead of "os.listdir()" can avoid these
> (expensive) file type examination.
>
>> +                shutil.rmtree(self.wvfs.join(f))
>
> I'm just working for replacing "shtuil.rmtree()" itself by vfs, and
> will post that series in a day (even though this series will conflict
> yours, unfortunately)
>
> I have other patches to replace os.*/os.path.*/util.* file API
> invocations in subrepo.py, too.
>
> So, let's coordinate our vfs-migration patches, if you have also other
> ones for subrepo.py.

This is all I have anywhere for now.  (There's a bunch of work to do in  
largefiles still, but I haven't done it yet.)  Your previous series  
reminded me that I hadn't submitted this.

>>              else:
>> -                os.remove(path)
>> +                self.wvfs.unlink(f)
>>
>>      def archive(self, archiver, prefix, match=None):
>>          total = 0
>>
>> @@ -1815,8 +1814,7 @@
>>                  bakname = "%s.orig" % name
>>                  self.ui.note(_('saving current version of %s as %s\n')  
>> %
>>                          (name, bakname))
>> -                util.rename(os.path.join(self._abspath, name),
>> -                            os.path.join(self._abspath, bakname))
>> +                self.wvfs.rename(name, bakname)
>>
>>          if not opts.get('dry_run'):
>>              self.get(substate, overwrite=True)
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -1300,7 +1300,7 @@ 
         return retdata, p.returncode
 
     def _gitmissing(self):
-        return not os.path.exists(os.path.join(self._abspath, '.git'))
+        return not self.wvfs.exists('.git')
 
     def _gitstate(self):
         return self._gitcommand(['rev-parse', 'HEAD'])
@@ -1632,14 +1632,13 @@ 
         # local-only history
         self.ui.note(_('removing subrepo %s\n') % self._relpath)
         self._gitcommand(['config', 'core.bare', 'true'])
-        for f in os.listdir(self._abspath):
+        for f in self.wvfs.listdir():
             if f == '.git':
                 continue
-            path = os.path.join(self._abspath, f)
-            if os.path.isdir(path) and not os.path.islink(path):
-                shutil.rmtree(path)
+            if self.wvfs.isdir(f) and not self.wvfs.islink(f):
+                shutil.rmtree(self.wvfs.join(f))
             else:
-                os.remove(path)
+                self.wvfs.unlink(f)
 
     def archive(self, archiver, prefix, match=None):
         total = 0
@@ -1815,8 +1814,7 @@ 
                 bakname = "%s.orig" % name
                 self.ui.note(_('saving current version of %s as %s\n') %
                         (name, bakname))
-                util.rename(os.path.join(self._abspath, name),
-                            os.path.join(self._abspath, bakname))
+                self.wvfs.rename(name, bakname)
 
         if not opts.get('dry_run'):
             self.get(substate, overwrite=True)