[PATCH 02 of 10] opener: add read & write utility methods

[Adrian Buehlmann]

On 2010-12-01 22:34, Dan Villiom Podlaski Christiansen wrote:
> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
> # Date 1291227679 -3600
> # Node ID 8c47f61bd78b2bdcbd088e0b2fed3bedc4e995eb
> # Parent  446c177fbf75f7134eec7671998fd112fbe90bed
> opener: add read & write utility methods
> 
> The two new methods are useful for quickly opening a file for reading
> or writing. Unlike 'opener(...).read()', they ensure they the file is
                                                       ----

> immediately closed without relying on CPython reference counting.
> 
> Similar methods are added to custom openers.
> 
> diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
> --- a/mercurial/statichttprepo.py
> +++ b/mercurial/statichttprepo.py
> @@ -75,6 +75,7 @@ def build_opener(ui, authinfo):
>                  raise IOError('Permission denied')
>              f = "/".join((p, urllib.quote(path)))
>              return httprangereader(f, urlopener)
> +        o.read = lambda *args, **kwargs: o(*args, **kwargs).read()
>          return o
>  
>      opener.options = {'nonlazy': 1}
> diff --git a/mercurial/store.py b/mercurial/store.py
> --- a/mercurial/store.py
> +++ b/mercurial/store.py
> @@ -176,6 +176,8 @@ class basicstore(object):
>          op = opener(self.path)
>          op.createmode = self.createmode
>          self.opener = lambda f, *args, **kw: op(encodedir(f), *args, **kw)
> +        self.opener.read = (lambda f, *args, **kw:
> +                            op(encodedir(f), *args, **kw).read())
>  
>      def join(self, f):
>          return self.pathjoiner(self.path, encodedir(f))
> @@ -221,6 +223,8 @@ class encodedstore(basicstore):
>          op = opener(self.path)
>          op.createmode = self.createmode
>          self.opener = lambda f, *args, **kw: op(encodefilename(f), *args, **kw)
> +        self.opener.read = (lambda f, *args, **kw:
> +                            op(encodefilename(f), *args, **kw).read())

IMHO, this is too intrusive. You shouldn't have to add a read member function to
every opener.

Consider creating free functions util.read and write instead, taking an opener
as a parameter.

>      def datafiles(self):
>          for a, b, size in self._walk('data', True):
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -871,6 +871,20 @@ class opener(object):
>              return
>          os.chmod(name, self.createmode & 0666)
>  
> +    def read(self, *args, **kwargs):
> +        fp = self(*args, **kwargs)
> +        try:
> +            return fp.read()
> +        finally:
> +            fp.close()
> +
> +    def write(self, data, *args, **kwargs):
> +        fp = self(*args, **kwargs)
> +        try:
> +            return fp.write(data)
> +        finally:
> +            fp.close()
> +
>      def __call__(self, path, mode="r", text=False, atomictemp=False):
>          self.auditor(path)
>          f = os.path.join(self.base, path)

in util.py:

def read(op, path, text=False):
    fp = op(path, 'r', text)
    try:
        return fp.read()
    finally:
        fp.close()

def write(data, op, path, mode='w', text=False, atomictemp=False):
    fp = op(path, mode, text, atomictemp)
    try:
        fp.write(data)
        if atomictemp:
            fp.rename()
    finally:
        fp.close()


and you probably need:


diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -822,7 +822,7 @@ class atomictempfile(object):
             self._fp.close()
             rename(self.temp, localpath(self.__name))
 
-    def __del__(self):
+    def close(self):
         if not self._fp:
             return
         if not self._fp.closed:
@@ -831,6 +831,9 @@ class atomictempfile(object):
             except: pass
             self._fp.close()
 
+    def __del__(self):
+        self.close()
+
 def makedirs(name, mode=None):
     """recursive directory creation with parent mode inheritance"""
     try:


and then use it as (example):

> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -272,14 +272,14 @@ class queue(object):
>              def parse(l):
>                  n, name = l.split(':', 1)
>                  return statusentry(bin(n), name)
> -            lines = self.opener(self.status_path).read().splitlines()
> +            lines = self.opener.read(self.status_path).splitlines()

               lines = util.read(self.opener, self.status_path).splitlines()

>              return [parse(l) for l in lines]
>          return []