Skip to content

config parsers as context mangers can now be reentered for locks #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions git/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,11 @@ def release(self):
return self._config.release()

def __enter__(self):
self._config.__enter__()
return self

def __exit__(self, exception_type, exception_value, traceback):
self.release()
self._config.__exit__(exception_type, exception_value, traceback)


class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, object)):
Expand All @@ -155,8 +156,7 @@ 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.
If used as a context manager, will release the locked file. This parser cannot
be used afterwards."""
If used as a context manager, will release the locked file."""

#{ Configuration
# The lock type determines the type of lock to use in new configuration readers.
Expand Down Expand Up @@ -206,18 +206,23 @@ def __init__(self, file_or_files, read_only=True, merge_includes=True):
self._is_initialized = False
self._merge_includes = merge_includes
self._lock = None

if not read_only:
if isinstance(file_or_files, (tuple, list)):
raise ValueError(
"Write-ConfigParsers can operate on a single file only, multiple files have been passed")
# END single file check

if not isinstance(file_or_files, string_types):
file_or_files = file_or_files.name
# END get filename from handle/stream
# initialize lock base - we want to write
self._lock = self.t_lock(file_or_files)
self._aquire_lock()

def _aquire_lock(self):
if not self._read_only:
if not self._lock:
if isinstance(self._file_or_files, (tuple, list)):
raise ValueError(
"Write-ConfigParsers can operate on a single file only, multiple files have been passed")
# END single file check

file_or_files = self._file_or_files
if not isinstance(self._file_or_files, string_types):
file_or_files = self._file_or_files.name
# END get filename from handle/stream
# initialize lock base - we want to write
self._lock = self.t_lock(file_or_files)
# END lock check

self._lock._obtain_lock()
# END read-only check
Expand All @@ -228,6 +233,7 @@ def __del__(self):
self.release()

def __enter__(self):
self._aquire_lock()
return self

def __exit__(self, exception_type, exception_value, traceback):
Expand Down
17 changes: 17 additions & 0 deletions git/test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,23 @@ def test_read_write(self):
w_config.release()
# END for each filename

@with_rw_directory
def test_lock_reentry(self, rw_dir):
fpl = os.path.join(rw_dir, 'l')
gcp = GitConfigParser(fpl, read_only=False)
with gcp as cw:
cw.set_value('include', 'some_value', 'a')
# entering again locks the file again...
with gcp as cw:
cw.set_value('include', 'some_other_value', 'b')
# ...so creating an additional config writer must fail due to exclusive access
self.failUnlessRaises(IOError, GitConfigParser, fpl, read_only=False)
# but work when the lock is removed
with GitConfigParser(fpl, read_only=False):
assert os.path.exists(fpl)
# reentering with an existing lock must fail due to exclusive access
self.failUnlessRaises(IOError, gcp.__enter__)

def test_multi_line_config(self):
file_obj = self._to_memcache(fixture_path("git_config_with_comments"))
config = GitConfigParser(file_obj, read_only=False)
Expand Down