Allow deprecating more than one property in config (#46221)

* Allow deprecating more than one property in config

This internal change allows the customization of errors
and warnings to be printed when deprecating a property.

Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

* fix

Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

* Use a list comprehension for "issues"

Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

---------

Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

lib/spack/spack/schema/__init__.py

@@ -3,22 +3,28 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 """This module contains jsonschema files for all of Spack's YAML formats."""
+import typing
 import warnings
 
 import llnl.util.lang
 
 
+class DeprecationMessage(typing.NamedTuple):
+    message: str
+    error: bool
+
+
 # jsonschema is imported lazily as it is heavy to import
 # and increases the start-up time
 def _make_validator():
     import jsonschema
 
-    import spack.parser
-
     def _validate_spec(validator, is_spec, instance, schema):
         """Check if the attributes on instance are valid specs."""
         import jsonschema
 
+        import spack.parser
+
         if not validator.is_type(instance, "object"):
             return
 
@@ -32,27 +38,31 @@ def _deprecated_properties(validator, deprecated, instance, schema):
         if not (validator.is_type(instance, "object") or validator.is_type(instance, "array")):
             return
 
+        if not deprecated:
+            return
+
+        deprecations = {
+            name: DeprecationMessage(message=x["message"], error=x["error"])
+            for x in deprecated
+            for name in x["names"]
+        }
+
         # Get a list of the deprecated properties, return if there is none
-        deprecated_properties = [x for x in instance if x in deprecated["properties"]]
+        issues = [entry for entry in instance if entry in deprecations]
-        if not deprecated_properties:
+        if not issues:
             return
 
-        # Retrieve the template message
+        # Process issues
-        msg_str_or_func = deprecated["message"]
+        errors = []
-        if isinstance(msg_str_or_func, str):
+        for name in issues:
-            msg = msg_str_or_func.format(properties=deprecated_properties)
+            msg = deprecations[name].message.format(name=name)
+            if deprecations[name].error:
+                errors.append(msg)
             else:
-            msg = msg_str_or_func(instance, deprecated_properties)
-            if msg is None:
-                return
-
-        is_error = deprecated["error"]
-        if not is_error:
                 warnings.warn(msg)
-        else:
-            import jsonschema
 
-            yield jsonschema.ValidationError(msg)
+        if errors:
+            yield jsonschema.ValidationError("\n".join(errors))
 
     return jsonschema.validators.extend(
         jsonschema.Draft4Validator,

lib/spack/spack/schema/config.py

@@ -96,12 +96,14 @@
             "binary_index_ttl": {"type": "integer", "minimum": 0},
             "aliases": {"type": "object", "patternProperties": {r"\w[\w-]*": {"type": "string"}}},
         },
-        "deprecatedProperties": {
+        "deprecatedProperties": [
-            "properties": ["concretizer"],
+            {
+                "names": ["concretizer"],
                 "message": "Spack supports only clingo as a concretizer from v0.23. "
                 "The config:concretizer config option is ignored.",
                 "error": False,
-        },
+            }
+        ],
     }
 }
 

lib/spack/spack/schema/packages.py

@@ -140,14 +140,16 @@
                     },
                     "variants": variants,
                 },
-                "deprecatedProperties": {
+                "deprecatedProperties": [
-                    "properties": ["version"],
+                    {
-                    "message": "setting version preferences in the 'all' section of packages.yaml "
+                        "names": ["version"],
-                    "is deprecated and will be removed in v0.23\n\n\tThese preferences "
+                        "message": "setting version preferences in the 'all' section of "
-                    "will be ignored by Spack. You can set them only in package-specific sections "
+                        "packages.yaml is deprecated and will be removed in v0.23"
-                    "of the same file.\n",
+                        "\n\n\tThese preferences will be ignored by Spack. You can "
+                        "set them only in package-specific sections of the same file.\n",
                         "error": False,
-                },
+                    }
+                ],
             }
         },
         "patternProperties": {
@@ -204,18 +206,20 @@
                         },
                     },
                 },
-                "deprecatedProperties": {
+                "deprecatedProperties": [
-                    "properties": ["target", "compiler", "providers"],
+                    {
-                    "message": "setting 'compiler:', 'target:' or 'provider:' preferences in "
+                        "names": ["target", "compiler", "providers"],
+                        "message": "setting '{name}:' preferences in "
                         "a package-specific section of packages.yaml is deprecated, and will be "
-                    "removed in v0.23.\n\n\tThese preferences will be ignored by Spack, and "
+                        "removed in v0.23.\n\n\tThis preferences will be ignored by Spack, and "
                         "can be set only in the 'all' section of the same file. "
-                    "You can run:\n\n\t\t$ spack audit configs\n\n\tto get better diagnostics, "
+                        "You can run:\n\n\t\t$ spack audit configs\n\n\tto get better "
-                    "including files:lines where the deprecated attributes are used.\n\n"
+                        "diagnostics, including files:lines where the deprecated "
-                    "\tUse requirements to enforce conditions on specific packages: "
+                        "attributes are used.\n\n\tUse requirements to enforce conditions"
-                    f"{REQUIREMENT_URL}\n",
+                        f" on specific packages: {REQUIREMENT_URL}\n",
                         "error": False,
-                },
+                    }
+                ],
             }
         },
     }

lib/spack/spack/test/schema.py

@@ -105,25 +105,21 @@ def test_schema_validation(meta_schema, config_name):
 
 def test_deprecated_properties(module_suffixes_schema):
     # Test that an error is reported when 'error: True'
-    msg_fmt = r"deprecated properties detected [properties={properties}]"
+    msg_fmt = r"{name} is deprecated"
-    module_suffixes_schema["deprecatedProperties"] = {
+    module_suffixes_schema["deprecatedProperties"] = [
-        "properties": ["tcl"],
+        {"names": ["tcl"], "message": msg_fmt, "error": True}
-        "message": msg_fmt,
+    ]
-        "error": True,
-    }
     v = spack.schema.Validator(module_suffixes_schema)
     data = {"tcl": {"all": {"suffixes": {"^python": "py"}}}}
 
-    expected_match = "deprecated properties detected"
+    expected_match = "tcl is deprecated"
     with pytest.raises(jsonschema.ValidationError, match=expected_match):
         v.validate(data)
 
     # Test that just a warning is reported when 'error: False'
-    module_suffixes_schema["deprecatedProperties"] = {
+    module_suffixes_schema["deprecatedProperties"] = [
-        "properties": ["tcl"],
+        {"names": ["tcl"], "message": msg_fmt, "error": False}
-        "message": msg_fmt,
+    ]
-        "error": False,
-    }
     v = spack.schema.Validator(module_suffixes_schema)
     data = {"tcl": {"all": {"suffixes": {"^python": "py"}}}}
     # The next validation doesn't raise anymore