From d17372290d3b7c6587e74f3af091ab5ac8d6d830 Mon Sep 17 00:00:00 2001 From: scheibelp Date: Tue, 1 May 2018 17:23:27 -0700 Subject: [PATCH] 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). --- lib/spack/spack/config.py | 5 ++-- lib/spack/spack/test/config.py | 44 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 619464727b..96225c0691 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -116,7 +116,8 @@ def extend_with_default(validator_class): def set_defaults(validator, properties, instance, schema): for property, subschema in iteritems(properties): if "default" in subschema: - instance.setdefault(property, subschema["default"]) + instance.setdefault( + property, copy.deepcopy(subschema["default"])) for err in validate_properties( validator, properties, instance, schema): yield err @@ -127,7 +128,7 @@ def set_pp_defaults(validator, properties, instance, schema): if isinstance(instance, dict): for key, val in iteritems(instance): 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( validator, properties, instance, schema): diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index f3a745ad17..5188af5465 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -167,6 +167,50 @@ '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): """Check that named compilers in comps match Spack's config.""" config = spack.config.get_config('compilers')