summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGreg Becker <becker33@llnl.gov>2020-12-23 20:29:38 -0800
committerGitHub <noreply@github.com>2020-12-23 20:29:38 -0800
commit6947951aaf9954b1dfd12ca7a9266d7335f07105 (patch)
tree62e9c3800609143b366711072dcf192ce7eca8f9
parent91e86f9d0d2d417486fbb95b519355667e0fbf2c (diff)
downloadspack-6947951aaf9954b1dfd12ca7a9266d7335f07105.tar.gz
spack-6947951aaf9954b1dfd12ca7a9266d7335f07105.tar.bz2
spack-6947951aaf9954b1dfd12ca7a9266d7335f07105.tar.xz
spack-6947951aaf9954b1dfd12ca7a9266d7335f07105.zip
bugfix: do not write empty default dicts/lists in envs (#20526)
Environment yaml files should not have default values written to them. To accomplish this, we change the validator to not add the default values to yaml. We rely on the code to set defaults for all values (and use defaulting getters like dict.get(key, default)). Includes regression test.
-rw-r--r--lib/spack/spack/environment.py7
-rw-r--r--lib/spack/spack/schema/__init__.py44
-rw-r--r--lib/spack/spack/test/cmd/env.py34
3 files changed, 38 insertions, 47 deletions
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index c098ed4b36..698ea91870 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -686,7 +686,7 @@ class Environment(object):
else:
self.spec_lists[name] = user_specs
- spec_list = config_dict(self.yaml).get(user_speclist_name)
+ spec_list = config_dict(self.yaml).get(user_speclist_name, [])
user_specs = SpecList(user_speclist_name, [s for s in spec_list if s],
self.spec_lists.copy())
self.spec_lists[user_speclist_name] = user_specs
@@ -708,10 +708,11 @@ class Environment(object):
self.views = {}
# Retrieve the current concretization strategy
configuration = config_dict(self.yaml)
- self.concretization = configuration.get('concretization')
+ # default concretization to separately
+ self.concretization = configuration.get('concretization', 'separately')
# Retrieve dev-build packages:
- self.dev_specs = configuration['develop']
+ self.dev_specs = configuration.get('develop', {})
for name, entry in self.dev_specs.items():
# spec must include a concrete version
assert Spec(entry['spec']).version.concrete
diff --git a/lib/spack/spack/schema/__init__.py b/lib/spack/spack/schema/__init__.py
index 8c485a4bfd..1ddca90cc6 100644
--- a/lib/spack/spack/schema/__init__.py
+++ b/lib/spack/spack/schema/__init__.py
@@ -4,9 +4,6 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
"""This module contains jsonschema files for all of Spack's YAML formats."""
-import copy
-import re
-
import six
import llnl.util.lang
@@ -18,45 +15,6 @@ import spack.spec
# and increases the start-up time
def _make_validator():
import jsonschema
- _validate_properties = jsonschema.Draft4Validator.VALIDATORS["properties"]
- _validate_pattern_properties = jsonschema.Draft4Validator.VALIDATORS[
- "patternProperties"
- ]
-
- def _set_defaults(validator, properties, instance, schema):
- """Adds support for the 'default' attribute in 'properties'.
-
- ``jsonschema`` does not handle this out of the box -- it only
- validates. This allows us to set default values for configs
- where certain fields are `None` b/c they're deleted or
- commented out.
- """
- for property, subschema in six.iteritems(properties):
- if "default" in subschema:
- instance.setdefault(
- property, copy.deepcopy(subschema["default"]))
- for err in _validate_properties(
- validator, properties, instance, schema):
- yield err
-
- def _set_pp_defaults(validator, properties, instance, schema):
- """Adds support for the 'default' attribute in 'patternProperties'.
-
- ``jsonschema`` does not handle this out of the box -- it only
- validates. This allows us to set default values for configs
- where certain fields are `None` b/c they're deleted or
- commented out.
- """
- for property, subschema in six.iteritems(properties):
- if "default" in subschema:
- if isinstance(instance, dict):
- for key, val in six.iteritems(instance):
- if re.match(property, key) and val is None:
- instance[key] = copy.deepcopy(subschema["default"])
-
- for err in _validate_pattern_properties(
- validator, properties, instance, schema):
- yield err
def _validate_spec(validator, is_spec, instance, schema):
"""Check if the attributes on instance are valid specs."""
@@ -101,8 +59,6 @@ def _make_validator():
return jsonschema.validators.extend(
jsonschema.Draft4Validator, {
"validate_spec": _validate_spec,
- "properties": _set_defaults,
- "patternProperties": _set_pp_defaults,
"deprecatedProperties": _deprecated_properties
}
)
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 49a2126877..c2d75d9d1f 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -2162,6 +2162,40 @@ def test_env_write_only_non_default():
assert yaml == ev.default_manifest_yaml
+@pytest.mark.regression('20526')
+def test_env_write_only_non_default_nested(tmpdir):
+ # setup an environment file
+ # the environment includes configuration because nested configs proved the
+ # most difficult to avoid writing.
+ filename = 'spack.yaml'
+ filepath = str(tmpdir.join(filename))
+ contents = """\
+env:
+ specs:
+ - matrix:
+ - [mpileaks]
+ packages:
+ mpileaks:
+ compiler: [gcc]
+ view: true
+"""
+
+ # create environment with some structure
+ with open(filepath, 'w') as f:
+ f.write(contents)
+ env('create', 'test', filepath)
+
+ # concretize
+ with ev.read('test') as e:
+ concretize()
+ e.write()
+
+ with open(e.manifest_path, 'r') as f:
+ manifest = f.read()
+
+ assert manifest == contents
+
+
@pytest.fixture
def packages_yaml_v015(tmpdir):
"""Return the path to an existing manifest in the v0.15.x format