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.

3 files changed, 38 insertions, 47 deletions

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 0b63bad6a7..38c2c4cdb7 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -685,7 +685,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
@@ -707,10 +707,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