From 4f7cce68b8531eea07eb347b2aee1c4ecbb89b75 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Thu, 23 Nov 2023 11:30:39 +0100
Subject: [PATCH] ASP-based solver: don't error for type mismatch on
 preferences (#41138)

This commit discards type mismatches or failures to validate a package preference during concretization. The values discarded are logged as debug level messages. It also adds a config audit to help users spot misconfigurations in packages.yaml preferences.
---
 lib/spack/spack/audit.py                      | 48 +++++++++++++++++++
 lib/spack/spack/solver/asp.py                 |  8 +++-
 .../spack/test/concretize_preferences.py      | 10 ++++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index d0a68cf212..970e4a3b36 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -286,6 +286,54 @@ def make_error(attribute_name, config_data, summary):
     return errors
 
 
+@config_packages
+def _avoid_mismatched_variants(error_cls):
+    """Warns if variant preferences have mismatched types or names."""
+    errors = []
+    packages_yaml = spack.config.CONFIG.get_config("packages")
+
+    def make_error(config_data, summary):
+        s = io.StringIO()
+        s.write("Occurring in the following file:\n")
+        syaml.dump_config(config_data, stream=s, blame=True)
+        return error_cls(summary=summary, details=[s.getvalue()])
+
+    for pkg_name in packages_yaml:
+        # 'all:' must be more forgiving, since it is setting defaults for everything
+        if pkg_name == "all" or "variants" not in packages_yaml[pkg_name]:
+            continue
+
+        preferences = packages_yaml[pkg_name]["variants"]
+        if not isinstance(preferences, list):
+            preferences = [preferences]
+
+        for variants in preferences:
+            current_spec = spack.spec.Spec(variants)
+            pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
+            for variant in current_spec.variants.values():
+                # Variant does not exist at all
+                if variant.name not in pkg_cls.variants:
+                    summary = (
+                        f"Setting a preference for the '{pkg_name}' package to the "
+                        f"non-existing variant '{variant.name}'"
+                    )
+                    errors.append(make_error(preferences, summary))
+                    continue
+
+                # Variant cannot accept this value
+                s = spack.spec.Spec(pkg_name)
+                try:
+                    s.update_variant_validate(variant.name, variant.value)
+                except Exception:
+                    summary = (
+                        f"Setting the variant '{variant.name}' of the '{pkg_name}' package "
+                        f"to the invalid value '{str(variant)}'"
+                    )
+                    errors.append(make_error(preferences, summary))
+
+    return errors
+
+
 #: Sanity checks on package directives
 package_directives = AuditClass(
     group="packages",
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 7587826f5e..0e298bfb4e 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1828,7 +1828,13 @@ def preferred_variants(self, pkg_name):
 
             # perform validation of the variant and values
             spec = spack.spec.Spec(pkg_name)
-            spec.update_variant_validate(variant_name, values)
+            try:
+                spec.update_variant_validate(variant_name, values)
+            except (spack.variant.InvalidVariantValueError, KeyError, ValueError) as e:
+                tty.debug(
+                    f"[SETUP]: rejected {str(variant)} as a preference for {pkg_name}: {str(e)}"
+                )
+                continue
 
             for value in values:
                 self.variant_values_from_specs.add((pkg_name, variant.name, value))
diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py
index d061f9a8f5..929ae0a9ec 100644
--- a/lib/spack/spack/test/concretize_preferences.py
+++ b/lib/spack/spack/test/concretize_preferences.py
@@ -504,3 +504,13 @@ def test_sticky_variant_accounts_for_packages_yaml(self):
         with spack.config.override("packages:sticky-variant", {"variants": "+allow-gcc"}):
             s = Spec("sticky-variant %gcc").concretized()
             assert s.satisfies("%gcc") and s.satisfies("+allow-gcc")
+
+    @pytest.mark.regression("41134")
+    @pytest.mark.only_clingo("Not backporting the fix to the old concretizer")
+    def test_default_preference_variant_different_type_does_not_error(self):
+        """Tests that a different type for an existing variant in the 'all:' section of
+        packages.yaml doesn't fail with an error.
+        """
+        with spack.config.override("packages:all", {"variants": "+foo"}):
+            s = Spec("a").concretized()
+            assert s.satisfies("foo=bar")