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.

lib/spack/spack/environment.py

@@ -685,7 +685,7 @@ def _read_manifest(self, f, raw_yaml=None):
                 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 @@ def _read_manifest(self, f, raw_yaml=None):
             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

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 @@
 # 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 _deprecated_properties(validator, deprecated, instance, schema):
     return jsonschema.validators.extend(
         jsonschema.Draft4Validator, {
             "validate_spec": _validate_spec,
-            "properties": _set_defaults,
-            "patternProperties": _set_pp_defaults,
             "deprecatedProperties": _deprecated_properties
         }
     )

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