diff --git a/gclient_scm.py b/gclient_scm.py index 0b44253f8..3563eacde 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -677,9 +677,27 @@ class GitWrapper(SCMWrapper): config_updates.append( ('blame.ignoreRevsFile', '.git-blame-ignore-revs')) - if scm.GIT.GetConfig(args[0].checkout_path, - 'diff.ignoresubmodules') != 'dirty': + ignore_submodules = scm.GIT.GetConfig(args[0].checkout_path, + 'diff.ignoresubmodules', + None, 'local') + + if not ignore_submodules: 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 a1ff59fe4..bea6483cd 100644 --- a/scm.py +++ b/scm.py @@ -235,17 +235,30 @@ class CachedGitConfigState(object): def GetConfig(self, key: str, - default: Optional[str] = None) -> Optional[str]: + default: Optional[str] = None, + scope: 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) - values = self._maybe_load_config().get(key, None) - if not values: + 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) + + if not values: + return default return values[-1] def GetConfigBool(self, key: str) -> bool: @@ -259,7 +272,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(key, ())) + return list(self._maybe_load_config().get('default', {}).get(key, ())) def YieldConfigRegexp(self, pattern: Optional[str] = None @@ -278,7 +291,8 @@ class CachedGitConfigState(object): pred = lambda _: True else: pred = re.compile(pattern).match - for key, values in sorted(self._maybe_load_config().items()): + for key, values in sorted(self._maybe_load_config().get('default', + {}).items()): if pred(key): for value in values: yield key, value @@ -373,19 +387,36 @@ class GitConfigStateReal(GitConfigStateBase): def load_config(self) -> GitFlatConfigData: # NOTE: `git config --list` already canonicalizes keys. try: - rawConfig = GIT.Capture(['config', '--list', '-z'], + rawConfig = GIT.Capture(['config', '--list', '-z', '--show-scope'], cwd=self.root, strip_out=False) except subprocess2.CalledProcessError: return {} assert isinstance(rawConfig, str) - 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) + 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 return cfg @@ -504,18 +535,20 @@ class GitConfigStateTest(GitConfigStateBase): raise GitConfigUnknownScope(scope) def load_config(self) -> GitFlatConfigData: - ret = {k: list(v) for k, v in self.system_state.items()} - for scope in GitScopeOrder: - if scope == 'system': + 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': continue - 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 + 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 def set_config(self, key: str, value: str, *, append: bool, scope: GitConfigScope): @@ -644,7 +677,8 @@ class GIT(object): state = {} for key, val in cls._CONFIG_CACHE.items(): if val is not None: - state[str(key)] = val._maybe_load_config() + state[str(key)] = val._maybe_load_config().get( + 'default', {}) return state @staticmethod @@ -714,13 +748,14 @@ class GIT(object): @staticmethod def GetConfig(cwd: str, key: str, - default: Optional[str] = None) -> Optional[str]: + default: Optional[str] = None, + scope: 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) + return GIT._get_config_state(cwd).GetConfig(key, default, scope) @staticmethod def GetConfigBool(cwd: str, key: str) -> bool: diff --git a/tests/gclient_git_smoketest.py b/tests/gclient_git_smoketest.py index 51a07af48..47ad0f16b 100755 --- a/tests/gclient_git_smoketest.py +++ b/tests/gclient_git_smoketest.py @@ -30,6 +30,7 @@ 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 631de5c67..63a10d442 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -396,26 +396,38 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - expected_file_list = [ - join(self.base_path, x) for x in ['a', 'b', 'submodule'] - ] + expected_file_list = [join(self.base_path, x) for x in ['a', 'b']] 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']), - 'dirty') + 'all') 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 23dadf47b..54b52b33e 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -11,6 +11,7 @@ import os import sys import tempfile import threading +from collections import defaultdict import unittest from unittest import mock @@ -449,28 +450,82 @@ class GitConfigStateTestTest(unittest.TestCase): self.assertDictEqual(m.load_config(), {}) gs['section.key'] = ['override'] - self.assertDictEqual(m.load_config(), {'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 def test_construction_global(self): - m, gs = self._make(global_state={'section.key': ['global']}) - self.assertDictEqual(gs, {'section.key': ['global']}) - self.assertDictEqual(m.load_config(), {'section.key': ['global']}) + + 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'] + }, + }) gs['section.key'] = ['override'] - self.assertDictEqual(m.load_config(), {'section.key': ['override']}) + self.assertDictEqual( + self.defaultdict_to_dict(m.load_config()), { + "global": { + 'section.key': ['override'] + }, + "default": { + 'section.key': ['override'] + }, + }) def test_construction_system(self): m, gs = self._make( global_state={'section.key': ['global']}, system_state={'section.key': ['system']}, ) - self.assertDictEqual(gs, {'section.key': ['global']}) - self.assertDictEqual(m.load_config(), - {'section.key': ['system', 'global']}) + 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'] + } + }) gs['section.key'] = ['override'] - self.assertDictEqual(m.load_config(), - {'section.key': ['system', 'override']}) + self.assertDictEqual( + self.defaultdict_to_dict(m.load_config()), { + "global": { + 'section.key': ['override'] + }, + "system": { + 'section.key': ['system'] + }, + 'default': { + 'section.key': ['system', 'override'] + } + }) def test_set_config_system(self): m, _ = self._make() @@ -496,9 +551,15 @@ 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(), { - 'section.key': ['new_global'], - }) + self.assertDictEqual( + m.load_config(), { + "default": { + 'section.key': ['new_global'] + }, + "global": { + 'section.key': ['new_global'] + } + }) def test_set_config_global(self): m, gs = self._make() @@ -506,44 +567,64 @@ class GitConfigStateTestTest(unittest.TestCase): self.assertDictEqual(m.load_config(), {}) m.set_config('section.key', 'new_global', append=False, scope='global') - self.assertDictEqual(m.load_config(), { - 'section.key': ['new_global'], - }) + self.assertDictEqual( + self.defaultdict_to_dict(m.load_config()), { + "global": { + 'section.key': ['new_global'] + }, + "default": { + 'section.key': ['new_global'] + } + }) m.set_config('section.key', 'new_global2', append=True, scope='global') - self.assertDictEqual(m.load_config(), { - 'section.key': ['new_global', 'new_global2'], - }) - - self.assertDictEqual(gs, { - 'section.key': ['new_global', 'new_global2'], - }) + 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']}) 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(m.load_config(), { - 'section.key': ['new_global'], - }) - - self.assertDictEqual(gs, { - 'section.key': ['new_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']}) m.set_config_multi('othersection.key', 'newval', value_pattern=None, scope='global') - self.assertDictEqual(m.load_config(), { - 'section.key': ['new_global'], - 'othersection.key': ['newval'], - }) + self.assertDictEqual( + m.load_config(), { + "global": { + 'section.key': ['new_global'], + 'othersection.key': ['newval'], + }, + "default": { + 'section.key': ['new_global'], + 'othersection.key': ['newval'], + } + }) self.assertDictEqual(gs, { 'section.key': ['new_global'], @@ -559,17 +640,29 @@ class GitConfigStateTestTest(unittest.TestCase): 'new_global', value_pattern='2', scope='global') - self.assertDictEqual(m.load_config(), { - 'section.key': ['1', '1', 'new_global', '3'], - }) + self.assertDictEqual( + m.load_config(), { + "global": { + 'section.key': ['1', '1', 'new_global', '3'] + }, + "default": { + 'section.key': ['1', '1', 'new_global', '3'] + } + }) m.set_config_multi('section.key', 'additional', value_pattern='narp', scope='global') - self.assertDictEqual(m.load_config(), { - 'section.key': ['1', '1', 'new_global', '3', 'additional'], - }) + self.assertDictEqual( + m.load_config(), { + "default": { + 'section.key': ['1', '1', 'new_global', '3', 'additional'] + }, + "global": { + 'section.key': ['1', '1', 'new_global', '3', 'additional'] + } + }) def test_unset_config_global(self): m, _ = self._make(global_state={ @@ -595,19 +688,34 @@ class GitConfigStateTestTest(unittest.TestCase): m.unset_config('section.key', scope='global', missing_ok=False) self.assertDictEqual(m.load_config(), { - 'extra': ['another'], + "global": { + 'extra': ['another'] + }, + "default": { + 'extra': ['another'] + } }) with self.assertRaises(scm.GitConfigUnsetMissingValue): m.unset_config('section.key', scope='global', missing_ok=False) self.assertDictEqual(m.load_config(), { - 'extra': ['another'], + "global": { + 'extra': ['another'] + }, + "default": { + 'extra': ['another'] + } }) m.unset_config('section.key', scope='global', missing_ok=True) self.assertDictEqual(m.load_config(), { - 'extra': ['another'], + "global": { + 'extra': ['another'] + }, + "default": { + 'extra': ['another'] + } }) def test_unset_config_global_multi(self): @@ -644,9 +752,15 @@ class GitConfigStateTestTest(unittest.TestCase): value_pattern='2', scope='global', missing_ok=False) - self.assertDictEqual(m.load_config(), { - 'section.key': ['1', '3', '1'], - }) + self.assertDictEqual( + m.load_config(), { + 'global': { + 'section.key': ['1', '3', '1'], + }, + 'default': { + 'section.key': ['1', '3', '1'], + } + }) class CanonicalizeGitConfigKeyTest(unittest.TestCase):