[PATCH 3 of 4] scmutil: use context managers for file handles

Gregory Szorc gregory.szorc at gmail.com
Sat Jan 2 17:38:47 CST 2016


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1451776787 28800
#      Sat Jan 02 15:19:47 2016 -0800
# Node ID 7626e49bc8d37409c6ef173eff3ddc1dc925413d
# Parent  55459e98812bcec222fabe63bb60b5e5c922d3c1
scmutil: use context managers for file handles

Now that we dropped support for Python 2.4, we are able to use context
managers. Let's replace the try..finally pattern in scmutil.py with
context managers, which close files automatically when the context
manager is exited.

There should be no change in behavior with this patch.

Why convert to context managers if nothing is broken? I'm working on
closing file handles in background threads to improve performance on
Windows. As part of this, I realized there could be some future issues
if the background file closing code isn't designed with context
managers in mind. So, I'd like to switch some code to context managers
so I can design an API that works with context managers.

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -260,49 +260,34 @@ class abstractvfs(object):
         Newly created directories are marked as "not to be indexed by
         the content indexing service", if ``notindexed`` is specified
         for "write" mode access.
         '''
         self.open = self.__call__
         return self.__call__(path, mode, text, atomictemp, notindexed)
 
     def read(self, path):
-        fp = self(path, 'rb')
-        try:
+        with self(path, 'rb') as fp:
             return fp.read()
-        finally:
-            fp.close()
 
     def readlines(self, path, mode='rb'):
-        fp = self(path, mode=mode)
-        try:
+        with self(path, mode=mode) as fp:
             return fp.readlines()
-        finally:
-            fp.close()
 
     def write(self, path, data):
-        fp = self(path, 'wb')
-        try:
+        with self(path, 'wb') as fp:
             return fp.write(data)
-        finally:
-            fp.close()
 
     def writelines(self, path, data, mode='wb', notindexed=False):
-        fp = self(path, mode=mode, notindexed=notindexed)
-        try:
+        with self(path, mode=mode, notindexed=notindexed) as fp:
             return fp.writelines(data)
-        finally:
-            fp.close()
 
     def append(self, path, data):
-        fp = self(path, 'ab')
-        try:
+        with self(path, 'ab') as fp:
             return fp.write(data)
-        finally:
-            fp.close()
 
     def basename(self, path):
         """return base element of a path (as os.path.basename would do)
 
         This exists to allow handling of strange encoding if needed."""
         return os.path.basename(path)
 
     def chmod(self, path, mode):
@@ -521,21 +506,20 @@ class vfs(abstractvfs):
                     return util.atomictempfile(f, mode, self.createmode)
                 try:
                     if 'w' in mode:
                         util.unlink(f)
                         nlink = 0
                     else:
                         # nlinks() may behave differently for files on Windows
                         # shares if the file is open.
-                        fd = util.posixfile(f)
-                        nlink = util.nlinks(f)
-                        if nlink < 1:
-                            nlink = 2 # force mktempcopy (issue1922)
-                        fd.close()
+                        with util.posixfile(f):
+                            nlink = util.nlinks(f)
+                            if nlink < 1:
+                                nlink = 2 # force mktempcopy (issue1922)
                 except (OSError, IOError) as e:
                     if e.errno != errno.ENOENT:
                         raise
                     nlink = 0
                     util.ensuredirs(dirname, self.createmode, notindexed)
                 if nlink > 0:
                     if self._trustnlink is None:
                         self._trustnlink = nlink > 1 or util.checknlink(f)
@@ -1022,20 +1006,19 @@ def readrequires(opener, supported):
         raise error.RequirementError(
             _("repository requires features unknown to this Mercurial: %s")
             % " ".join(missings),
             hint=_("see https://mercurial-scm.org/wiki/MissingRequirement"
                    " for more information"))
     return requirements
 
 def writerequires(opener, requirements):
-    reqfile = opener("requires", "w")
-    for r in sorted(requirements):
-        reqfile.write("%s\n" % r)
-    reqfile.close()
+    with opener('requires', 'w') as fp:
+        for r in sorted(requirements):
+            fp.write("%s\n" % r)
 
 class filecachesubentry(object):
     def __init__(self, path, stat):
         self.path = path
         self.cachestat = None
         self._cacheable = None
 
         if stat:


More information about the Mercurial-devel mailing list