diff --git a/gclient_scm.py b/gclient_scm.py index 3563eacde..0b44253f8 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -677,27 +677,9 @@ class GitWrapper(SCMWrapper): config_updates.append( ('blame.ignoreRevsFile', '.git-blame-ignore-revs')) - ignore_submodules = scm.GIT.GetConfig(args[0].checkout_path, - 'diff.ignoresubmodules', - None, 'local') - - if not ignore_submodules: + if scm.GIT.GetConfig(args[0].checkout_path, + 'diff.ignoresubmodules') != 'dirty': config_updates.append(('diff.ignoreSubmodules', 'dirty')) - elif ignore_submodules != 'dirty': - warning_message = ( - "diff.ignoreSubmodules is not set to 'dirty' " - "for this repository.\n" - "This may cause unexpected behavior with submodules; " - "see //docs/git_submodules.md\n" - "Consider setting the config:\n" - "\tgit config diff.ignoreSubmodule dirty\n" - "or disable this warning by setting the " - "GCLIENT_SUPPRESS_SUBMODULE_WARNING environment " - "variable to 1.") - if os.environ.get( - 'GCLIENT_SUPPRESS_SUBMODULE_WARNING') != '1': - gclient_utils.AddWarning(warning_message) - if scm.GIT.GetConfig(args[0].checkout_path, 'fetch.recursesubmodules') != 'off': diff --git a/scm.py b/scm.py index bea6483cd..a1ff59fe4 100644 --- a/scm.py +++ b/scm.py @@ -235,30 +235,17 @@ class CachedGitConfigState(object): def GetConfig(self, key: str, - default: Optional[str] = None, - scope: Optional[str] = None) -> Optional[str]: + default: Optional[str] = None) -> Optional[str]: """Lazily loads all configration observable for this CachedGitConfigState, then returns the last value for `key` as a string. If `key` is missing, returns default. """ - key = canonicalize_git_config_key(key) - if not scope: - scope = "local" - - scoped_config = self._maybe_load_config() - if not scoped_config: - return default - - scoped_config = scoped_config.get(scope, None) - if not scoped_config: - return default - - values = scoped_config.get(key, None) - + values = self._maybe_load_config().get(key, None) if not values: return default + return values[-1] def GetConfigBool(self, key: str) -> bool: @@ -272,7 +259,7 @@ class CachedGitConfigState(object): def GetConfigList(self, key: str) -> list[str]: """Returns all values of `key` as a list of strings.""" key = canonicalize_git_config_key(key) - return list(self._maybe_load_config().get('default', {}).get(key, ())) + return list(self._maybe_load_config().get(key, ())) def YieldConfigRegexp(self, pattern: Optional[str] = None @@ -291,8 +278,7 @@ class CachedGitConfigState(object): pred = lambda _: True else: pred = re.compile(pattern).match - for key, values in sorted(self._maybe_load_config().get('default', - {}).items()): + for key, values in sorted(self._maybe_load_config().items()): if pred(key): for value in values: yield key, value @@ -387,36 +373,19 @@ class GitConfigStateReal(GitConfigStateBase): def load_config(self) -> GitFlatConfigData: # NOTE: `git config --list` already canonicalizes keys. try: - rawConfig = GIT.Capture(['config', '--list', '-z', '--show-scope'], + rawConfig = GIT.Capture(['config', '--list', '-z'], cwd=self.root, strip_out=False) except subprocess2.CalledProcessError: return {} assert isinstance(rawConfig, str) - cfg: Dict[str, Dict[str, - List[str]]] = defaultdict(lambda: defaultdict(list)) - - entries = rawConfig.split('\x00')[:-1] - - def process_entry(entry: str, scope: str) -> None: - parts = entry.split('\n', 1) - key, value = parts if len(parts) == 2 else (parts[0], '') - key, value = key.strip(), value.strip() - cfg[scope][key].append(value) - if scope != "default": - cfg["default"][key].append(value) - - i = 0 - while i < len(entries): - if entries[i] in ['local', 'global', 'system']: - scope = entries[i] - i += 1 - if i < len(entries): - process_entry(entries[i], scope) - else: - process_entry(entries[i], "default") - i += 1 + cfg: Dict[str, list[str]] = defaultdict(list) + + # Splitting by '\x00' gets an additional empty string at the end. + for line in rawConfig.split('\x00')[:-1]: + key, value = map(str.strip, line.split('\n', 1)) + cfg[key].append(value) return cfg @@ -535,20 +504,18 @@ class GitConfigStateTest(GitConfigStateBase): raise GitConfigUnknownScope(scope) def load_config(self) -> GitFlatConfigData: - cfg: Dict[str, Dict[str, - List[str]]] = defaultdict(lambda: defaultdict(list)) - - for key, values in self.system_state.items(): - cfg['system'][key].extend(values) - cfg['default'][key].extend(values) - for ordered_scope in GitScopeOrder: - if ordered_scope == 'system': + ret = {k: list(v) for k, v in self.system_state.items()} + for scope in GitScopeOrder: + if scope == 'system': continue - with self._editable_scope(ordered_scope) as scope_cfg: - for key, values in scope_cfg.items(): - cfg[ordered_scope][key].extend(values) - cfg['default'][key].extend(values) - return cfg + with self._editable_scope(scope) as cfg: + for key, value in cfg.items(): + curvals = ret.get(key, None) + if curvals is None: + curvals = [] + ret[key] = curvals + curvals.extend(value) + return ret def set_config(self, key: str, value: str, *, append: bool, scope: GitConfigScope): @@ -677,8 +644,7 @@ class GIT(object): state = {} for key, val in cls._CONFIG_CACHE.items(): if val is not None: - state[str(key)] = val._maybe_load_config().get( - 'default', {}) + state[str(key)] = val._maybe_load_config() return state @staticmethod @@ -748,14 +714,13 @@ class GIT(object): @staticmethod def GetConfig(cwd: str, key: str, - default: Optional[str] = None, - scope: Optional[str] = None) -> Optional[str]: + default: Optional[str] = None) -> Optional[str]: """Lazily loads all configration observable for this CachedGitConfigState, then returns the last value for `key` as a string. If `key` is missing, returns default. """ - return GIT._get_config_state(cwd).GetConfig(key, default, scope) + return GIT._get_config_state(cwd).GetConfig(key, default) @staticmethod def GetConfigBool(cwd: str, key: str) -> bool: diff --git a/tests/gclient_git_smoketest.py b/tests/gclient_git_smoketest.py index 47ad0f16b..51a07af48 100755 --- a/tests/gclient_git_smoketest.py +++ b/tests/gclient_git_smoketest.py @@ -30,7 +30,6 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase): super(GClientSmokeGIT, self).setUp() self.env['PATH'] = (os.path.join(ROOT_DIR, 'testing_support') + os.pathsep + self.env['PATH']) - self.env['GCLIENT_SUPPRESS_SUBMODULE_WARNING'] = '1' self.enabled = self.FAKE_REPOS.set_up_git() if not self.enabled: self.skipTest('git fake repos not available') diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 63a10d442..631de5c67 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -396,38 +396,26 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - expected_file_list = [join(self.base_path, x) for x in ['a', 'b']] + expected_file_list = [ + join(self.base_path, x) for x in ['a', 'b', 'submodule'] + ] git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) file_list = [] - # Set diff.ignoreSubmodules to something other than 'dirty' - git_wrapper._Run(['config', 'diff.ignoreSubmodules', 'all'], options) git_wrapper.update(options, (), file_list) - expected_warning = "diff.ignoreSubmodules is not set to 'dirty'" - self.assertTrue( - any(expected_warning in w for w in gclient_utils._WARNINGS), - f"Expected warning not found. " - f"New warnings: {gclient_utils._WARNINGS}") self.assertEqual(file_list, expected_file_list) self.assertEqual(git_wrapper.revinfo(options, (), None), '4091c7d010ca99d0f2dd416d4b70b758ae432187') self.assertEqual( git_wrapper._Capture(['config', '--get', 'diff.ignoreSubmodules']), - 'all') + 'dirty') self.assertEqual( git_wrapper._Capture(['config', '--get', 'fetch.recurseSubmodules']), 'off') self.assertEqual( git_wrapper._Capture(['config', '--get', 'push.recurseSubmodules']), 'off') - os.environ['GCLIENT_SUPPRESS_SUBMODULE_WARNING'] = '1' - gclient_utils._WARNINGS.clear() - git_wrapper.update(options, (), file_list) - self.assertEqual(len(gclient_utils._WARNINGS), 0, - "Warning was added despite being suppressed") - # Clean up - del os.environ['GCLIENT_SUPPRESS_SUBMODULE_WARNING'] sys.stdout.close() def testUpdateMerge(self): diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 54b52b33e..23dadf47b 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -11,7 +11,6 @@ import os import sys import tempfile import threading -from collections import defaultdict import unittest from unittest import mock @@ -450,82 +449,28 @@ class GitConfigStateTestTest(unittest.TestCase): self.assertDictEqual(m.load_config(), {}) gs['section.key'] = ['override'] - self.assertDictEqual( - m.load_config(), { - "global": { - 'section.key': ['override'] - }, - "default": { - 'section.key': ['override'] - }, - }) - - def defaultdict_to_dict(self, d): - if isinstance(d, defaultdict): - return {k: self.defaultdict_to_dict(v) for k, v in d.items()} - return d + self.assertDictEqual(m.load_config(), {'section.key': ['override']}) def test_construction_global(self): - - m, gs = self._make(global_state={ - 'section.key': ['global'], - }) - self.assertDictEqual(self.defaultdict_to_dict(gs), - {'section.key': ['global']}) - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - "global": { - 'section.key': ['global'] - }, - "default": { - 'section.key': ['global'] - }, - }) + m, gs = self._make(global_state={'section.key': ['global']}) + self.assertDictEqual(gs, {'section.key': ['global']}) + self.assertDictEqual(m.load_config(), {'section.key': ['global']}) gs['section.key'] = ['override'] - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - "global": { - 'section.key': ['override'] - }, - "default": { - 'section.key': ['override'] - }, - }) + self.assertDictEqual(m.load_config(), {'section.key': ['override']}) def test_construction_system(self): m, gs = self._make( global_state={'section.key': ['global']}, system_state={'section.key': ['system']}, ) - self.assertDictEqual(self.defaultdict_to_dict(gs), - {'section.key': ['global']}) - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - 'default': { - 'section.key': ['system', 'global'] - }, - "global": { - 'section.key': ['global'] - }, - "system": { - 'section.key': ['system'] - } - }) + self.assertDictEqual(gs, {'section.key': ['global']}) + self.assertDictEqual(m.load_config(), + {'section.key': ['system', 'global']}) gs['section.key'] = ['override'] - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - "global": { - 'section.key': ['override'] - }, - "system": { - 'section.key': ['system'] - }, - 'default': { - 'section.key': ['system', 'override'] - } - }) + self.assertDictEqual(m.load_config(), + {'section.key': ['system', 'override']}) def test_set_config_system(self): m, _ = self._make() @@ -551,15 +496,9 @@ class GitConfigStateTestTest(unittest.TestCase): self.assertDictEqual(m.load_config(), {}) m.set_config('section.key', 'new_global', append=True, scope='global') - self.assertDictEqual( - m.load_config(), { - "default": { - 'section.key': ['new_global'] - }, - "global": { - 'section.key': ['new_global'] - } - }) + self.assertDictEqual(m.load_config(), { + 'section.key': ['new_global'], + }) def test_set_config_global(self): m, gs = self._make() @@ -567,64 +506,44 @@ class GitConfigStateTestTest(unittest.TestCase): self.assertDictEqual(m.load_config(), {}) m.set_config('section.key', 'new_global', append=False, scope='global') - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - "global": { - 'section.key': ['new_global'] - }, - "default": { - 'section.key': ['new_global'] - } - }) + self.assertDictEqual(m.load_config(), { + 'section.key': ['new_global'], + }) m.set_config('section.key', 'new_global2', append=True, scope='global') - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - "global": { - 'section.key': ['new_global', 'new_global2'] - }, - "default": { - 'section.key': ['new_global', 'new_global2'] - }, - }) - - self.assertDictEqual(self.defaultdict_to_dict(gs), - {'section.key': ['new_global', 'new_global2']}) + self.assertDictEqual(m.load_config(), { + 'section.key': ['new_global', 'new_global2'], + }) + + self.assertDictEqual(gs, { + 'section.key': ['new_global', 'new_global2'], + }) def test_set_config_multi_global(self): - m, gs = self._make(global_state={'section.key': ['1', '2']}) + m, gs = self._make(global_state={ + 'section.key': ['1', '2'], + }) m.set_config_multi('section.key', 'new_global', value_pattern=None, scope='global') - self.assertDictEqual( - self.defaultdict_to_dict(m.load_config()), { - "default": { - 'section.key': ['new_global'] - }, - "global": { - 'section.key': ['new_global'] - } - }) - - self.assertDictEqual(gs, {'section.key': ['new_global']}) + self.assertDictEqual(m.load_config(), { + 'section.key': ['new_global'], + }) + + self.assertDictEqual(gs, { + 'section.key': ['new_global'], + }) m.set_config_multi('othersection.key', 'newval', value_pattern=None, scope='global') - self.assertDictEqual( - m.load_config(), { - "global": { - 'section.key': ['new_global'], - 'othersection.key': ['newval'], - }, - "default": { - 'section.key': ['new_global'], - 'othersection.key': ['newval'], - } - }) + self.assertDictEqual(m.load_config(), { + 'section.key': ['new_global'], + 'othersection.key': ['newval'], + }) self.assertDictEqual(gs, { 'section.key': ['new_global'], @@ -640,29 +559,17 @@ class GitConfigStateTestTest(unittest.TestCase): 'new_global', value_pattern='2', scope='global') - self.assertDictEqual( - m.load_config(), { - "global": { - 'section.key': ['1', '1', 'new_global', '3'] - }, - "default": { - 'section.key': ['1', '1', 'new_global', '3'] - } - }) + self.assertDictEqual(m.load_config(), { + 'section.key': ['1', '1', 'new_global', '3'], + }) m.set_config_multi('section.key', 'additional', value_pattern='narp', scope='global') - self.assertDictEqual( - m.load_config(), { - "default": { - 'section.key': ['1', '1', 'new_global', '3', 'additional'] - }, - "global": { - 'section.key': ['1', '1', 'new_global', '3', 'additional'] - } - }) + self.assertDictEqual(m.load_config(), { + 'section.key': ['1', '1', 'new_global', '3', 'additional'], + }) def test_unset_config_global(self): m, _ = self._make(global_state={ @@ -688,34 +595,19 @@ class GitConfigStateTestTest(unittest.TestCase): m.unset_config('section.key', scope='global', missing_ok=False) self.assertDictEqual(m.load_config(), { - "global": { - 'extra': ['another'] - }, - "default": { - 'extra': ['another'] - } + 'extra': ['another'], }) with self.assertRaises(scm.GitConfigUnsetMissingValue): m.unset_config('section.key', scope='global', missing_ok=False) self.assertDictEqual(m.load_config(), { - "global": { - 'extra': ['another'] - }, - "default": { - 'extra': ['another'] - } + 'extra': ['another'], }) m.unset_config('section.key', scope='global', missing_ok=True) self.assertDictEqual(m.load_config(), { - "global": { - 'extra': ['another'] - }, - "default": { - 'extra': ['another'] - } + 'extra': ['another'], }) def test_unset_config_global_multi(self): @@ -752,15 +644,9 @@ class GitConfigStateTestTest(unittest.TestCase): value_pattern='2', scope='global', missing_ok=False) - self.assertDictEqual( - m.load_config(), { - 'global': { - 'section.key': ['1', '3', '1'], - }, - 'default': { - 'section.key': ['1', '3', '1'], - } - }) + self.assertDictEqual(m.load_config(), { + 'section.key': ['1', '3', '1'], + }) class CanonicalizeGitConfigKeyTest(unittest.TestCase):