variant.py: extract spec bits into spec.py (#45941)

lib/spack/spack/package_prefs.py

@@ -5,8 +5,10 @@
 import stat
 import warnings
 
+import spack.config
 import spack.error
 import spack.repo
+import spack.spec
 from spack.config import ConfigError
 from spack.util.path import canonicalize_path
 from spack.version import Version

lib/spack/spack/spec.py

@@ -51,6 +51,7 @@
 import collections
 import collections.abc
 import enum
+import io
 import itertools
 import os
 import pathlib
@@ -1427,7 +1428,7 @@ def __init__(
         # init an empty spec that matches anything.
         self.name = None
         self.versions = vn.VersionList(":")
-        self.variants = vt.VariantMap(self)
+        self.variants = VariantMap(self)
         self.architecture = None
         self.compiler = None
         self.compiler_flags = FlagMap(self)
@@ -2592,7 +2593,7 @@ def from_detection(spec_str, extra_attributes=None):
         extra_attributes = syaml.sorted_dict(extra_attributes or {})
         # This is needed to be able to validate multi-valued variants,
         # otherwise they'll still be abstract in the context of detection.
-        vt.substitute_abstract_variants(s)
+        substitute_abstract_variants(s)
         s.extra_attributes = extra_attributes
         return s
 
@@ -2915,7 +2916,7 @@ def validate_or_raise(self):
             # Ensure correctness of variants (if the spec is not virtual)
             if not spec.virtual:
                 Spec.ensure_valid_variants(spec)
-                vt.substitute_abstract_variants(spec)
+                substitute_abstract_variants(spec)
 
     @staticmethod
     def ensure_valid_variants(spec):
@@ -3884,7 +3885,7 @@ def format_attribute(match_object: Match) -> str:
                 if part.startswith("_"):
                     raise SpecFormatStringError("Attempted to format private attribute")
                 else:
-                    if part == "variants" and isinstance(current, vt.VariantMap):
+                    if part == "variants" and isinstance(current, VariantMap):
                         # subscript instead of getattr for variant names
                         current = current[part]
                     else:
@@ -4339,6 +4340,152 @@ def attach_git_version_lookup(self):
                 v.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(self.fullname))
 
 
+class VariantMap(lang.HashableMap):
+    """Map containing variant instances. New values can be added only
+    if the key is not already present."""
+
+    def __init__(self, spec: Spec):
+        super().__init__()
+        self.spec = spec
+
+    def __setitem__(self, name, vspec):
+        # Raise a TypeError if vspec is not of the right type
+        if not isinstance(vspec, vt.AbstractVariant):
+            raise TypeError(
+                "VariantMap accepts only values of variant types "
+                f"[got {type(vspec).__name__} instead]"
+            )
+
+        # Raise an error if the variant was already in this map
+        if name in self.dict:
+            msg = 'Cannot specify variant "{0}" twice'.format(name)
+            raise vt.DuplicateVariantError(msg)
+
+        # Raise an error if name and vspec.name don't match
+        if name != vspec.name:
+            raise KeyError(
+                f'Inconsistent key "{name}", must be "{vspec.name}" to ' "match VariantSpec"
+            )
+
+        # Set the item
+        super().__setitem__(name, vspec)
+
+    def substitute(self, vspec):
+        """Substitutes the entry under ``vspec.name`` with ``vspec``.
+
+        Args:
+            vspec: variant spec to be substituted
+        """
+        if vspec.name not in self:
+            raise KeyError(f"cannot substitute a key that does not exist [{vspec.name}]")
+
+        # Set the item
+        super().__setitem__(vspec.name, vspec)
+
+    def satisfies(self, other):
+        return all(k in self and self[k].satisfies(other[k]) for k in other)
+
+    def intersects(self, other):
+        return all(self[k].intersects(other[k]) for k in other if k in self)
+
+    def constrain(self, other: "VariantMap") -> bool:
+        """Add all variants in other that aren't in self to self. Also constrain all multi-valued
+        variants that are already present. Return True iff self changed"""
+        if other.spec is not None and other.spec._concrete:
+            for k in self:
+                if k not in other:
+                    raise vt.UnsatisfiableVariantSpecError(self[k], "<absent>")
+
+        changed = False
+        for k in other:
+            if k in self:
+                # If they are not compatible raise an error
+                if not self[k].compatible(other[k]):
+                    raise vt.UnsatisfiableVariantSpecError(self[k], other[k])
+                # If they are compatible merge them
+                changed |= self[k].constrain(other[k])
+            else:
+                # If it is not present copy it straight away
+                self[k] = other[k].copy()
+                changed = True
+
+        return changed
+
+    @property
+    def concrete(self):
+        """Returns True if the spec is concrete in terms of variants.
+
+        Returns:
+            bool: True or False
+        """
+        return self.spec._concrete or all(v in self for v in self.spec.package_class.variants)
+
+    def copy(self) -> "VariantMap":
+        clone = VariantMap(self.spec)
+        for name, variant in self.items():
+            clone[name] = variant.copy()
+        return clone
+
+    def __str__(self):
+        if not self:
+            return ""
+
+        # print keys in order
+        sorted_keys = sorted(self.keys())
+
+        # Separate boolean variants from key-value pairs as they print
+        # differently. All booleans go first to avoid ' ~foo' strings that
+        # break spec reuse in zsh.
+        bool_keys = []
+        kv_keys = []
+        for key in sorted_keys:
+            bool_keys.append(key) if isinstance(self[key].value, bool) else kv_keys.append(key)
+
+        # add spaces before and after key/value variants.
+        string = io.StringIO()
+
+        for key in bool_keys:
+            string.write(str(self[key]))
+
+        for key in kv_keys:
+            string.write(" ")
+            string.write(str(self[key]))
+
+        return string.getvalue()
+
+
+def substitute_abstract_variants(spec: Spec):
+    """Uses the information in `spec.package` to turn any variant that needs
+    it into a SingleValuedVariant.
+
+    This method is best effort. All variants that can be substituted will be
+    substituted before any error is raised.
+
+    Args:
+        spec: spec on which to operate the substitution
+    """
+    # This method needs to be best effort so that it works in matrix exlusion
+    # in $spack/lib/spack/spack/spec_list.py
+    failed = []
+    for name, v in spec.variants.items():
+        if name == "dev_path":
+            spec.variants.substitute(vt.SingleValuedVariant(name, v._original_value))
+            continue
+        elif name in vt.reserved_names:
+            continue
+        elif name not in spec.package_class.variants:
+            failed.append(name)
+            continue
+        pkg_variant, _ = spec.package_class.variants[name]
+        new_variant = pkg_variant.make_variant(v._original_value)
+        pkg_variant.validate_or_raise(new_variant, spec.package_class)
+        spec.variants.substitute(new_variant)
+
+    # Raise all errors at once
+    if failed:
+        raise vt.UnknownVariantError(spec, failed)
+
+
 def parse_with_version_concrete(spec_like: Union[str, Spec], compiler: bool = False):
     """Same as Spec(string), but interprets @x as @=x"""
     s: Union[CompilerSpec, Spec] = CompilerSpec(spec_like) if compiler else Spec(spec_like)

lib/spack/spack/spec_list.py

@@ -5,6 +5,7 @@
 import itertools
 from typing import List
 
+import spack.spec
 import spack.variant
 from spack.error import SpackError
 from spack.spec import Spec
@@ -225,7 +226,7 @@ def _expand_matrix_constraints(matrix_config):
         # Catch exceptions because we want to be able to operate on
         # abstract specs without needing package information
         try:
-            spack.variant.substitute_abstract_variants(test_spec)
+            spack.spec.substitute_abstract_variants(test_spec)
         except spack.variant.UnknownVariantError:
             pass
 

lib/spack/spack/test/variant.py

@@ -8,6 +8,7 @@
 
 import spack.error
 import spack.variant
+from spack.spec import VariantMap
 from spack.variant import (
     BoolValuedVariant,
     DuplicateVariantError,
@@ -18,7 +19,6 @@
     SingleValuedVariant,
     UnsatisfiableVariantSpecError,
     Variant,
-    VariantMap,
     disjoint_sets,
 )
 

lib/spack/spack/variant.py

@@ -9,7 +9,6 @@
 import collections.abc
 import functools
 import inspect
-import io
 import itertools
 import re
 
@@ -550,165 +549,6 @@ def __str__(self):
         return "{0}{1}".format("+" if self.value else "~", self.name)
 
 
-class VariantMap(lang.HashableMap):
-    """Map containing variant instances. New values can be added only
-    if the key is not already present.
-    """
-
-    def __init__(self, spec):
-        super().__init__()
-        self.spec = spec
-
-    def __setitem__(self, name, vspec):
-        # Raise a TypeError if vspec is not of the right type
-        if not isinstance(vspec, AbstractVariant):
-            msg = "VariantMap accepts only values of variant types"
-            msg += " [got {0} instead]".format(type(vspec).__name__)
-            raise TypeError(msg)
-
-        # Raise an error if the variant was already in this map
-        if name in self.dict:
-            msg = 'Cannot specify variant "{0}" twice'.format(name)
-            raise DuplicateVariantError(msg)
-
-        # Raise an error if name and vspec.name don't match
-        if name != vspec.name:
-            msg = 'Inconsistent key "{0}", must be "{1}" to match VariantSpec'
-            raise KeyError(msg.format(name, vspec.name))
-
-        # Set the item
-        super().__setitem__(name, vspec)
-
-    def substitute(self, vspec):
-        """Substitutes the entry under ``vspec.name`` with ``vspec``.
-
-        Args:
-            vspec: variant spec to be substituted
-        """
-        if vspec.name not in self:
-            msg = "cannot substitute a key that does not exist [{0}]"
-            raise KeyError(msg.format(vspec.name))
-
-        # Set the item
-        super().__setitem__(vspec.name, vspec)
-
-    def satisfies(self, other):
-        return all(k in self and self[k].satisfies(other[k]) for k in other)
-
-    def intersects(self, other):
-        return all(self[k].intersects(other[k]) for k in other if k in self)
-
-    def constrain(self, other):
-        """Add all variants in other that aren't in self to self. Also
-        constrain all multi-valued variants that are already present.
-        Return True if self changed, False otherwise
-
-        Args:
-            other (VariantMap): instance against which we constrain self
-
-        Returns:
-            bool: True or False
-        """
-        if other.spec is not None and other.spec._concrete:
-            for k in self:
-                if k not in other:
-                    raise UnsatisfiableVariantSpecError(self[k], "<absent>")
-
-        changed = False
-        for k in other:
-            if k in self:
-                # If they are not compatible raise an error
-                if not self[k].compatible(other[k]):
-                    raise UnsatisfiableVariantSpecError(self[k], other[k])
-                # If they are compatible merge them
-                changed |= self[k].constrain(other[k])
-            else:
-                # If it is not present copy it straight away
-                self[k] = other[k].copy()
-                changed = True
-
-        return changed
-
-    @property
-    def concrete(self):
-        """Returns True if the spec is concrete in terms of variants.
-
-        Returns:
-            bool: True or False
-        """
-        return self.spec._concrete or all(v in self for v in self.spec.package_class.variants)
-
-    def copy(self):
-        """Return an instance of VariantMap equivalent to self.
-
-        Returns:
-            VariantMap: a copy of self
-        """
-        clone = VariantMap(self.spec)
-        for name, variant in self.items():
-            clone[name] = variant.copy()
-        return clone
-
-    def __str__(self):
-        if not self:
-            return ""
-
-        # print keys in order
-        sorted_keys = sorted(self.keys())
-
-        # Separate boolean variants from key-value pairs as they print
-        # differently. All booleans go first to avoid ' ~foo' strings that
-        # break spec reuse in zsh.
-        bool_keys = []
-        kv_keys = []
-        for key in sorted_keys:
-            bool_keys.append(key) if isinstance(self[key].value, bool) else kv_keys.append(key)
-
-        # add spaces before and after key/value variants.
-        string = io.StringIO()
-
-        for key in bool_keys:
-            string.write(str(self[key]))
-
-        for key in kv_keys:
-            string.write(" ")
-            string.write(str(self[key]))
-
-        return string.getvalue()
-
-
-def substitute_abstract_variants(spec):
-    """Uses the information in `spec.package` to turn any variant that needs
-    it into a SingleValuedVariant.
-
-    This method is best effort. All variants that can be substituted will be
-    substituted before any error is raised.
-
-    Args:
-        spec: spec on which to operate the substitution
-    """
-    # This method needs to be best effort so that it works in matrix exlusion
-    # in $spack/lib/spack/spack/spec_list.py
-    failed = []
-    for name, v in spec.variants.items():
-        if name in reserved_names:
-            if name == "dev_path":
-                new_variant = SingleValuedVariant(name, v._original_value)
-                spec.variants.substitute(new_variant)
-            continue
-        if name not in spec.package_class.variants:
-            failed.append(name)
-            continue
-        pkg_variant, _ = spec.package_class.variants[name]
-        new_variant = pkg_variant.make_variant(v._original_value)
-        pkg_variant.validate_or_raise(new_variant, spec.package_class)
-        spec.variants.substitute(new_variant)
-
-    # Raise all errors at once
-    if failed:
-        raise UnknownVariantError(spec, failed)
-
-
 # The class below inherit from Sequence to disguise as a tuple and comply
 # with the semantic expected by the 'values' argument of the variant directive
 class DisjointSetsOfValues(collections.abc.Sequence):