Fix a couple of minor bugs with ASP weights (#36522)

Reorder versions so that deprecated ones are last. 

Account for default not used when the variant exists.

2 files changed, 14 insertions, 23 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 4d409bb8c8..abe325cb86 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1579,7 +1579,12 @@ class SpackSolverSetup(object):
                 # When COMPARING VERSIONS, the '@develop' version is always
                 # larger than other versions. BUT when CONCRETIZING, the largest
                 # NON-develop version is selected by default.
-                return info.get("preferred", False), not version.isdevelop(), version
+                return (
+                    info.get("preferred", False),
+                    not info.get("deprecated", False),
+                    not version.isdevelop(),
+                    version,
+                )
 
             for idx, item in enumerate(sorted(pkg_cls.versions.items(), key=key_fn, reverse=True)):
                 v, version_info = item
@@ -2282,10 +2287,8 @@ class SpecBuilder(object):
     def version(self, pkg, version):
         self._specs[pkg].versions = spack.version.ver([version])
 
-    def node_compiler(self, pkg, compiler):
-        self._specs[pkg].compiler = spack.spec.CompilerSpec(compiler)
-
     def node_compiler_version(self, pkg, compiler, version):
+        self._specs[pkg].compiler = spack.spec.CompilerSpec(compiler)
         self._specs[pkg].compiler.versions = spack.version.VersionList([version])
 
     def node_flag_compiler_default(self, pkg):
@@ -2390,7 +2393,6 @@ class SpecBuilder(object):
 
         hash attributes are handled first, since they imply entire concrete specs
         node attributes are handled next, since they instantiate nodes
-        node_compiler attributes are handled next to ensure they come before node_compiler_version
         external_spec_selected attributes are handled last, so that external extensions can find
         the concrete specs on which they depend because all nodes are fully constructed before we
         consider which ones are external.
@@ -2400,8 +2402,6 @@ class SpecBuilder(object):
             return (-5, 0)
         elif name == "node":
             return (-4, 0)
-        elif name == "node_compiler":
-            return (-3, 0)
         elif name == "node_flag":
             return (-2, 0)
         elif name == "external_spec_selected":
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 1f25612689..60d48b9fae 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -303,8 +303,8 @@ attr("virtual_node", Virtual)
   :- attr("virtual_node", Virtual).
 error(2, "Cannot find valid provider for virtual {0}", Virtual)
   :- attr("virtual_node", Virtual),
-     P = #count{ Package : provider(Package, Virtual)},
-     P < 1.
+     not provider(_, Virtual).
+
 error(2, "Spec cannot include multiple providers for virtual '{0}'\n    Requested '{1}' and '{2}'", Virtual, P1, P2)
   :- attr("virtual_node", Virtual),
      provider(P1, Virtual),
@@ -566,8 +566,7 @@ error(2, "No valid value for variant '{1}' of package '{0}'", Package, Variant)
   :- attr("node", Package),
      variant(Package, Variant),
      build(Package),
-     C = #count{ Value : attr("variant_value", Package, Variant, Value) },
-     C < 1.
+     not attr("variant_value", Package, Variant, _).
 
 % if a variant is set to anything, it is considered 'set'.
 attr("variant_set", Package, Variant) :- attr("variant_set", Package, Variant, _).
@@ -618,6 +617,7 @@ variant_not_default(Package, Variant, Value)
 % A default variant value that is not used
 variant_default_not_used(Package, Variant, Value)
   :- variant_default_value(Package, Variant, Value),
+     variant(Package, Variant),
      not attr("variant_value", Package, Variant, Value),
      attr("node", Package).
 
@@ -701,8 +701,7 @@ attr("node_platform_set", Package) :- attr("node_platform_set", Package, _).
 % each node must have a single platform
 error(2, "No valid platform found for {0}", Package)
   :- attr("node", Package),
-     C = #count{ Platform : attr("node_platform", Package, Platform)},
-     C < 1.
+     not attr("node_platform", Package, _).
 
 error(2, "Cannot concretize {0} with multiple platforms\n    Requested 'platform={1}' and 'platform={2}'", Package, Platform1, Platform2)
   :- attr("node", Package),
@@ -721,8 +720,7 @@ os(OS) :- os(OS, _).
 
 error(2, "Cannot find valid operating system for '{0}'", Package)
   :- attr("node", Package),
-     C = #count{ OS : attr("node_os", Package, OS)},
-     C < 1.
+     not attr("node_os", Package, _).
 
 error(2, "Cannot concretize {0} with multiple operating systems\n    Requested 'os={1}' and 'os={2}'", Package, OS1, OS2)
   :- attr("node", Package),
@@ -783,8 +781,7 @@ attr("node_os", Package, OS) :- attr("node_os_set", Package, OS), attr("node", P
 
 error(2, "Cannot find valid target for '{0}'", Package)
   :- attr("node", Package),
-     C = #count{Target : attr("node_target", Package, Target)},
-     C < 1.
+     not attr("node_target", Package, _).
 
 error(2, "Cannot concretize '{0}' with multiple targets\n    Requested 'target={1}' and 'target={2}'", Package, Target1, Target2)
   :- attr("node", Package),
@@ -885,12 +882,6 @@ error(2, "No valid compiler version found for '{0}'", Package)
   :- attr("node", Package),
      not node_compiler(Package, _).
 
-error(2, "Cannot concretize {0} with two compilers {1}@{2} and {3}@{4}", Package, C1, V1, C2, V2)
-  :- attr("node", Package),
-     attr("node_compiler_version", Package, C1, V1),
-     attr("node_compiler_version", Package, C2, V2),
-     (C1, V1) < (C2, V2). % see[1]
-
 % We can't have a compiler be enforced and select the version from another compiler
 error(2, "Cannot concretize {0} with two compilers {1}@{2} and {3}@{4}", Package, C1, V1, C2, V2)
  :- attr("node_compiler_version", Package, C1, V1),