From c16f584725a4cadafc6e113abef45f4ea52d03b3 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 13:55:47 -0700 Subject: [PATCH 01/12] Add Regex to match content of "includeIf" section --- git/config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git/config.py b/git/config.py index 43f854f21..4de050a58 100644 --- a/git/config.py +++ b/git/config.py @@ -38,6 +38,9 @@ # represents the configuration level of a configuration file CONFIG_LEVELS = ("system", "user", "global", "repository") +# Section pattern to detect conditional includes. +# https://git-scm.com/docs/git-config#_conditional_includes +CONDITIONAL_INCLUDE_REGEXP = re.compile(r"(?<=includeIf )\"(gitdir|gitdir/i|onbranch):(.+)\"") class MetaParserBuilder(abc.ABCMeta): From 9766832e11cdd8afed16dfd2d64529c2ae9c3382 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 13:57:06 -0700 Subject: [PATCH 02/12] Update check method to find all includes --- git/config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index 4de050a58..e686dcd3e 100644 --- a/git/config.py +++ b/git/config.py @@ -446,7 +446,10 @@ def string_decode(v): raise e def _has_includes(self): - return self._merge_includes and self.has_section('include') + return self._merge_includes and any( + section == 'include' or section.startswith('includeIf ') + for section in self.sections() + ) def read(self): """Reads the data stored in the files we have been initialized with. It will From d5262acbd33b70fb676284991207fb24fa9ac895 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 14:03:17 -0700 Subject: [PATCH 03/12] Add reference to repository to config. This is necessary when working with conditional include sections as it requires the git directory or active branch name. https://git-scm.com/docs/git-config#_conditional_includes --- git/config.py | 8 ++++++-- git/repo/base.py | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/git/config.py b/git/config.py index e686dcd3e..6d4135f54 100644 --- a/git/config.py +++ b/git/config.py @@ -250,7 +250,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=None, read_only=True, merge_includes=True, config_level=None): + def __init__(self, file_or_files=None, read_only=True, merge_includes=True, config_level=None, repo=None): """Initialize a configuration reader to read the given file_or_files and to possibly allow changes to it by setting read_only False @@ -265,7 +265,10 @@ def __init__(self, file_or_files=None, read_only=True, merge_includes=True, conf :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 configuration file, turn this off to leave the original - dataset unaltered when reading it.""" + dataset unaltered when reading it. + :param repo: Reference to repository to use if [includeIf] sections are found in configuration files. + + """ cp.RawConfigParser.__init__(self, dict_type=_OMD) # Used in python 3, needs to stay in sync with sections for underlying implementation to work @@ -287,6 +290,7 @@ def __init__(self, file_or_files=None, read_only=True, merge_includes=True, conf self._dirty = False self._is_initialized = False self._merge_includes = merge_includes + self._repo = repo self._lock = None self._acquire_lock() diff --git a/git/repo/base.py b/git/repo/base.py index 591ccec52..2579a7d7a 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -452,7 +452,7 @@ def config_reader(self, config_level=None): files = [self._get_config_path(f) for f in self.config_level] else: files = [self._get_config_path(config_level)] - return GitConfigParser(files, read_only=True) + return GitConfigParser(files, read_only=True, repo=self) def config_writer(self, config_level="repository"): """ @@ -467,7 +467,7 @@ def config_writer(self, config_level="repository"): system = system wide configuration file global = user level configuration file repository = configuration file for this repostory only""" - return GitConfigParser(self._get_config_path(config_level), read_only=False) + return GitConfigParser(self._get_config_path(config_level), read_only=False, repo=self) def commit(self, rev=None): """The Commit object for the specified revision From 8e263b8f972f78c673f36f2bbc1f8563ce6acb10 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 14:05:06 -0700 Subject: [PATCH 04/12] Add method to retrieve all possible paths to include --- git/config.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index 6d4135f54..0618c8ac4 100644 --- a/git/config.py +++ b/git/config.py @@ -13,6 +13,8 @@ import logging import os import re +import glob +import fnmatch from collections import OrderedDict from git.compat import ( @@ -455,6 +457,39 @@ def _has_includes(self): for section in self.sections() ) + def _included_paths(self): + """Return all paths that must be included to configuration. + """ + paths = [] + + for section in self.sections(): + if section == "include": + paths += self.items(section) + + match = CONDITIONAL_INCLUDE_REGEXP.search(section) + if match is not None and self._repo is not None: + keyword = match.group(1) + value = match.group(2).strip() + + if keyword in ["gitdir", "gitdir/i"]: + value = osp.expanduser(value) + flags = [re.IGNORECASE] if keyword == "gitdir/i" else [] + regexp = re.compile(fnmatch.translate(value), *flags) + + if any( + regexp.match(path) is not None + and self._repo.git_dir.startswith(path) + for path in glob.glob(value) + ): + paths += self.items(section) + + + elif keyword == "onbranch": + if value == self._repo.active_branch.name: + paths += self.items(section) + + return paths + 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 @@ -492,7 +527,7 @@ def read(self): # 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'): + for _, include_path in self._included_paths(): if include_path.startswith('~'): include_path = osp.expanduser(include_path) if not osp.isabs(include_path): From f37011d7627e4a46cff26f07ea7ade48b284edee Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 17:14:14 -0700 Subject: [PATCH 05/12] Fix logic to properly compare glob pattern to value --- git/config.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/git/config.py b/git/config.py index 0618c8ac4..e36af9fe2 100644 --- a/git/config.py +++ b/git/config.py @@ -13,7 +13,6 @@ import logging import os import re -import glob import fnmatch from collections import OrderedDict @@ -452,10 +451,7 @@ def string_decode(v): raise e def _has_includes(self): - return self._merge_includes and any( - section == 'include' or section.startswith('includeIf ') - for section in self.sections() - ) + return self._merge_includes and len(self._included_paths()) def _included_paths(self): """Return all paths that must be included to configuration. @@ -471,21 +467,26 @@ def _included_paths(self): keyword = match.group(1) value = match.group(2).strip() - if keyword in ["gitdir", "gitdir/i"]: + if keyword == "gitdir": value = osp.expanduser(value) - flags = [re.IGNORECASE] if keyword == "gitdir/i" else [] - regexp = re.compile(fnmatch.translate(value), *flags) - - if any( - regexp.match(path) is not None - and self._repo.git_dir.startswith(path) - for path in glob.glob(value) - ): + if fnmatch.fnmatchcase(self._repo.git_dir, value): paths += self.items(section) + elif keyword == "gitdir/i": + value = osp.expanduser(value) + + # Ensure that glob is always case insensitive. + value = re.sub( + r"[a-zA-Z]", + lambda m: f"[{m.group().lower()}{m.group().upper()}]", + value + ) + + if fnmatch.fnmatchcase(self._repo.git_dir, value): + paths += self.items(section) elif keyword == "onbranch": - if value == self._repo.active_branch.name: + if fnmatch.fnmatchcase(self._repo.active_branch.name, value): paths += self.items(section) return paths From 3dcb520caa069914f9ab014798ab321730f569cc Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 17:16:43 -0700 Subject: [PATCH 06/12] Add unit tests --- test/test_config.py | 99 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/test/test_config.py b/test/test_config.py index 720d775ee..12fad15cc 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -6,6 +6,8 @@ import glob import io +import os +from unittest import mock from git import ( GitConfigParser @@ -238,6 +240,103 @@ def check_test_value(cr, value): with GitConfigParser(fpa, read_only=True) as cr: check_test_value(cr, tv) + @with_rw_directory + def test_conditional_includes_from_git_dir(self, rw_dir): + # Initiate repository path + git_dir = osp.join(rw_dir, "target1", "repo1") + os.makedirs(git_dir) + + # Initiate mocked repository + repo = mock.Mock(git_dir=git_dir) + + # Initiate config files. + path1 = osp.join(rw_dir, "config1") + path2 = osp.join(rw_dir, "config2") + template = "[includeIf \"{}:{}\"]\n path={}\n" + + with open(path1, "w") as stream: + stream.write(template.format("gitdir", git_dir, path2)) + + # Ensure that config is ignored if no repo is set. + with GitConfigParser(path1) as config: + assert not config._has_includes() + assert config._included_paths() == [] + + # Ensure that config is included if path is matching git_dir. + with GitConfigParser(path1, repo=repo) as config: + assert config._has_includes() + assert config._included_paths() == [("path", path2)] + + # Ensure that config is ignored if case is incorrect. + with open(path1, "w") as stream: + stream.write(template.format("gitdir", git_dir.upper(), path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert not config._has_includes() + assert config._included_paths() == [] + + # Ensure that config is included if case is ignored. + with open(path1, "w") as stream: + stream.write(template.format("gitdir/i", git_dir.upper(), path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert config._has_includes() + assert config._included_paths() == [("path", path2)] + + # Ensure that config is included with path using glob pattern. + with open(path1, "w") as stream: + stream.write(template.format("gitdir", "**/repo1", path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert config._has_includes() + assert config._included_paths() == [("path", path2)] + + # Ensure that config is ignored if path is not matching git_dir. + with open(path1, "w") as stream: + stream.write(template.format("gitdir", "incorrect", path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert not config._has_includes() + assert config._included_paths() == [] + + @with_rw_directory + def test_conditional_includes_from_branch_name(self, rw_dir): + # Initiate mocked branch + branch = mock.Mock() + type(branch).name = mock.PropertyMock(return_value="/foo/branch") + + # Initiate mocked repository + repo = mock.Mock(active_branch=branch) + + # Initiate config files. + path1 = osp.join(rw_dir, "config1") + path2 = osp.join(rw_dir, "config2") + template = "[includeIf \"onbranch:{}\"]\n path={}\n" + + # Ensure that config is included is branch is correct. + with open(path1, "w") as stream: + stream.write(template.format("/foo/branch", path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert config._has_includes() + assert config._included_paths() == [("path", path2)] + + # Ensure that config is included is branch is incorrect. + with open(path1, "w") as stream: + stream.write(template.format("incorrect", path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert not config._has_includes() + assert config._included_paths() == [] + + # Ensure that config is included with branch using glob pattern. + with open(path1, "w") as stream: + stream.write(template.format("/foo/**", path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert config._has_includes() + assert config._included_paths() == [("path", path2)] + def test_rename(self): file_obj = self._to_memcache(fixture_path('git_config')) with GitConfigParser(file_obj, read_only=False, merge_includes=False) as cw: From 16d3ebfa3ad32d281ebdd77de587251015d04b3b Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 17:18:24 -0700 Subject: [PATCH 07/12] Update AUTHOR to respect to contributing guidelines. --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 7b21b2b26..f4e46ba31 100644 --- a/AUTHORS +++ b/AUTHORS @@ -43,4 +43,5 @@ Contributors are: -Liam Beguin -Ram Rachum -Alba Mendez +-Jeremy Retailleau Portions derived from other open source works and are clearly marked. From c3fc83f2333eaee5fbcbef6df9f4ed9eb320fd11 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 17:38:43 -0700 Subject: [PATCH 08/12] Add missing rules to match hierarchy path --- git/config.py | 28 ++++++++++++++++------------ test/test_config.py | 8 ++++++++ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/git/config.py b/git/config.py index e36af9fe2..eb46c41ba 100644 --- a/git/config.py +++ b/git/config.py @@ -467,20 +467,24 @@ def _included_paths(self): keyword = match.group(1) value = match.group(2).strip() - if keyword == "gitdir": - value = osp.expanduser(value) - if fnmatch.fnmatchcase(self._repo.git_dir, value): - paths += self.items(section) - - elif keyword == "gitdir/i": + if keyword in ["gitdir", "gitdir/i"]: value = osp.expanduser(value) - # Ensure that glob is always case insensitive. - value = re.sub( - r"[a-zA-Z]", - lambda m: f"[{m.group().lower()}{m.group().upper()}]", - value - ) + if not any(value.startswith(s) for s in ["./", "/"]): + value = "**/" + value + if value.endswith("/"): + value += "**" + + # Ensure that glob is always case insensitive if required. + if keyword.endswith("/i"): + value = re.sub( + r"[a-zA-Z]", + lambda m: "[{}{}]".format( + m.group().lower(), + m.group().upper() + ), + value + ) if fnmatch.fnmatchcase(self._repo.git_dir, value): paths += self.items(section) diff --git a/test/test_config.py b/test/test_config.py index 12fad15cc..406d794ee 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -299,6 +299,14 @@ def test_conditional_includes_from_git_dir(self, rw_dir): assert not config._has_includes() assert config._included_paths() == [] + # Ensure that config is included if path in hierarchy. + with open(path1, "w") as stream: + stream.write(template.format("gitdir", "target1/", path2)) + + with GitConfigParser(path1, repo=repo) as config: + assert config._has_includes() + assert config._included_paths() == [("path", path2)] + @with_rw_directory def test_conditional_includes_from_branch_name(self, rw_dir): # Initiate mocked branch From c3eae2cdd688d8969a4f36b96a8b41fa55c0d3ed Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Wed, 2 Sep 2020 17:44:47 -0700 Subject: [PATCH 09/12] Add missing blank line --- git/config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/git/config.py b/git/config.py index eb46c41ba..f9a5a46bc 100644 --- a/git/config.py +++ b/git/config.py @@ -43,6 +43,7 @@ # https://git-scm.com/docs/git-config#_conditional_includes CONDITIONAL_INCLUDE_REGEXP = re.compile(r"(?<=includeIf )\"(gitdir|gitdir/i|onbranch):(.+)\"") + class MetaParserBuilder(abc.ABCMeta): """Utlity class wrapping base-class methods into decorators that assure read-only properties""" From 2c4dec45d387ccebfe9bd423bc8e633210d3cdbd Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Thu, 3 Sep 2020 09:40:51 -0700 Subject: [PATCH 10/12] Remove name as not necessary to track down authors. --- AUTHORS | 1 - 1 file changed, 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index f4e46ba31..7b21b2b26 100644 --- a/AUTHORS +++ b/AUTHORS @@ -43,5 +43,4 @@ Contributors are: -Liam Beguin -Ram Rachum -Alba Mendez --Jeremy Retailleau Portions derived from other open source works and are clearly marked. From 3787f622e59c2fecfa47efc114c409f51a27bbe7 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Thu, 3 Sep 2020 11:11:34 -0700 Subject: [PATCH 11/12] Reformat code to remove unnecessary indentation --- git/config.py | 60 ++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/git/config.py b/git/config.py index f9a5a46bc..b49f0790e 100644 --- a/git/config.py +++ b/git/config.py @@ -464,35 +464,37 @@ def _included_paths(self): paths += self.items(section) match = CONDITIONAL_INCLUDE_REGEXP.search(section) - if match is not None and self._repo is not None: - keyword = match.group(1) - value = match.group(2).strip() - - if keyword in ["gitdir", "gitdir/i"]: - value = osp.expanduser(value) - - if not any(value.startswith(s) for s in ["./", "/"]): - value = "**/" + value - if value.endswith("/"): - value += "**" - - # Ensure that glob is always case insensitive if required. - if keyword.endswith("/i"): - value = re.sub( - r"[a-zA-Z]", - lambda m: "[{}{}]".format( - m.group().lower(), - m.group().upper() - ), - value - ) - - if fnmatch.fnmatchcase(self._repo.git_dir, value): - paths += self.items(section) - - elif keyword == "onbranch": - if fnmatch.fnmatchcase(self._repo.active_branch.name, value): - paths += self.items(section) + if match is None or self._repo is None: + continue + + keyword = match.group(1) + value = match.group(2).strip() + + if keyword in ["gitdir", "gitdir/i"]: + value = osp.expanduser(value) + + if not any(value.startswith(s) for s in ["./", "/"]): + value = "**/" + value + if value.endswith("/"): + value += "**" + + # Ensure that glob is always case insensitive if required. + if keyword.endswith("/i"): + value = re.sub( + r"[a-zA-Z]", + lambda m: "[{}{}]".format( + m.group().lower(), + m.group().upper() + ), + value + ) + + if fnmatch.fnmatchcase(self._repo.git_dir, value): + paths += self.items(section) + + elif keyword == "onbranch": + if fnmatch.fnmatchcase(self._repo.active_branch.name, value): + paths += self.items(section) return paths From 5b88532a5346a9a7e8f0e45fec14632a9bfe2c89 Mon Sep 17 00:00:00 2001 From: Jeremy Retailleau Date: Thu, 3 Sep 2020 11:24:10 -0700 Subject: [PATCH 12/12] Ensure that detached HEAD does not raise when comparing branch name. --- git/config.py | 8 +++++++- test/test_config.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index b49f0790e..9f09efe2b 100644 --- a/git/config.py +++ b/git/config.py @@ -493,7 +493,13 @@ def _included_paths(self): paths += self.items(section) elif keyword == "onbranch": - if fnmatch.fnmatchcase(self._repo.active_branch.name, value): + try: + branch_name = self._repo.active_branch.name + except TypeError: + # Ignore section if active branch cannot be retrieved. + continue + + if fnmatch.fnmatchcase(branch_name, value): paths += self.items(section) return paths diff --git a/test/test_config.py b/test/test_config.py index 406d794ee..8892b8399 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -345,6 +345,23 @@ def test_conditional_includes_from_branch_name(self, rw_dir): assert config._has_includes() assert config._included_paths() == [("path", path2)] + @with_rw_directory + def test_conditional_includes_from_branch_name_error(self, rw_dir): + # Initiate mocked repository to raise an error if HEAD is detached. + repo = mock.Mock() + type(repo).active_branch = mock.PropertyMock(side_effect=TypeError) + + # Initiate config file. + path1 = osp.join(rw_dir, "config1") + + # Ensure that config is ignored when active branch cannot be found. + with open(path1, "w") as stream: + stream.write("[includeIf \"onbranch:foo\"]\n path=/path\n") + + with GitConfigParser(path1, repo=repo) as config: + assert not config._has_includes() + assert config._included_paths() == [] + def test_rename(self): file_obj = self._to_memcache(fixture_path('git_config')) with GitConfigParser(file_obj, read_only=False, merge_includes=False) as cw: