Repository: gitpython-developers/GitPython
Pull request #388 (2016-02-20 21:11:58)

enable config parsers to be used as context managers
If used as context managers, the parsers will automatically release their file locks. This was mentioned in #333 and seems like a good thing to have, as you can't forget to call `release` that way.
Another nice thing might be to use `__enter__` to re-acquire locks if necessary, so that the readers can be used in multiple contexts.


Patch:
diff --git a/git/config.py b/git/config.py
index 51b15989..21af6159 100644
--- a/git/config.py
+++ b/git/config.py
@@ -95,7 +95,10 @@ class SectionConstraint(object):
     """Constrains a ConfigParser to only option commands which are constrained to
     always use the section we have been initialized with.
 
-    It supports all ConfigParser methods that operate on an option"""
+    It supports all ConfigParser methods that operate on an option.
+
+    :note:
+        If used as a context manager, will release the wrapped ConfigParser."""
     __slots__ = ("_config", "_section_name")
     _valid_attrs_ = ("get_value", "set_value", "get", "set", "getint", "getfloat", "getboolean", "has_option",
                      "remove_section", "remove_option", "options")
@@ -129,6 +132,12 @@ class SectionConstraint(object):
         """Equivalent to GitConfigParser.release(), which is called on our underlying parser instance"""
         return self._config.release()
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exception_type, exception_value, traceback):
+        self.release()
+
 
 class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, object)):
 
@@ -145,7 +154,9 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje
 
     :note:
         The config is case-sensitive even when queried, hence section and option names
-        must match perfectly."""
+        must match perfectly.
+        If used as a context manager, will release the locked file. This parser cannot
+        be used afterwards."""
 
     #{ Configuration
     # The lock type determines the type of lock to use in new configuration readers.
@@ -216,6 +227,12 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje
         # NOTE: only consistent in PY2
         self.release()
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exception_type, exception_value, traceback):
+        self.release()
+
     def release(self):
         """Flush changes and release the configuration write lock. This instance must not be used anymore afterwards.
         In Python 3, it's required to explicitly release locks and flush changes, as __del__ is not called
diff --git a/git/repo/base.py b/git/repo/base.py
index 9f077fa6..a23e767a 100644
--- a/git/repo/base.py
+++ b/git/repo/base.py
@@ -403,7 +403,7 @@ class Repo(object):
         """
         :return:
             GitConfigParser allowing to write values of the specified configuration file level.
-            Config writers should be retrieved, used to change the configuration ,and written
+            Config writers should be retrieved, used to change the configuration, and written
             right away as they will lock the configuration file in question and prevent other's
             to write it.
 


Test patch:
diff --git a/git/test/test_config.py b/git/test/test_config.py
index 22f342aa..81eacd20 100644
--- a/git/test/test_config.py
+++ b/git/test/test_config.py
@@ -139,57 +139,49 @@ class TestBase(TestCase):
 
         # PREPARE CONFIG FILE A
         fpa = os.path.join(rw_dir, 'a')
-        cw = GitConfigParser(fpa, read_only=False)
-        write_test_value(cw, 'a')
-
-        fpb = os.path.join(rw_dir, 'b')
-        fpc = os.path.join(rw_dir, 'c')
-        cw.set_value('include', 'relative_path_b', 'b')
-        cw.set_value('include', 'doesntexist', 'foobar')
-        cw.set_value('include', 'relative_cycle_a_a', 'a')
-        cw.set_value('include', 'absolute_cycle_a_a', fpa)
-        cw.release()
+        with GitConfigParser(fpa, read_only=False) as cw:
+            write_test_value(cw, 'a')
+
+            fpb = os.path.join(rw_dir, 'b')
+            fpc = os.path.join(rw_dir, 'c')
+            cw.set_value('include', 'relative_path_b', 'b')
+            cw.set_value('include', 'doesntexist', 'foobar')
+            cw.set_value('include', 'relative_cycle_a_a', 'a')
+            cw.set_value('include', 'absolute_cycle_a_a', fpa)
         assert os.path.exists(fpa)
 
         # PREPARE CONFIG FILE B
-        cw = GitConfigParser(fpb, read_only=False)
-        write_test_value(cw, 'b')
-        cw.set_value('include', 'relative_cycle_b_a', 'a')
-        cw.set_value('include', 'absolute_cycle_b_a', fpa)
-        cw.set_value('include', 'relative_path_c', 'c')
-        cw.set_value('include', 'absolute_path_c', fpc)
-        cw.release()
+        with GitConfigParser(fpb, read_only=False) as cw:
+            write_test_value(cw, 'b')
+            cw.set_value('include', 'relative_cycle_b_a', 'a')
+            cw.set_value('include', 'absolute_cycle_b_a', fpa)
+            cw.set_value('include', 'relative_path_c', 'c')
+            cw.set_value('include', 'absolute_path_c', fpc)
 
         # PREPARE CONFIG FILE C
-        cw = GitConfigParser(fpc, read_only=False)
-        write_test_value(cw, 'c')
-        cw.release()
+        with GitConfigParser(fpc, read_only=False) as cw:
+            write_test_value(cw, 'c')
 
-        cr = GitConfigParser(fpa, read_only=True)
-        for tv in ('a', 'b', 'c'):
-            check_test_value(cr, tv)
-        # end for each test to verify
-        assert len(cr.items('include')) == 8, "Expected all include sections to be merged"
-        cr.release()
+        with GitConfigParser(fpa, read_only=True) as cr:
+            for tv in ('a', 'b', 'c'):
+                check_test_value(cr, tv)
+            # end for each test to verify
+            assert len(cr.items('include')) == 8, "Expected all include sections to be merged"
 
         # test writable config writers - assure write-back doesn't involve includes
-        cw = GitConfigParser(fpa, read_only=False, merge_includes=True)
-        tv = 'x'
-        write_test_value(cw, tv)
-        cw.release()
+        with GitConfigParser(fpa, read_only=False, merge_includes=True) as cw:
+            tv = 'x'
+            write_test_value(cw, tv)
 
-        cr = GitConfigParser(fpa, read_only=True)
-        self.failUnlessRaises(cp.NoSectionError, check_test_value, cr, tv)
-        cr.release()
+        with GitConfigParser(fpa, read_only=True) as cr:
+            self.failUnlessRaises(cp.NoSectionError, check_test_value, cr, tv)
 
         # But can make it skip includes alltogether, and thus allow write-backs
-        cw = GitConfigParser(fpa, read_only=False, merge_includes=False)
-        write_test_value(cw, tv)
-        cw.release()
+        with GitConfigParser(fpa, read_only=False, merge_includes=False) as cw:
+            write_test_value(cw, tv)
 
-        cr = GitConfigParser(fpa, read_only=True)
-        check_test_value(cr, tv)
-        cr.release()
+        with GitConfigParser(fpa, read_only=True) as cr:
+            check_test_value(cr, tv)
 
     def test_rename(self):
         file_obj = self._to_memcache(fixture_path('git_config'))
diff --git a/test-requirements.txt b/test-requirements.txt
index d9425c03..4d08a501 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -1,3 +1,5 @@
+-r requirements.txt
+
 coverage
 flake8
 nose