convert: fix various leaked file descriptors

Some of these only leaked if an exception occurred between the open and close,
but a lot of these leaked unconditionally.

A type hint is added to `parsesplicemap` because otherwise this change caused
pytype to change the return type from this to `Dict[nothing, nothing]`.

hgext/convert/common.py

@@ -575,22 +575,25 b' class mapfile(dict):'
             fp = open(self.path, b'rb')
         except FileNotFoundError:
             return
-        for i, line in enumerate(fp):
-            line = line.splitlines()[0].rstrip()
-            if not line:
-                # Ignore blank lines
-                continue
-            try:
-                key, value = line.rsplit(b' ', 1)
-            except ValueError:
-                raise error.Abort(
-                    _(b'syntax error in %s(%d): key/value pair expected')
-                    % (self.path, i + 1)
-                )
-            if key not in self:
-                self.order.append(key)
-            super(mapfile, self).__setitem__(key, value)
-        fp.close()
+
+        try:
+            for i, line in enumerate(fp):
+                line = line.splitlines()[0].rstrip()
+                if not line:
+                    # Ignore blank lines
+                    continue
+                try:
+                    key, value = line.rsplit(b' ', 1)
+                except ValueError:
+                    raise error.Abort(
+                        _(b'syntax error in %s(%d): key/value pair expected')
+                        % (self.path, i + 1)
+                    )
+                if key not in self:
+                    self.order.append(key)
+                super(mapfile, self).__setitem__(key, value)
+        finally:
+            fp.close()
 
     def __setitem__(self, key, value) -> None:
         if self.fp is None:

hgext/convert/convcmd.py

@@ -13,6 +13,8 b' import typing'
 
 from typing import (
     AnyStr,
+    Dict,
+    List,
     Mapping,
     Optional,
     Union,
@@ -297,7 +299,7 b' class converter:'
         self.splicemap = self.parsesplicemap(opts.get(b'splicemap'))
         self.branchmap = mapfile(ui, opts.get(b'branchmap'))
 
-    def parsesplicemap(self, path: bytes):
+    def parsesplicemap(self, path: bytes) -> Dict[bytes, List[bytes]]:
         """check and validate the splicemap format and
         return a child/parents dictionary.
         Format checking has two parts.
@@ -312,31 +314,31 b' class converter:'
             return {}
         m = {}
         try:
-            fp = open(path, b'rb')
-            for i, line in enumerate(fp):
-                line = line.splitlines()[0].rstrip()
-                if not line:
-                    # Ignore blank lines
-                    continue
-                # split line
-                lex = common.shlexer(data=line, whitespace=b',')
-                line = list(lex)
-                # check number of parents
-                if not (2 <= len(line) <= 3):
-                    raise error.Abort(
-                        _(
-                            b'syntax error in %s(%d): child parent1'
-                            b'[,parent2] expected'
+            with open(path, b'rb') as fp:
+                for i, line in enumerate(fp):
+                    line = line.splitlines()[0].rstrip()
+                    if not line:
+                        # Ignore blank lines
+                        continue
+                    # split line
+                    lex = common.shlexer(data=line, whitespace=b',')
+                    line = list(lex)
+                    # check number of parents
+                    if not (2 <= len(line) <= 3):
+                        raise error.Abort(
+                            _(
+                                b'syntax error in %s(%d): child parent1'
+                                b'[,parent2] expected'
+                            )
+                            % (path, i + 1)
                         )
-                        % (path, i + 1)
-                    )
-                for part in line:
-                    self.source.checkrevformat(part)
-                child, p1, p2 = line[0], line[1:2], line[2:]
-                if p1 == p2:
-                    m[child] = p1
-                else:
-                    m[child] = p1 + p2
+                    for part in line:
+                        self.source.checkrevformat(part)
+                    child, p1, p2 = line[0], line[1:2], line[2:]
+                    if p1 == p2:
+                        m[child] = p1
+                    else:
+                        m[child] = p1 + p2
         # if file does not exist or error reading, exit
         except IOError:
             raise error.Abort(
@@ -509,14 +511,13 b' class converter:'
         authorfile = self.authorfile
         if authorfile:
             self.ui.status(_(b'writing author map file %s\n') % authorfile)
-            ofile = open(authorfile, b'wb+')
-            for author in self.authors:
-                ofile.write(
-                    util.tonativeeol(
-                        b"%s=%s\n" % (author, self.authors[author])
+            with open(authorfile, b'wb+') as ofile:
+                for author in self.authors:
+                    ofile.write(
+                        util.tonativeeol(
+                            b"%s=%s\n" % (author, self.authors[author])
+                        )
                     )
-                )
-            ofile.close()
 
     def readauthormap(self, authorfile) -> None:
         self.authors = readauthormap(self.ui, authorfile, self.authors)

hgext/convert/cvs.py

@@ -11,9 +11,6 b' import re'
 import socket
 
 from mercurial.i18n import _
-from mercurial.pycompat import (
-    open,
-)
 from mercurial import (
     encoding,
     error,
@@ -52,8 +49,8 b' class convert_cvs(converter_source):'
         self.tags = {}
         self.lastbranch = {}
         self.socket = None
-        self.cvsroot = open(os.path.join(cvs, b"Root"), b'rb').read()[:-1]
-        self.cvsrepo = open(os.path.join(cvs, b"Repository"), b'rb').read()[:-1]
+        self.cvsroot = util.readfile(os.path.join(cvs, b"Root"))[:-1]
+        self.cvsrepo = util.readfile(os.path.join(cvs, b"Repository"))[:-1]
         self.encoding = encoding.encoding
 
         self._connect()
@@ -160,8 +157,7 b' class convert_cvs(converter_source):'
                     passw = b"A"
                     cvspass = os.path.expanduser(b"~/.cvspass")
                     try:
-                        pf = open(cvspass, b'rb')
-                        for line in pf.read().splitlines():
+                        for line in util.readfile(cvspass).splitlines():
                             part1, part2 = line.split(b' ', 1)
                             # /1 :pserver:[e-mail]:2401/cvsroot/foo
                             # Ah<Z
@@ -174,7 +170,6 b' class convert_cvs(converter_source):'
                             if part1 == format:
                                 passw = part2
                                 break
-                        pf.close()
                     except IOError as inst:
                         if inst.errno != errno.ENOENT:
                             if not getattr(inst, 'filename', None):

hgext/convert/cvsps.py

@@ -161,7 +161,7 b' def createlog(ui, directory=None, root=b'
 
         # Use the Root file in the sandbox, if it exists
         try:
-            root = open(os.path.join(b'CVS', b'Root'), b'rb').read().strip()
+            root = util.readfile(os.path.join(b'CVS', b'Root')).strip()
         except IOError:
             pass
 
@@ -195,16 +195,17 b' def createlog(ui, directory=None, root=b'
     if cache == b'update':
         try:
             ui.note(_(b'reading cvs log cache %s\n') % cachefile)
-            oldlog = pickle.load(open(cachefile, b'rb'))
-            for e in oldlog:
-                if not (
-                    hasattr(e, 'branchpoints')
-                    and hasattr(e, 'commitid')
-                    and hasattr(e, 'mergepoint')
-                ):
-                    ui.status(_(b'ignoring old cache\n'))
-                    oldlog = []
-                    break
+            with open(cachefile, b'rb') as fp:
+                oldlog = pickle.load(fp)
+                for e in oldlog:
+                    if not (
+                        hasattr(e, 'branchpoints')
+                        and hasattr(e, 'commitid')
+                        and hasattr(e, 'mergepoint')
+                    ):
+                        ui.status(_(b'ignoring old cache\n'))
+                        oldlog = []
+                        break
 
             ui.note(_(b'cache has %d log entries\n') % len(oldlog))
         except Exception as e:
@@ -526,7 +527,9 b' def createlog(ui, directory=None, root=b'
 
             # write the new cachefile
             ui.note(_(b'writing cvs log cache %s\n') % cachefile)
-            pickle.dump(log, open(cachefile, b'wb'))
+
+            with open(cachefile, b'wb') as fp:
+                pickle.dump(log, fp)
         else:
             log = oldlog
 

hgext/convert/monotone.py

@@ -43,9 +43,8 b' class monotone_source(common.converter_s'
         if not os.path.exists(os.path.join(path, b'_MTN')):
             # Could be a monotone repository (SQLite db file)
             try:
-                f = open(path, b'rb')
-                header = f.read(16)
-                f.close()
+                with open(path, b'rb') as f:
+                    header = f.read(16)
             except IOError:
                 header = b''
             if header != b'SQLite format 3\x00':

hgext/convert/subversion.py

@@ -1488,9 +1488,11 b' class svn_sink(converter_sink, commandli'
                 prop_actions_allowed.append((b'M', b'svn:date'))
 
             hook = os.path.join(created, b'hooks', b'pre-revprop-change')
-            fp = open(hook, b'wb')
-            fp.write(gen_pre_revprop_change_hook(prop_actions_allowed))
-            fp.close()
+
+            util.writefile(
+                hook, gen_pre_revprop_change_hook(prop_actions_allowed)
+            )
+
             util.setflags(hook, False, True)
 
         output = self.run0(b'info')