io, gitpython-developers#519: ALL open() --> with open()

+ Some cases had restructuring of code.

Author: ankostis

doc/source/conf.py

@@ -50,7 +50,8 @@
 # built documents.
 #
 # The short X.Y version.
-VERSION = open(os.path.join(os.path.dirname(__file__),"..", "..", 'VERSION')).readline().strip()
+with open(os.path.join(os.path.dirname(__file__),"..", "..", 'VERSION')) as fd:
+    VERSION = fd.readline().strip()
 version = VERSION
 # The full version, including alpha/beta/rc tags.
 release = VERSION

git/objects/submodule/base.py

@@ -854,6 +854,8 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
                     self._clear_cache()
                     wtd = mod.working_tree_dir
                     del(mod)        # release file-handles (windows)
+                    import gc
+                    gc.collect()
                     rmtree(wtd)
                 # END delete tree if possible
             # END handle force

git/refs/symbolic.py

@@ -134,9 +134,8 @@ def _get_ref_info(cls, repo, ref_path):
         point to, or None"""
         tokens = None
         try:
-            fp = open(join(repo.git_dir, ref_path), 'rt')
-            value = fp.read().rstrip()
-            fp.close()
+            with open(join(repo.git_dir, ref_path), 'rt') as fp:
+                value = fp.read().rstrip()
             # Don't only split on spaces, but on whitespace, which allows to parse lines like
             # 60b64ef992065e2600bfef6187a97f92398a9144                branch 'master' of git-server:/path/to/repo
             tokens = value.split()
@@ -313,13 +312,17 @@ def set_reference(self, ref, logmsg=None):
 
         lfd = LockedFD(fpath)
         fd = lfd.open(write=True, stream=True)
-        fd.write(write_value.encode('ascii') + b'\n')
-        lfd.commit()
-
+        ok = True
+        try:
+            fd.write(write_value.encode('ascii') + b'\n')
+            lfd.commit()
+            ok = True
+        finally:
+            if not ok:
+                lfd.rollback()
         # Adjust the reflog
         if logmsg is not None:
             self.log_append(oldbinsha, logmsg)
-        # END handle reflog
 
         return self
 
@@ -422,40 +425,36 @@ def delete(cls, repo, path):
             # check packed refs
             pack_file_path = cls._get_packed_refs_path(repo)
             try:
-                reader = open(pack_file_path, 'rb')
-            except (OSError, IOError):
-                pass  # it didnt exist at all
-            else:
-                new_lines = list()
-                made_change = False
-                dropped_last_line = False
-                for line in reader:
-                    # keep line if it is a comment or if the ref to delete is not
-                    # in the line
-                    # If we deleted the last line and this one is a tag-reference object,
-                    # we drop it as well
-                    line = line.decode(defenc)
-                    if (line.startswith('#') or full_ref_path not in line) and \
-                            (not dropped_last_line or dropped_last_line and not line.startswith('^')):
-                        new_lines.append(line)
-                        dropped_last_line = False
-                        continue
-                    # END skip comments and lines without our path
-
-                    # drop this line
-                    made_change = True
-                    dropped_last_line = True
-                # END for each line in packed refs
-                reader.close()
+                with open(pack_file_path, 'rb') as reader:
+                    new_lines = list()
+                    made_change = False
+                    dropped_last_line = False
+                    for line in reader:
+                        # keep line if it is a comment or if the ref to delete is not
+                        # in the line
+                        # If we deleted the last line and this one is a tag-reference object,
+                        # we drop it as well
+                        line = line.decode(defenc)
+                        if (line.startswith('#') or full_ref_path not in line) and \
+                                (not dropped_last_line or dropped_last_line and not line.startswith('^')):
+                            new_lines.append(line)
+                            dropped_last_line = False
+                            continue
+                        # END skip comments and lines without our path
+
+                        # drop this line
+                        made_change = True
+                        dropped_last_line = True
 
                 # write the new lines
                 if made_change:
                     # write-binary is required, otherwise windows will
                     # open the file in text mode and change LF to CRLF !
-                    open(pack_file_path, 'wb').writelines(l.encode(defenc) for l in new_lines)
-                # END write out file
-            # END open exception handling
-        # END handle deletion
+                    with open(pack_file_path, 'wb') as fd:
+                        fd.writelines(l.encode(defenc) for l in new_lines)
+
+            except (OSError, IOError):
+                pass  # it didnt exist at all
 
         # delete the reflog
         reflog_path = RefLog.path(cls(repo, full_ref_path))
@@ -484,7 +483,8 @@ def _create(cls, repo, path, resolve, reference, force, logmsg=None):
                 target_data = target.path
             if not resolve:
                 target_data = "ref: " + target_data
-            existing_data = open(abs_ref_path, 'rb').read().decode(defenc).strip()
+            with open(abs_ref_path, 'rb') as fd:
+                existing_data = fd.read().decode(defenc).strip()
             if existing_data != target_data:
                 raise OSError("Reference at %r does already exist, pointing to %r, requested was %r" %
                               (full_ref_path, existing_data, target_data))
@@ -549,7 +549,11 @@ def rename(self, new_path, force=False):
         if isfile(new_abs_path):
             if not force:
                 # if they point to the same file, its not an error
-                if open(new_abs_path, 'rb').read().strip() != open(cur_abs_path, 'rb').read().strip():
+                with open(new_abs_path, 'rb') as fd1:
+                    f1 = fd1.read().strip()
+                with open(cur_abs_path, 'rb') as fd2:
+                    f2 = fd2.read().strip()
+                if f1 != f2:
                     raise OSError("File at path %r already exists" % new_abs_path)
                 # else: we could remove ourselves and use the otherone, but
                 # but clarity we just continue as usual

git/remote.py

@@ -638,9 +638,8 @@ def _get_fetch_info_from_stderr(self, proc, progress):
         finalize_process(proc, stderr=stderr_text)
 
         # read head information
-        fp = open(join(self.repo.git_dir, 'FETCH_HEAD'), 'rb')
-        fetch_head_info = [l.decode(defenc) for l in fp.readlines()]
-        fp.close()
+        with open(join(self.repo.git_dir, 'FETCH_HEAD'), 'rb') as fp:
+            fetch_head_info = [l.decode(defenc) for l in fp.readlines()]
 
         l_fil = len(fetch_info_lines)
         l_fhi = len(fetch_head_info)

git/test/fixtures/cat_file.py

@@ -1,5 +1,6 @@
 import sys
 
-for line in open(sys.argv[1]).readlines():
-    sys.stdout.write(line)
-    sys.stderr.write(line)
+with open(sys.argv[1]) as fd:
+    for line in fd.readlines():
+        sys.stdout.write(line)
+        sys.stderr.write(line)

git/test/lib/helper.py

@@ -39,7 +39,8 @@ def fixture_path(name):
 
 
 def fixture(name):
-    return open(fixture_path(name), 'rb').read()
+    with open(fixture_path(name), 'rb') as fd:
+        return fd.read()
 
 
 def absolute_project_path():
@@ -373,7 +374,6 @@ def _make_file(self, rela_path, data, repo=None):
         """
         repo = repo or self.rorepo
         abs_path = os.path.join(repo.working_tree_dir, rela_path)
-        fp = open(abs_path, "w")
-        fp.write(data)
-        fp.close()
+        with open(abs_path, "w") as fp:
+            fp.write(data)
         return abs_path

git/test/test_base.py

@@ -77,13 +77,11 @@ def test_base_object(self):
             assert data
 
             tmpfilename = tempfile.mktemp(suffix='test-stream')
-            tmpfile = open(tmpfilename, 'wb+')
-            assert item == item.stream_data(tmpfile)
-            tmpfile.seek(0)
-            assert tmpfile.read() == data
-            tmpfile.close()
+            with open(tmpfilename, 'wb+') as tmpfile:
+                assert item == item.stream_data(tmpfile)
+                tmpfile.seek(0)
+                assert tmpfile.read() == data
             os.remove(tmpfilename)
-            # END stream to file directly
         # END for each object type to create
 
         # each has a unique sha
@@ -133,7 +131,8 @@ def test_add_unicode(self, rw_repo):
             from nose import SkipTest
             raise SkipTest("Environment doesn't support unicode filenames")
 
-        open(file_path, "wb").write(b'something')
+        with open(file_path, "wb") as fp:
+            fp.write(b'something')
 
         if is_win:
             # on windows, there is no way this works, see images on

git/test/test_commit.py

@@ -313,14 +313,16 @@ def test_serialization_unicode_support(self):
 
     def test_invalid_commit(self):
         cmt = self.rorepo.commit()
-        cmt._deserialize(open(fixture_path('commit_invalid_data'), 'rb'))
+        with open(fixture_path('commit_invalid_data'), 'rb') as fd:
+            cmt._deserialize(fd)
 
         self.assertEqual(cmt.author.name, u'E.Azer Ko�o�o�oculu', cmt.author.name)
         self.assertEqual(cmt.author.email, 'azer@kodfabrik.com', cmt.author.email)
 
     def test_gpgsig(self):
         cmt = self.rorepo.commit()
-        cmt._deserialize(open(fixture_path('commit_with_gpgsig'), 'rb'))
+        with open(fixture_path('commit_with_gpgsig'), 'rb') as fd:
+            cmt._deserialize(fd)
 
         fixture_sig = """-----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.11 (GNU/Linux)

git/test/test_docs.py

@@ -53,7 +53,8 @@ def test_init_repo_object(self, rw_dir):
         # ![5-test_init_repo_object]
 
         # [6-test_init_repo_object]
-        repo.archive(open(join(rw_dir, 'repo.tar'), 'wb'))
+        with open(join(rw_dir, 'repo.tar'), 'wb') as fp:
+            repo.archive(fp)
         # ![6-test_init_repo_object]
 
         # repository paths

git/test/test_git.py

@@ -93,10 +93,9 @@ def test_it_executes_git_to_shell_and_returns_result(self):
 
     def test_it_accepts_stdin(self):
         filename = fixture_path("cat_file_blob")
-        fh = open(filename, 'r')
-        assert_equal("70c379b63ffa0795fdbfbc128e5a2818397b7ef8",
-                     self.git.hash_object(istream=fh, stdin=True))
-        fh.close()
+        with open(filename, 'r') as fh:
+            assert_equal("70c379b63ffa0795fdbfbc128e5a2818397b7ef8",
+                         self.git.hash_object(istream=fh, stdin=True))
 
     @patch.object(Git, 'execute')
     def test_it_ignores_false_kwargs(self, git):
@@ -200,10 +199,9 @@ def test_environment(self, rw_dir):
         self.assertEqual(self.git.environment(), {})
 
         path = os.path.join(rw_dir, 'failing-script.sh')
-        stream = open(path, 'wt')
-        stream.write("#!/usr/bin/env sh\n" +
-                     "echo FOO\n")
-        stream.close()
+        with open(path, 'wt') as stream:
+            stream.write("#!/usr/bin/env sh\n"
+                         "echo FOO\n")
         os.chmod(path, 0o777)
 
         rw_repo = Repo.init(os.path.join(rw_dir, 'repo'))

git/test/test_remote.py

@@ -105,8 +105,8 @@ def tearDown(self):
         gc.collect()
 
     def _print_fetchhead(self, repo):
-        fp = open(os.path.join(repo.git_dir, "FETCH_HEAD"))
-        fp.close()
+        with open(os.path.join(repo.git_dir, "FETCH_HEAD")):
+            pass
 
     def _do_test_fetch_result(self, results, remote):
         # self._print_fetchhead(remote.repo)

git/test/test_repo.py

@@ -781,14 +781,16 @@ def test_git_file(self, rwrepo):
         real_path_abs = os.path.abspath(join_path_native(rwrepo.working_tree_dir, '.real'))
         os.rename(rwrepo.git_dir, real_path_abs)
         git_file_path = join_path_native(rwrepo.working_tree_dir, '.git')
-        open(git_file_path, 'wb').write(fixture('git_file'))
+        with open(git_file_path, 'wb') as fp:
+            fp.write(fixture('git_file'))
 
         # Create a repo and make sure it's pointing to the relocated .git directory.
         git_file_repo = Repo(rwrepo.working_tree_dir)
         self.assertEqual(os.path.abspath(git_file_repo.git_dir), real_path_abs)
 
         # Test using an absolute gitdir path in the .git file.
-        open(git_file_path, 'wb').write(('gitdir: %s\n' % real_path_abs).encode('ascii'))
+        with open(git_file_path, 'wb') as fp:
+            fp.write(('gitdir: %s\n' % real_path_abs).encode('ascii'))
         git_file_repo = Repo(rwrepo.working_tree_dir)
         self.assertEqual(os.path.abspath(git_file_repo.git_dir), real_path_abs)
 

git/util.py

@@ -576,7 +576,7 @@ def _obtain_lock_or_raise(self):
         try:
             flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL
             if is_win:
-                flags |= getattr(os, 'O_SHORT_LIVED')
+                flags |= os.O_SHORT_LIVED
             fd = os.open(lock_file, flags, 0)
             os.close(fd)
         except OSError as e:

setup.py

@@ -15,9 +15,8 @@
 import sys
 from os import path
 
-v = open(path.join(path.dirname(__file__), 'VERSION'))
-VERSION = v.readline().strip()
-v.close()
+with open(path.join(path.dirname(__file__), 'VERSION')) as v:
+    VERSION = v.readline().strip()
 
 with open('requirements.txt') as reqs_file:
     requirements = reqs_file.read().splitlines()
@@ -50,22 +49,18 @@ def make_release_tree(self, base_dir, files):
 def _stamp_version(filename):
     found, out = False, list()
     try:
-        f = open(filename, 'r')
+        with open(filename, 'r') as f:
+            for line in f:
+                if '__version__ =' in line:
+                    line = line.replace("'git'", "'%s'" % VERSION)
+                    found = True
+                out.append(line)
     except (IOError, OSError):
         print("Couldn't find file %s to stamp version" % filename, file=sys.stderr)
-        return
-    # END handle error, usually happens during binary builds
-    for line in f:
-        if '__version__ =' in line:
-            line = line.replace("'git'", "'%s'" % VERSION)
-            found = True
-        out.append(line)
-    f.close()
 
     if found:
-        f = open(filename, 'w')
-        f.writelines(out)
-        f.close()
+        with open(filename, 'w') as f:
+            f.writelines(out)
     else:
         print("WARNING: Couldn't find version line in file %s" % filename, file=sys.stderr)
 
@@ -109,8 +104,7 @@ def _stamp_version(filename):
     install_requires=install_requires,
     test_requirements=test_requires + install_requires,
     zip_safe=False,
-    long_description="""\
-GitPython is a python library used to interact with Git repositories""",
+    long_description="""GitPython is a python library used to interact with Git repositories""",
     classifiers=[
         # Picked from
         #   http://pypi.python.org/pypi?:action=list_classifiers