Allow overrides for spack.config set() and override(). (#14432)

Allows spack.config InternalConfigScope and Configuration.set() to
handle keys with trailing ':' to indicate replacement vs merge
behavior with respect to lower priority scopes.

Lists may now be replaced rather than merged (this behavior was
previously only available for dictionaries).

This commit adds tests for the new behavior.

2 files changed, 141 insertions, 17 deletions

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 7ed47340bf..445d62d2ab 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -280,6 +280,7 @@ class InternalConfigScope(ConfigScope):
         self.sections = syaml.syaml_dict()
 
         if data:
+            data = InternalConfigScope._process_dict_keyname_overrides(data)
             for section in data:
                 dsec = data[section]
                 validate({section: dsec}, section_schemas[section])
@@ -306,6 +307,25 @@ class InternalConfigScope(ConfigScope):
     def __repr__(self):
         return '<InternalConfigScope: %s>' % self.name
 
+    @staticmethod
+    def _process_dict_keyname_overrides(data):
+        """Turn a trailing `:' in a key name into an override attribute."""
+        result = {}
+        for sk, sv in iteritems(data):
+            if sk.endswith(':'):
+                key = syaml.syaml_str(sk[:-1])
+                key.override = True
+            else:
+                key = sk
+
+            if isinstance(sv, dict):
+                result[key]\
+                    = InternalConfigScope._process_dict_keyname_overrides(sv)
+            else:
+                result[key] = copy.copy(sv)
+
+        return result
+
 
 class Configuration(object):
     """A full Spack configuration, from a hierarchy of config files.
@@ -505,14 +525,14 @@ class Configuration(object):
 
         Accepts the path syntax described in ``get()``.
         """
-        section, _, rest = path.partition(':')
+        parts = _process_config_path(path)
+        section = parts.pop(0)
 
-        if not rest:
+        if not parts:
             self.update_config(section, value, scope=scope)
         else:
             section_data = self.get_config(section, scope=scope)
 
-            parts = rest.split(':')
             data = section_data
             while len(parts) > 1:
                 key = parts.pop(0)
@@ -612,7 +632,7 @@ def _config():
     """Singleton Configuration instance.
 
     This constructs one instance associated with this module and returns
-    it. It is bundled inside a function so that configuratoin can be
+    it. It is bundled inside a function so that configuration can be
     initialized lazily.
 
     Return:
@@ -763,17 +783,12 @@ def _merge_yaml(dest, source):
     Config file authors can optionally end any attribute in a dict
     with `::` instead of `:`, and the key will override that of the
     parent instead of merging.
-
     """
     def they_are(t):
         return isinstance(dest, t) and isinstance(source, t)
 
-    # If both are None, handle specially and return None.
-    if source is None and dest is None:
-        return None
-
     # If source is None, overwrite with source.
-    elif source is None:
+    if source is None:
         return None
 
     # Source list is prepended (for precedence)
@@ -799,8 +814,9 @@ def _merge_yaml(dest, source):
             # to copy mark information on source keys to dest.
             key_marks[sk] = sk
 
-        # ensure that keys are marked in the destination.  the key_marks dict
-        # ensures we can get the actual source key objects from dest keys
+        # ensure that keys are marked in the destination. The
+        # key_marks dict ensures we can get the actual source key
+        # objects from dest keys
         for dk in list(dest.keys()):
             if dk in key_marks and syaml.markable(dk):
                 syaml.mark(dk, key_marks[dk])
@@ -812,9 +828,34 @@ def _merge_yaml(dest, source):
 
         return dest
 
-    # In any other case, overwrite with a copy of the source value.
-    else:
-        return copy.copy(source)
+    # If we reach here source and dest are either different types or are
+    # not both lists or dicts: replace with source.
+    return copy.copy(source)
+
+
+#
+# Process a path argument to config.set() that may contain overrides ('::' or
+# trailing ':')
+#
+def _process_config_path(path):
+    result = []
+    if path.startswith(':'):
+        raise syaml.SpackYAMLError("Illegal leading `:' in path `{0}'".
+                                   format(path), '')
+    seen_override_in_path = False
+    while path:
+        front, sep, path = path.partition(':')
+        if (sep and not path) or path.startswith(':'):
+            if seen_override_in_path:
+                raise syaml.SpackYAMLError("Meaningless second override"
+                                           " indicator `::' in path `{0}'".
+                                           format(path), '')
+            path = path.lstrip(':')
+            front = syaml.syaml_str(front)
+            front.override = True
+            seen_override_in_path = True
+        result.append(front)
+    return result
 
 
 #
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index feb2b9cae4..b8598616d5 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -46,7 +46,19 @@ config_merge_list = {
 
 config_override_list = {
     'config': {
-        'build_stage:': ['patha', 'pathb']}}
+        'build_stage:': ['pathd', 'pathe']}}
+
+config_merge_dict = {
+    'config': {
+        'info': {
+            'a': 3,
+            'b': 4}}}
+
+config_override_dict = {
+    'config': {
+        'info:': {
+            'a': 7,
+            'c': 9}}}
 
 
 @pytest.fixture()
@@ -382,7 +394,7 @@ def test_read_config_override_list(mock_low_high_config, write_config_file):
     write_config_file('config', config_override_list, 'high')
     assert spack.config.get('config') == {
         'install_tree': 'install_tree_path',
-        'build_stage': ['patha', 'pathb']
+        'build_stage': config_override_list['config']['build_stage:']
     }
 
 
@@ -857,3 +869,74 @@ def test_dotkit_in_config_does_not_raise(
     # we throw a a deprecation warning without raising
     assert '_sp_sys_type' in captured[0]  # stdout
     assert 'Warning' in captured[1]  # stderr
+
+
+def test_internal_config_section_override(mock_low_high_config,
+                                          write_config_file):
+    write_config_file('config', config_merge_list, 'low')
+    wanted_list = config_override_list['config']['build_stage:']
+    mock_low_high_config.push_scope(spack.config.InternalConfigScope
+                                    ('high', {
+                                        'config:': {
+                                            'build_stage': wanted_list
+                                        }
+                                    }))
+    assert mock_low_high_config.get('config:build_stage') == wanted_list
+
+
+def test_internal_config_dict_override(mock_low_high_config,
+                                       write_config_file):
+    write_config_file('config', config_merge_dict, 'low')
+    wanted_dict = config_override_dict['config']['info:']
+    mock_low_high_config.push_scope(spack.config.InternalConfigScope
+                                    ('high', config_override_dict))
+    assert mock_low_high_config.get('config:info') == wanted_dict
+
+
+def test_internal_config_list_override(mock_low_high_config,
+                                       write_config_file):
+    write_config_file('config', config_merge_list, 'low')
+    wanted_list = config_override_list['config']['build_stage:']
+    mock_low_high_config.push_scope(spack.config.InternalConfigScope
+                                    ('high', config_override_list))
+    assert mock_low_high_config.get('config:build_stage') == wanted_list
+
+
+def test_set_section_override(mock_low_high_config, write_config_file):
+    write_config_file('config', config_merge_list, 'low')
+    wanted_list = config_override_list['config']['build_stage:']
+    with spack.config.override('config::build_stage', wanted_list):
+        assert mock_low_high_config.get('config:build_stage') == wanted_list
+    assert config_merge_list['config']['build_stage'] == \
+        mock_low_high_config.get('config:build_stage')
+
+
+def test_set_list_override(mock_low_high_config, write_config_file):
+    write_config_file('config', config_merge_list, 'low')
+    wanted_list = config_override_list['config']['build_stage:']
+    with spack.config.override('config:build_stage:', wanted_list):
+        assert wanted_list == mock_low_high_config.get('config:build_stage')
+    assert config_merge_list['config']['build_stage'] == \
+        mock_low_high_config.get('config:build_stage')
+
+
+def test_set_dict_override(mock_low_high_config, write_config_file):
+    write_config_file('config', config_merge_dict, 'low')
+    wanted_dict = config_override_dict['config']['info:']
+    with spack.config.override('config:info:', wanted_dict):
+        assert wanted_dict == mock_low_high_config.get('config:info')
+    assert config_merge_dict['config']['info'] == \
+        mock_low_high_config.get('config:info')
+
+
+def test_set_bad_path(config):
+    with pytest.raises(syaml.SpackYAMLError, match='Illegal leading'):
+        with spack.config.override(':bad:path', ''):
+            pass
+
+
+def test_bad_path_double_override(config):
+    with pytest.raises(syaml.SpackYAMLError,
+                       match='Meaningless second override'):
+        with spack.config.override('bad::double:override::directive', ''):
+            pass