Bugfix: '::' only worked on top-level key in config.

- generalized and fixed to work with any key in YAML file
- simplified schema writing, as well
- add more unit tests for the config system
- Rename test/yaml.py to test/spack_yaml.py
  - Add test/yaml.pyc to ignored pyc files.
This commit is contained in:
Todd Gamblin 2016-10-30 18:29:44 -07:00
parent d155156e32
commit 8f21332fec
12 changed files with 256 additions and 151 deletions

View file

@ -72,7 +72,8 @@ orphaned_pyc_files = [
os.path.join(SPACK_LIB_PATH, 'spack', 'platforms', 'cray_xc.pyc'), os.path.join(SPACK_LIB_PATH, 'spack', 'platforms', 'cray_xc.pyc'),
os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'package-list.pyc'), os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'package-list.pyc'),
os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'test-install.pyc'), os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'test-install.pyc'),
os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'url-parse.pyc') os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'url-parse.pyc'),
os.path.join(SPACK_LIB_PATH, 'spack', 'test', 'yaml.pyc')
] ]
for pyc_file in orphaned_pyc_files: for pyc_file in orphaned_pyc_files:

View file

@ -266,7 +266,7 @@ def _read_config_file(filename, schema):
try: try:
tty.debug("Reading config file %s" % filename) tty.debug("Reading config file %s" % filename)
with open(filename) as f: with open(filename) as f:
data = syaml.load(f) data = _mark_overrides(syaml.load(f))
if data: if data:
validate_section(data, schema) validate_section(data, schema)
@ -288,6 +288,34 @@ def clear_config_caches():
scope.clear() scope.clear()
def override(string):
"""Test if a spack YAML string is an override.
See ``spack_yaml`` for details. Keys in Spack YAML can end in `::`,
and if they do, their values completely replace lower-precedence
configs instead of merging into them.
"""
return hasattr(string, 'override') and string.override
def _mark_overrides(data):
if isinstance(data, list):
return [_mark_overrides(elt) for elt in data]
elif isinstance(data, dict):
marked = {}
for key, val in data.iteritems():
if isinstance(key, basestring) and key.endswith(':'):
key = syaml.syaml_str(key[:-1])
key.override = True
marked[key] = _mark_overrides(val)
return marked
else:
return data
def _merge_yaml(dest, source): def _merge_yaml(dest, source):
"""Merges source into dest; entries in source take precedence over dest. """Merges source into dest; entries in source take precedence over dest.
@ -320,9 +348,11 @@ def they_are(t):
# Source dict is merged into dest. # Source dict is merged into dest.
elif they_are(dict): elif they_are(dict):
for sk, sv in source.iteritems(): for sk, sv in source.iteritems():
if sk not in dest: if override(sk) or sk not in dest:
# if sk ended with ::, or if it's new, completely override
dest[sk] = copy.copy(sv) dest[sk] = copy.copy(sv)
else: else:
# otherwise, merge the YAML
dest[sk] = _merge_yaml(dest[sk], source[sk]) dest[sk] = _merge_yaml(dest[sk], source[sk])
return dest return dest
@ -371,18 +401,18 @@ def get_config(section, scope=None):
if not data or not isinstance(data, dict): if not data or not isinstance(data, dict):
continue continue
# Allow complete override of site config with '<section>::' if section not in data:
override_key = section + ':'
if not (section in data or override_key in data):
tty.warn("Skipping bad configuration file: '%s'" % scope.path) tty.warn("Skipping bad configuration file: '%s'" % scope.path)
continue continue
if override_key in data: merged_section = _merge_yaml(merged_section, data)
merged_section = data[override_key]
else:
merged_section = _merge_yaml(merged_section, data[section])
return merged_section # no config files -- empty config.
if section not in merged_section:
return {}
# take the top key off before returning.
return merged_section[section]
def get_config_filename(scope, section): def get_config_filename(scope, section):

View file

@ -35,7 +35,7 @@
'type': 'object', 'type': 'object',
'additionalProperties': False, 'additionalProperties': False,
'patternProperties': { 'patternProperties': {
'compilers:?': { # optional colon for overriding site config. 'compilers': {
'type': 'array', 'type': 'array',
'items': { 'items': {
'compiler': { 'compiler': {

View file

@ -49,7 +49,6 @@
}, },
'module_roots': { 'module_roots': {
'type': 'object', 'type': 'object',
'default': {},
'additionalProperties': False, 'additionalProperties': False,
'properties': { 'properties': {
'tcl': {'type': 'string'}, 'tcl': {'type': 'string'},
@ -59,18 +58,9 @@
}, },
'source_cache': {'type': 'string'}, 'source_cache': {'type': 'string'},
'misc_cache': {'type': 'string'}, 'misc_cache': {'type': 'string'},
'verify_ssl': { 'verify_ssl': {'type': 'boolean'},
'type': 'boolean', 'checksum': {'type': 'boolean'},
'default': True, 'dirty': {'type': 'boolean'},
},
'checksum': {
'type': 'boolean',
'default': True,
},
'dirty': {
'type': 'boolean',
'default': False,
},
} }
}, },
}, },

View file

@ -35,7 +35,7 @@
'type': 'object', 'type': 'object',
'additionalProperties': False, 'additionalProperties': False,
'patternProperties': { 'patternProperties': {
r'mirrors:?': { r'mirrors': {
'type': 'object', 'type': 'object',
'default': {}, 'default': {},
'additionalProperties': False, 'additionalProperties': False,

View file

@ -127,7 +127,7 @@
} }
}, },
'patternProperties': { 'patternProperties': {
r'modules:?': { r'modules': {
'type': 'object', 'type': 'object',
'default': {}, 'default': {},
'additionalProperties': False, 'additionalProperties': False,

View file

@ -35,7 +35,7 @@
'type': 'object', 'type': 'object',
'additionalProperties': False, 'additionalProperties': False,
'patternProperties': { 'patternProperties': {
r'packages:?': { r'packages': {
'type': 'object', 'type': 'object',
'default': {}, 'default': {},
'additionalProperties': False, 'additionalProperties': False,

View file

@ -35,7 +35,7 @@
'type': 'object', 'type': 'object',
'additionalProperties': False, 'additionalProperties': False,
'patternProperties': { 'patternProperties': {
r'repos:?': { r'repos': {
'type': 'array', 'type': 'array',
'default': [], 'default': [],
'items': { 'items': {

View file

@ -78,7 +78,7 @@
'url_substitution', 'url_substitution',
'versions', 'versions',
'provider_index', 'provider_index',
'yaml', 'spack_yaml',
# This test needs to be last until global compiler cache is fixed. # This test needs to be last until global compiler cache is fixed.
'cmd.test_compiler_cmd', 'cmd.test_compiler_cmd',
] ]

View file

@ -25,6 +25,7 @@
import os import os
import shutil import shutil
import getpass import getpass
import yaml
from tempfile import mkdtemp from tempfile import mkdtemp
import spack import spack
@ -34,109 +35,136 @@
from spack.test.mock_packages_test import * from spack.test.mock_packages_test import *
# Some sample compiler config data # Some sample compiler config data
a_comps = [ a_comps = {
{'compiler': { 'compilers': [
'paths': { {'compiler': {
"cc": "/gcc473", 'paths': {
"cxx": "/g++473", "cc": "/gcc473",
"f77": None, "cxx": "/g++473",
"fc": None "f77": None,
}, "fc": None
'modules': None, },
'spec': 'gcc@4.7.3', 'modules': None,
'operating_system': 'CNL10' 'spec': 'gcc@4.7.3',
}}, 'operating_system': 'CNL10'
{'compiler': { }},
'paths': { {'compiler': {
"cc": "/gcc450", 'paths': {
"cxx": "/g++450", "cc": "/gcc450",
"f77": 'gfortran', "cxx": "/g++450",
"fc": 'gfortran' "f77": 'gfortran',
}, "fc": 'gfortran'
'modules': None, },
'spec': 'gcc@4.5.0', 'modules': None,
'operating_system': 'CNL10' 'spec': 'gcc@4.5.0',
}}, 'operating_system': 'CNL10'
{'compiler': { }},
'paths': { {'compiler': {
"cc": "/gcc422", 'paths': {
"cxx": "/g++422", "cc": "/gcc422",
"f77": 'gfortran', "cxx": "/g++422",
"fc": 'gfortran' "f77": 'gfortran',
}, "fc": 'gfortran'
'flags': { },
"cppflags": "-O0 -fpic", 'flags': {
"fflags": "-f77", "cppflags": "-O0 -fpic",
}, "fflags": "-f77",
'modules': None, },
'spec': 'gcc@4.2.2', 'modules': None,
'operating_system': 'CNL10' 'spec': 'gcc@4.2.2',
}}, 'operating_system': 'CNL10'
{'compiler': { }},
'paths': { {'compiler': {
"cc": "<overwritten>", 'paths': {
"cxx": "<overwritten>", "cc": "<overwritten>",
"f77": '<overwritten>', "cxx": "<overwritten>",
"fc": '<overwritten>'}, "f77": '<overwritten>',
'modules': None, "fc": '<overwritten>'},
'spec': 'clang@3.3', 'modules': None,
'operating_system': 'CNL10' 'spec': 'clang@3.3',
}} 'operating_system': 'CNL10'
] }}
]
}
b_comps = [ b_comps = {
{'compiler': { 'compilers': [
'paths': { {'compiler': {
"cc": "/icc100", 'paths': {
"cxx": "/icp100", "cc": "/icc100",
"f77": None, "cxx": "/icp100",
"fc": None "f77": None,
}, "fc": None
'modules': None, },
'spec': 'icc@10.0', 'modules': None,
'operating_system': 'CNL10' 'spec': 'icc@10.0',
}}, 'operating_system': 'CNL10'
{'compiler': { }},
'paths': { {'compiler': {
"cc": "/icc111", 'paths': {
"cxx": "/icp111", "cc": "/icc111",
"f77": 'ifort', "cxx": "/icp111",
"fc": 'ifort' "f77": 'ifort',
}, "fc": 'ifort'
'modules': None, },
'spec': 'icc@11.1', 'modules': None,
'operating_system': 'CNL10' 'spec': 'icc@11.1',
}}, 'operating_system': 'CNL10'
{'compiler': { }},
'paths': { {'compiler': {
"cc": "/icc123", 'paths': {
"cxx": "/icp123", "cc": "/icc123",
"f77": 'ifort', "cxx": "/icp123",
"fc": 'ifort' "f77": 'ifort',
}, "fc": 'ifort'
'flags': { },
"cppflags": "-O3", 'flags': {
"fflags": "-f77rtl", "cppflags": "-O3",
}, "fflags": "-f77rtl",
'modules': None, },
'spec': 'icc@12.3', 'modules': None,
'operating_system': 'CNL10' 'spec': 'icc@12.3',
}}, 'operating_system': 'CNL10'
{'compiler': { }},
'paths': { {'compiler': {
"cc": "<overwritten>", 'paths': {
"cxx": "<overwritten>", "cc": "<overwritten>",
"f77": '<overwritten>', "cxx": "<overwritten>",
"fc": '<overwritten>'}, "f77": '<overwritten>',
'modules': None, "fc": '<overwritten>'},
'spec': 'clang@3.3', 'modules': None,
'operating_system': 'CNL10' 'spec': 'clang@3.3',
}} 'operating_system': 'CNL10'
] }}
]
}
# Some Sample repo data # Some Sample repo data
repos_low = ["/some/path"] repos_low = {'repos': ["/some/path"]}
repos_high = ["/some/other/path"] repos_high = {'repos': ["/some/other/path"]}
# sample config data
config_low = {
'config': {
'install_tree': 'install_tree_path',
'build_stage': ['path1', 'path2', 'path3']}}
config_override_all = {
'config:': {
'install_tree:': 'override_all'}}
config_override_key = {
'config': {
'install_tree:': 'override_key'}}
config_merge_list = {
'config': {
'build_stage': ['patha', 'pathb']}}
config_override_list = {
'config': {
'build_stage:': ['patha', 'pathb']}}
class ConfigTest(MockPackagesTest): class ConfigTest(MockPackagesTest):
@ -144,19 +172,30 @@ class ConfigTest(MockPackagesTest):
def setUp(self): def setUp(self):
super(ConfigTest, self).setUp() super(ConfigTest, self).setUp()
self.tmp_dir = mkdtemp('.tmp', 'spack-config-test-') self.tmp_dir = mkdtemp('.tmp', 'spack-config-test-')
self.a_comp_specs = [ac['compiler']['spec'] for ac in a_comps] self.a_comp_specs = [
self.b_comp_specs = [bc['compiler']['spec'] for bc in b_comps] ac['compiler']['spec'] for ac in a_comps['compilers']]
self.b_comp_specs = [
bc['compiler']['spec'] for bc in b_comps['compilers']]
spack.config.config_scopes = OrderedDict() spack.config.config_scopes = OrderedDict()
for priority in ['low', 'high']: for priority in ['low', 'high']:
spack.config.ConfigScope('test_{0}_priority'.format(priority), scope_dir = os.path.join(self.tmp_dir, priority)
os.path.join(self.tmp_dir, priority)) spack.config.ConfigScope(priority, scope_dir)
def tearDown(self): def tearDown(self):
super(ConfigTest, self).tearDown() super(ConfigTest, self).tearDown()
shutil.rmtree(self.tmp_dir, True) shutil.rmtree(self.tmp_dir, True)
def check_config(self, comps, *compiler_names): def write_config_file(self, config, data, scope):
scope_dir = os.path.join(self.tmp_dir, scope)
mkdirp(scope_dir)
path = os.path.join(scope_dir, config + '.yaml')
with open(path, 'w') as f:
print yaml
yaml.dump(data, f)
def check_compiler_config(self, comps, *compiler_names):
"""Check that named compilers in comps match Spack's config.""" """Check that named compilers in comps match Spack's config."""
config = spack.config.get_config('compilers') config = spack.config.get_config('compilers')
compiler_list = ['cc', 'cxx', 'f77', 'fc'] compiler_list = ['cc', 'cxx', 'f77', 'fc']
@ -182,43 +221,50 @@ def check_config(self, comps, *compiler_names):
self.assertEqual(expected, actual) self.assertEqual(expected, actual)
def test_write_list_in_memory(self): def test_write_list_in_memory(self):
spack.config.update_config('repos', repos_low, 'test_low_priority') spack.config.update_config('repos', repos_low['repos'], scope='low')
spack.config.update_config('repos', repos_high, 'test_high_priority') spack.config.update_config('repos', repos_high['repos'], scope='high')
config = spack.config.get_config('repos') config = spack.config.get_config('repos')
self.assertEqual(config, repos_high + repos_low) self.assertEqual(config, repos_high['repos'] + repos_low['repos'])
def test_write_key_in_memory(self): def test_write_key_in_memory(self):
# Write b_comps "on top of" a_comps. # Write b_comps "on top of" a_comps.
spack.config.update_config('compilers', a_comps, 'test_low_priority') spack.config.update_config(
spack.config.update_config('compilers', b_comps, 'test_high_priority') 'compilers', a_comps['compilers'], scope='low')
spack.config.update_config(
'compilers', b_comps['compilers'], scope='high')
# Make sure the config looks how we expect. # Make sure the config looks how we expect.
self.check_config(a_comps, *self.a_comp_specs) self.check_compiler_config(a_comps['compilers'], *self.a_comp_specs)
self.check_config(b_comps, *self.b_comp_specs) self.check_compiler_config(b_comps['compilers'], *self.b_comp_specs)
def test_write_key_to_disk(self): def test_write_key_to_disk(self):
# Write b_comps "on top of" a_comps. # Write b_comps "on top of" a_comps.
spack.config.update_config('compilers', a_comps, 'test_low_priority') spack.config.update_config(
spack.config.update_config('compilers', b_comps, 'test_high_priority') 'compilers', a_comps['compilers'], scope='low')
spack.config.update_config(
'compilers', b_comps['compilers'], scope='high')
# Clear caches so we're forced to read from disk. # Clear caches so we're forced to read from disk.
spack.config.clear_config_caches() spack.config.clear_config_caches()
# Same check again, to ensure consistency. # Same check again, to ensure consistency.
self.check_config(a_comps, *self.a_comp_specs) self.check_compiler_config(a_comps['compilers'], *self.a_comp_specs)
self.check_config(b_comps, *self.b_comp_specs) self.check_compiler_config(b_comps['compilers'], *self.b_comp_specs)
def test_write_to_same_priority_file(self): def test_write_to_same_priority_file(self):
# Write b_comps in the same file as a_comps. # Write b_comps in the same file as a_comps.
spack.config.update_config('compilers', a_comps, 'test_low_priority') spack.config.update_config(
spack.config.update_config('compilers', b_comps, 'test_low_priority') 'compilers', a_comps['compilers'], scope='low')
spack.config.update_config(
'compilers', b_comps['compilers'], scope='low')
# Clear caches so we're forced to read from disk. # Clear caches so we're forced to read from disk.
spack.config.clear_config_caches() spack.config.clear_config_caches()
# Same check again, to ensure consistency. # Same check again, to ensure consistency.
self.check_config(a_comps, *self.a_comp_specs) self.check_compiler_config(a_comps['compilers'], *self.a_comp_specs)
self.check_config(b_comps, *self.b_comp_specs) self.check_compiler_config(b_comps['compilers'], *self.b_comp_specs)
def check_canonical(self, var, expected): def check_canonical(self, var, expected):
"""Ensure that <expected> is substituted properly for <var> in strings """Ensure that <expected> is substituted properly for <var> in strings
@ -270,3 +316,39 @@ def test_substitute_tempdir(self):
self.assertEqual(tempdir, canonicalize_path('$tempdir')) self.assertEqual(tempdir, canonicalize_path('$tempdir'))
self.assertEqual(tempdir + '/foo/bar/baz', self.assertEqual(tempdir + '/foo/bar/baz',
canonicalize_path('$tempdir/foo/bar/baz')) canonicalize_path('$tempdir/foo/bar/baz'))
def test_read_config(self):
self.write_config_file('config', config_low, 'low')
self.assertEqual(spack.config.get_config('config'),
config_low['config'])
def test_read_config_override_all(self):
self.write_config_file('config', config_low, 'low')
self.write_config_file('config', config_override_all, 'high')
self.assertEqual(spack.config.get_config('config'), {
'install_tree': 'override_all'
})
def test_read_config_override_key(self):
self.write_config_file('config', config_low, 'low')
self.write_config_file('config', config_override_key, 'high')
self.assertEqual(spack.config.get_config('config'), {
'install_tree': 'override_key',
'build_stage': ['path1', 'path2', 'path3']
})
def test_read_config_merge_list(self):
self.write_config_file('config', config_low, 'low')
self.write_config_file('config', config_merge_list, 'high')
self.assertEqual(spack.config.get_config('config'), {
'install_tree': 'install_tree_path',
'build_stage': ['patha', 'pathb', 'path1', 'path2', 'path3']
})
def test_read_config_override_list(self):
self.write_config_file('config', config_low, 'low')
self.write_config_file('config', config_override_list, 'high')
self.assertEqual(spack.config.get_config('config'), {
'install_tree': 'install_tree_path',
'build_stage': ['patha', 'pathb']
})

View file

@ -56,7 +56,7 @@
])} ])}
class YamlTest(unittest.TestCase): class SpackYamlTest(unittest.TestCase):
def setUp(self): def setUp(self):
self.data = syaml.load(test_file) self.data = syaml.load(test_file)

View file

@ -91,7 +91,9 @@ def construct_yaml_str(self, node):
value = value.encode('ascii') value = value.encode('ascii')
except UnicodeEncodeError: except UnicodeEncodeError:
pass pass
value = syaml_str(value) value = syaml_str(value)
mark(value, node) mark(value, node)
return value return value