Fix initialization of defaults in yaml schema validation (#7959)

Fixes #7924

Spack's yaml schema validator was initializing all instances of
unspecified properties with the same object, so that updating the
property for one entry was updating it for others (e.g. updating
versions for one package would update it for other packages).

This updates the schema validator to instantiate defaults with
separate object instances and adds a test to confirm this behavior
(and also confirms #7924 without this change).
This commit is contained in:
scheibelp 2018-05-01 17:23:27 -07:00 committed by GitHub
parent 29b47e0e01
commit d17372290d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 47 additions and 2 deletions

View file

@ -116,7 +116,8 @@ def extend_with_default(validator_class):
def set_defaults(validator, properties, instance, schema): def set_defaults(validator, properties, instance, schema):
for property, subschema in iteritems(properties): for property, subschema in iteritems(properties):
if "default" in subschema: if "default" in subschema:
instance.setdefault(property, subschema["default"]) instance.setdefault(
property, copy.deepcopy(subschema["default"]))
for err in validate_properties( for err in validate_properties(
validator, properties, instance, schema): validator, properties, instance, schema):
yield err yield err
@ -127,7 +128,7 @@ def set_pp_defaults(validator, properties, instance, schema):
if isinstance(instance, dict): if isinstance(instance, dict):
for key, val in iteritems(instance): for key, val in iteritems(instance):
if re.match(property, key) and val is None: if re.match(property, key) and val is None:
instance[key] = subschema["default"] instance[key] = copy.deepcopy(subschema["default"])
for err in validate_pattern_properties( for err in validate_pattern_properties(
validator, properties, instance, schema): validator, properties, instance, schema):

View file

@ -167,6 +167,50 @@
'build_stage:': ['patha', 'pathb']}} 'build_stage:': ['patha', 'pathb']}}
packages_merge_low = {
'packages': {
'foo': {
'variants': ['+v1']
},
'bar': {
'variants': ['+v2']
}
}
}
packages_merge_high = {
'packages': {
'foo': {
'version': ['a']
},
'bar': {
'version': ['b'],
'variants': ['+v3']
},
'baz': {
'version': ['c']
}
}
}
@pytest.mark.regression('7924')
def test_merge_with_defaults(config, write_config_file):
"""This ensures that specified preferences merge with defaults as
expected. Originally all defaults were initialized with the
exact same object, which led to aliasing problems. Therefore
the test configs used here leave 'version' blank for multiple
packages in 'packages_merge_low'.
"""
write_config_file('packages', packages_merge_low, 'low')
write_config_file('packages', packages_merge_high, 'high')
cfg = spack.config.get_config('packages')
assert cfg['foo']['version'] == ['a']
assert cfg['bar']['version'] == ['b']
assert cfg['baz']['version'] == ['c']
def check_compiler_config(comps, *compiler_names): def check_compiler_config(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')