Patchwork [4,of,4] patch: replace file api with vfs api in fsbackend

login
register
mail settings
Submitter Chinmay Joshi
Date May 27, 2014, 6:58 p.m.
Message ID <fdd07320898be7a84710.1401217083@ubuntu>
Download mbox | patch
Permalink /patch/4871/
State Changes Requested
Headers show

Comments

Chinmay Joshi - May 27, 2014, 6:58 p.m.
# HG changeset patch
# User Chinmay Joshi <c@chinmayjoshi.com>
# Date 1401216458 -19800
#      Wed May 28 00:17:38 2014 +0530
# Node ID fdd07320898be7a8471005f8900b0bdc4c0b36b3
# Parent  0375426f4b794f252af6d1d7fbfd600a68071027
patch: replace file api with vfs api in fsbackend

This patch replaces users with os.* and util.* with vfs API from opener.
Katsunori FUJIWARA - May 27, 2014, 11:51 p.m.
At Wed, 28 May 2014 00:28:03 +0530,
Chinmay Joshi wrote:
> 
> # HG changeset patch
> # User Chinmay Joshi <c@chinmayjoshi.com>
> # Date 1401216458 -19800
> #      Wed May 28 00:17:38 2014 +0530
> # Node ID fdd07320898be7a8471005f8900b0bdc4c0b36b3
> # Parent  0375426f4b794f252af6d1d7fbfd600a68071027
> patch: replace file api with vfs api in fsbackend
> 
> This patch replaces users with os.* and util.* with vfs API from opener.
> 
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -418,11 +418,11 @@
>  
>      def getfile(self, fname):
>          path = self._join(fname)
> -        if os.path.islink(path):
> -            return (os.readlink(path), (True, False))
> +        if self.opener.islink(path):
> +            return (self.opener.readlink(path), (True, False))
>          isexec = False
>          try:
> -            isexec = os.lstat(path).st_mode & 0100 != 0
> +            isexec = self.opener.lstat(path).st_mode & 0100 != 0
>          except OSError, e:
>              if e.errno != errno.ENOENT:
>                  raise

In this case, "path" is already joined with "self.opener.base" by
"_join()" below and it should be absolute path:

    def _join(self, f):
        return os.path.join(self.opener.base, f)

So, "islink", "readlink" and "lstat" of vfs/opener should be applied
on not "path" but "fname" for efficiency.

This problem can't be detected by test suite, because "os.join" for
two absolute paths can return expected result, even though it isn't
efficiency.

Only checking the origin of values held by each variables (you may
have to check also callers of the target function retroactively) can
avoids problems of this kind :-<


> @@ -438,7 +438,7 @@
>          else:
>              self.opener.write(fname, data)
>              if isexec:
> -                util.setflags(self._join(fname), False, True)
> +                self.opener.setflags(self._join(fname), False, True)
>  
>      def unlink(self, fname):
>          util.unlinkpath(self._join(fname), ignoremissing=True)
> @@ -453,7 +453,7 @@
>          fp.close()
>  
>      def exists(self, fname):
> -        return os.path.lexists(self._join(fname))
> +        return self.opener.lexists(self._join(fname))
>  
>  class workingbackend(fsbackend):
>      def __init__(self, ui, repo, similarity):

These hunks should use "fname" instead of "self._join(fname)", too.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -418,11 +418,11 @@ 
 
     def getfile(self, fname):
         path = self._join(fname)
-        if os.path.islink(path):
-            return (os.readlink(path), (True, False))
+        if self.opener.islink(path):
+            return (self.opener.readlink(path), (True, False))
         isexec = False
         try:
-            isexec = os.lstat(path).st_mode & 0100 != 0
+            isexec = self.opener.lstat(path).st_mode & 0100 != 0
         except OSError, e:
             if e.errno != errno.ENOENT:
                 raise
@@ -438,7 +438,7 @@ 
         else:
             self.opener.write(fname, data)
             if isexec:
-                util.setflags(self._join(fname), False, True)
+                self.opener.setflags(self._join(fname), False, True)
 
     def unlink(self, fname):
         util.unlinkpath(self._join(fname), ignoremissing=True)
@@ -453,7 +453,7 @@ 
         fp.close()
 
     def exists(self, fname):
-        return os.path.lexists(self._join(fname))
+        return self.opener.lexists(self._join(fname))
 
 class workingbackend(fsbackend):
     def __init__(self, ui, repo, similarity):