From e1f98fd2061f967d58eb796c2040cbd46d40929f Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Thu, 16 Nov 2023 23:30:29 +0100
Subject: [PATCH] Improve the error message for deprecated preferences (#41075)

Improves the warning for deprecated preferences, and adds a configuration
audit to get files:lines details of the issues.

Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>
---
 lib/spack/spack/audit.py           | 36 ++++++++++++++++++++++++++++++
 lib/spack/spack/cmd/audit.py       |  8 +++++--
 lib/spack/spack/schema/packages.py | 16 ++++++++-----
 3 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index 66c7008580..d0a68cf212 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -40,6 +40,7 @@ def _search_duplicate_compilers(error_cls):
 import collections.abc
 import glob
 import inspect
+import io
 import itertools
 import pathlib
 import pickle
@@ -54,6 +55,7 @@ def _search_duplicate_compilers(error_cls):
 import spack.repo
 import spack.spec
 import spack.util.crypto
+import spack.util.spack_yaml as syaml
 import spack.variant
 
 #: Map an audit tag to a list of callables implementing checks
@@ -250,6 +252,40 @@ def _search_duplicate_specs_in_externals(error_cls):
     return errors
 
 
+@config_packages
+def _deprecated_preferences(error_cls):
+    """Search package preferences deprecated in v0.21 (and slated for removal in v0.22)"""
+    # TODO (v0.22): remove this audit as the attributes will not be allowed in config
+    errors = []
+    packages_yaml = spack.config.CONFIG.get_config("packages")
+
+    def make_error(attribute_name, config_data, summary):
+        s = io.StringIO()
+        s.write("Occurring in the following file:\n")
+        dict_view = syaml.syaml_dict((k, v) for k, v in config_data.items() if k == attribute_name)
+        syaml.dump_config(dict_view, stream=s, blame=True)
+        return error_cls(summary=summary, details=[s.getvalue()])
+
+    if "all" in packages_yaml and "version" in packages_yaml["all"]:
+        summary = "Using the deprecated 'version' attribute under 'packages:all'"
+        errors.append(make_error("version", packages_yaml["all"], summary))
+
+    for package_name in packages_yaml:
+        if package_name == "all":
+            continue
+
+        package_conf = packages_yaml[package_name]
+        for attribute in ("compiler", "providers", "target"):
+            if attribute not in package_conf:
+                continue
+            summary = (
+                f"Using the deprecated '{attribute}' attribute " f"under 'packages:{package_name}'"
+            )
+            errors.append(make_error(attribute, package_conf, summary))
+
+    return errors
+
+
 #: Sanity checks on package directives
 package_directives = AuditClass(
     group="packages",
diff --git a/lib/spack/spack/cmd/audit.py b/lib/spack/spack/cmd/audit.py
index 86eea9f7bc..58d7a5362c 100644
--- a/lib/spack/spack/cmd/audit.py
+++ b/lib/spack/spack/cmd/audit.py
@@ -2,6 +2,8 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import warnings
+
 import llnl.util.tty as tty
 import llnl.util.tty.colify
 import llnl.util.tty.color as cl
@@ -52,8 +54,10 @@ def setup_parser(subparser):
 
 
 def configs(parser, args):
-    reports = spack.audit.run_group(args.subcommand)
-    _process_reports(reports)
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
+        reports = spack.audit.run_group(args.subcommand)
+        _process_reports(reports)
 
 
 def packages(parser, args):
diff --git a/lib/spack/spack/schema/packages.py b/lib/spack/spack/schema/packages.py
index 2e651ec798..2802f89529 100644
--- a/lib/spack/spack/schema/packages.py
+++ b/lib/spack/spack/schema/packages.py
@@ -69,6 +69,8 @@
     "patternProperties": {r"\w+": {}},
 }
 
+REQUIREMENT_URL = "https://spack.readthedocs.io/en/latest/packages_yaml.html#package-requirements"
+
 #: Properties for inclusion in other schemas
 properties = {
     "packages": {
@@ -117,7 +119,7 @@
                     "properties": ["version"],
                     "message": "setting version preferences in the 'all' section of packages.yaml "
                     "is deprecated and will be removed in v0.22\n\n\tThese preferences "
-                    "will be ignored by Spack. You can set them only in package specific sections "
+                    "will be ignored by Spack. You can set them only in package-specific sections "
                     "of the same file.\n",
                     "error": False,
                 },
@@ -162,10 +164,14 @@
                 },
                 "deprecatedProperties": {
                     "properties": ["target", "compiler", "providers"],
-                    "message": "setting compiler, target or provider preferences in a package "
-                    "specific section of packages.yaml is deprecated, and will be removed in "
-                    "v0.22.\n\n\tThese preferences will be ignored by Spack. You "
-                    "can set them only in the 'all' section of the same file.\n",
+                    "message": "setting 'compiler:', 'target:' or 'provider:' preferences in "
+                    "a package-specific section of packages.yaml is deprecated, and will be "
+                    "removed in v0.22.\n\n\tThese 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, "
+                    "including files:lines where the deprecated attributes are used.\n\n"
+                    "\tUse requirements to enforce conditions on specific packages: "
+                    f"{REQUIREMENT_URL}\n",
                     "error": False,
                 },
             }