diff options
author | Sebastian Thiel <byronimo@gmail.com> | 2015-01-14 12:46:51 +0100 |
---|---|---|
committer | Sebastian Thiel <byronimo@gmail.com> | 2015-01-14 12:46:51 +0100 |
commit | 619c989915b568e4737951fafcbae14cd06d6ea6 (patch) | |
tree | 3dde760f0018eb418a8c2bb9f0587dc42f74e680 | |
parent | be074c655ad53927541fc6443eed8b0c2550e415 (diff) | |
download | gitpython-619c989915b568e4737951fafcbae14cd06d6ea6.tar.gz |
GitConfigParser now respects and merges 'include' sections
We implement it as described in this article:
http://stackoverflow.com/questions/1557183/is-it-possible-to-include-a-file-in-your-gitconfig
Thus we handle
* cycles
* relative and absolute include paths
* write-backs in case of writable GitConfigParser instances
Fixes #201
-rw-r--r-- | .gitmodules | 2 | ||||
-rw-r--r-- | git/config.py | 83 | ||||
-rw-r--r-- | git/repo/base.py | 4 | ||||
-rw-r--r-- | git/test/fixtures/git_config | 3 | ||||
-rw-r--r-- | git/test/test_config.py | 68 |
5 files changed, 143 insertions, 17 deletions
diff --git a/.gitmodules b/.gitmodules index 612c39d9..4a3f37c2 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,3 @@ [submodule "gitdb"] - path = git/ext/gitdb url = https://github.com/gitpython-developers/gitdb.git + path = git/ext/gitdb diff --git a/git/config.py b/git/config.py index 5828a1c1..4c4cb491 100644 --- a/git/config.py +++ b/git/config.py @@ -15,6 +15,7 @@ except ImportError: import inspect import logging import abc +import os from git.odict import OrderedDict from git.util import LockFile @@ -164,7 +165,7 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje # list of RawConfigParser methods able to change the instance _mutating_methods_ = ("add_section", "remove_section", "remove_option", "set") - def __init__(self, file_or_files, read_only=True): + def __init__(self, file_or_files, read_only=True, merge_includes=True): """Initialize a configuration reader to read the given file_or_files and to possibly allow changes to it by setting read_only False @@ -173,7 +174,13 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje :param read_only: If True, the ConfigParser may only read the data , but not change it. - If False, only a single file path or file object may be given.""" + If False, only a single file path or file object may be given. We will write back the changes + when they happen, or when the ConfigParser is released. This will not happen if other + configuration files have been included + :param merge_includes: if True, we will read files mentioned in [include] sections and merge their + contents into ours. This makes it impossible to write back an individual configuration file. + Thus, if you want to modify a single conifguration file, turn this off to leave the original + dataset unaltered when reading it.""" cp.RawConfigParser.__init__(self, dict_type=OrderedDict) # Used in python 3, needs to stay in sync with sections for underlying implementation to work @@ -183,6 +190,7 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje self._file_or_files = file_or_files self._read_only = read_only self._is_initialized = False + self._merge_includes = merge_includes self._lock = None if not read_only: @@ -313,7 +321,6 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje if not e: e = cp.ParsingError(fpname) e.append(lineno, repr(line)) - print(lineno, line) continue else: line = line.rstrip() @@ -329,6 +336,9 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje if e: raise e + def _has_includes(self): + return self._merge_includes and self.has_section('include') + def read(self): """Reads the data stored in the files we have been initialized with. It will ignore files that cannot be read, possibly leaving an empty configuration @@ -337,18 +347,25 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje :raise IOError: if a file cannot be handled""" if self._is_initialized: return + self._is_initialized = True - files_to_read = self._file_or_files - if not isinstance(files_to_read, (tuple, list)): - files_to_read = [files_to_read] - - for file_object in files_to_read: - fp = file_object + if not isinstance(self._file_or_files, (tuple, list)): + files_to_read = [self._file_or_files] + else: + files_to_read = list(self._file_or_files) + # end assure we have a copy of the paths to handle + + seen = set(files_to_read) + num_read_include_files = 0 + while files_to_read: + file_path = files_to_read.pop(0) + fp = file_path close_fp = False + # assume a path if it is not a file-object - if not hasattr(file_object, "seek"): + if not hasattr(fp, "seek"): try: - fp = open(file_object, 'rb') + fp = open(file_path, 'rb') close_fp = True except IOError: continue @@ -360,8 +377,33 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje if close_fp: fp.close() # END read-handling - # END for each file object to read - self._is_initialized = True + + # Read includes and append those that we didn't handle yet + # We expect all paths to be normalized and absolute (and will assure that is the case) + if self._has_includes(): + for _, include_path in self.items('include'): + if not os.path.isabs(include_path): + if not close_fp: + continue + # end ignore relative paths if we don't know the configuration file path + assert os.path.isabs(file_path), "Need absolute paths to be sure our cycle checks will work" + include_path = os.path.join(os.path.dirname(file_path), include_path) + # end make include path absolute + include_path = os.path.normpath(include_path) + if include_path in seen or not os.access(include_path, os.R_OK): + continue + seen.add(include_path) + files_to_read.append(include_path) + num_read_include_files += 1 + # each include path in configuration file + # end handle includes + # END for each file object to read + + # If there was no file included, we can safely write back (potentially) the configuration file + # without altering it's meaning + if num_read_include_files == 0: + self._merge_includes = False + # end def _write(self, fp): """Write an .ini-format representation of the configuration state in @@ -379,6 +421,10 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje for name, value in self._sections.items(): write_section(name, value) + def items(self, section_name): + """:return: list((option, value), ...) pairs of all items in the given section""" + return [(k, v) for k, v in super(GitConfigParser, self).items(section_name) if k != '__name__'] + @needs_values def write(self): """Write changes to our file, if there are changes at all @@ -387,6 +433,17 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje a file lock""" self._assure_writable("write") + if isinstance(self._file_or_files, (list, tuple)): + raise AssertionError("Cannot write back if there is not exactly a single file to write to, have %i files" + % len(self._file_or_files)) + # end assert multiple files + + if self._has_includes(): + log.debug("Skipping write-back of confiuration file as include files were merged in." + + "Set merge_includes=False to prevent this.") + return + # end + fp = self._file_or_files close_fp = False diff --git a/git/repo/base.py b/git/repo/base.py index f5ed2479..155a674f 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -359,11 +359,11 @@ class Repo(object): return "/etc/gitconfig" elif config_level == "user": config_home = os.environ.get("XDG_CONFIG_HOME") or os.path.join(os.environ.get("HOME", '~'), ".config") - return os.path.expanduser(join(config_home, "git", "config")) + return os.path.normpath(os.path.expanduser(join(config_home, "git", "config"))) elif config_level == "global": return os.path.normpath(os.path.expanduser("~/.gitconfig")) elif config_level == "repository": - return join(self.git_dir, "config") + return os.path.normpath(join(self.git_dir, "config")) raise ValueError("Invalid configuration level: %r" % config_level) diff --git a/git/test/fixtures/git_config b/git/test/fixtures/git_config index ff8e7114..c9945cd5 100644 --- a/git/test/fixtures/git_config +++ b/git/test/fixtures/git_config @@ -27,3 +27,6 @@ [branch "mainline_performance"] remote = mainline merge = refs/heads/master +[include] + path = doesntexist.cfg + abspath = /usr/bin/foodoesntexist.bar
\ No newline at end of file diff --git a/git/test/test_config.py b/git/test/test_config.py index 6851b2a1..9a44d9e3 100644 --- a/git/test/test_config.py +++ b/git/test/test_config.py @@ -7,8 +7,9 @@ from git.test.lib import ( TestCase, fixture_path, - assert_equal + assert_equal, ) +from gitdb.test.lib import with_rw_directory from git import ( GitConfigParser ) @@ -16,6 +17,7 @@ from git.compat import ( string_types, ) import io +import os from copy import copy from git.config import cp @@ -127,3 +129,67 @@ class TestBase(TestCase): # it raises if there is no default though self.failUnlessRaises(cp.NoSectionError, r_config.get_value, "doesnt", "exist") + + @with_rw_directory + def test_config_include(self, rw_dir): + def write_test_value(cw, value): + cw.set_value(value, 'value', value) + # end + + def check_test_value(cr, value): + assert cr.get_value(value, 'value') == value + # end + + # 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() + 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() + + # PREPARE CONFIG FILE C + cw = GitConfigParser(fpc, read_only=False) + write_test_value(cw, 'c') + cw.release() + + 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() + + # 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() + + cr = GitConfigParser(fpa, read_only=True) + self.failUnlessRaises(cp.NoSectionError, check_test_value, cr, tv) + cr.release() + + # 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() + + cr = GitConfigParser(fpa, read_only=True) + check_test_value(cr, tv) + cr.release() |