diff options
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/audit.py | 140 | ||||
-rw-r--r-- | lib/spack/spack/build_systems/autotools.py | 23 | ||||
-rw-r--r-- | lib/spack/spack/build_systems/cached_cmake.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/build_systems/cmake.py | 3 | ||||
-rw-r--r-- | lib/spack/spack/cmd/config.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/cmd/info.py | 51 | ||||
-rw-r--r-- | lib/spack/spack/cray_manifest.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/directives.py | 24 | ||||
-rw-r--r-- | lib/spack/spack/package_base.py | 110 | ||||
-rw-r--r-- | lib/spack/spack/package_prefs.py | 10 | ||||
-rw-r--r-- | lib/spack/spack/solver/asp.py | 358 | ||||
-rw-r--r-- | lib/spack/spack/solver/concretize.lp | 219 | ||||
-rw-r--r-- | lib/spack/spack/solver/display.lp | 4 | ||||
-rw-r--r-- | lib/spack/spack/solver/error_messages.lp | 18 | ||||
-rw-r--r-- | lib/spack/spack/spec.py | 106 | ||||
-rw-r--r-- | lib/spack/spack/test/concretize.py | 8 | ||||
-rw-r--r-- | lib/spack/spack/test/spec_semantics.py | 26 | ||||
-rw-r--r-- | lib/spack/spack/test/variant.py | 224 | ||||
-rw-r--r-- | lib/spack/spack/variant.py | 421 |
19 files changed, 1122 insertions, 633 deletions
diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 486e3c3d65..6b441c6965 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -282,7 +282,7 @@ def _avoid_mismatched_variants(error_cls): 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: + if variant.name not in pkg_cls.variant_names(): summary = ( f"Setting a preference for the '{pkg_name}' package to the " f"non-existing variant '{variant.name}'" @@ -291,9 +291,8 @@ def _avoid_mismatched_variants(error_cls): continue # Variant cannot accept this value - s = spack.spec.Spec(pkg_name) try: - s.update_variant_validate(variant.name, variant.value) + spack.variant.prevalidate_variant_value(pkg_cls, variant, strict=True) except Exception: summary = ( f"Setting the variant '{variant.name}' of the '{pkg_name}' package " @@ -663,9 +662,15 @@ def _ensure_env_methods_are_ported_to_builders(pkgs, error_cls): errors = [] for pkg_name in pkgs: pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) - buildsystem_variant, _ = pkg_cls.variants["build_system"] - buildsystem_names = [getattr(x, "value", x) for x in buildsystem_variant.values] - builder_cls_names = [spack.builder.BUILDER_CLS[x].__name__ for x in buildsystem_names] + + # values are either Value objects (for conditional values) or the values themselves + build_system_names = set( + v.value if isinstance(v, spack.variant.Value) else v + for _, variant in pkg_cls.variant_definitions("build_system") + for v in variant.values + ) + builder_cls_names = [spack.builder.BUILDER_CLS[x].__name__ for x in build_system_names] + module = pkg_cls.module has_builders_in_package_py = any( getattr(module, name, False) for name in builder_cls_names @@ -932,20 +937,22 @@ def _issues_in_depends_on_directive(pkgs, error_cls): # check variants dependency_variants = dep.spec.variants - for name, value in dependency_variants.items(): + for name, variant in dependency_variants.items(): try: - v, _ = dependency_pkg_cls.variants[name] - v.validate_or_raise(value, pkg_cls=dependency_pkg_cls) + spack.variant.prevalidate_variant_value( + dependency_pkg_cls, variant, dep.spec, strict=True + ) except Exception as e: summary = ( f"{pkg_name}: wrong variant used for dependency in 'depends_on()'" ) + error_msg = str(e) if isinstance(e, KeyError): error_msg = ( f"variant {str(e).strip()} does not exist in package {dep_name}" + f" in package '{dep_name}'" ) - error_msg += f" in package '{dep_name}'" errors.append( error_cls(summary=summary, details=[error_msg, f"in {filename}"]) @@ -957,39 +964,38 @@ def _issues_in_depends_on_directive(pkgs, error_cls): @package_directives def _ensure_variant_defaults_are_parsable(pkgs, error_cls): """Ensures that variant defaults are present and parsable from cli""" - errors = [] - for pkg_name in pkgs: - pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) - for variant_name, entry in pkg_cls.variants.items(): - variant, _ = entry - default_is_parsable = ( - # Permitting a default that is an instance on 'int' permits - # to have foo=false or foo=0. Other falsish values are - # not allowed, since they can't be parsed from cli ('foo=') - isinstance(variant.default, int) - or variant.default - ) - if not default_is_parsable: - error_msg = "Variant '{}' of package '{}' has a bad default value" - errors.append(error_cls(error_msg.format(variant_name, pkg_name), [])) - continue - try: - vspec = variant.make_default() - except spack.variant.MultipleValuesInExclusiveVariantError: - error_msg = "Cannot create a default value for the variant '{}' in package '{}'" - errors.append(error_cls(error_msg.format(variant_name, pkg_name), [])) - continue + def check_variant(pkg_cls, variant, vname): + # bool is a subclass of int in python. Permitting a default that is an instance + # of 'int' means both foo=false and foo=0 are accepted. Other falsish values are + # not allowed, since they can't be parsed from CLI ('foo=') + default_is_parsable = isinstance(variant.default, int) or variant.default - try: - variant.validate_or_raise(vspec, pkg_cls=pkg_cls) - except spack.variant.InvalidVariantValueError: - error_msg = ( - "The default value of the variant '{}' in package '{}' failed validation" - ) - question = "Is it among the allowed values?" - errors.append(error_cls(error_msg.format(variant_name, pkg_name), [question])) + if not default_is_parsable: + msg = f"Variant '{vname}' of package '{pkg_cls.name}' has an unparsable default value" + return [error_cls(msg, [])] + + try: + vspec = variant.make_default() + except spack.variant.MultipleValuesInExclusiveVariantError: + msg = f"Can't create default value for variant '{vname}' in package '{pkg_cls.name}'" + return [error_cls(msg, [])] + + try: + variant.validate_or_raise(vspec, pkg_cls.name) + except spack.variant.InvalidVariantValueError: + msg = "Default value of variant '{vname}' in package '{pkg.name}' is invalid" + question = "Is it among the allowed values?" + return [error_cls(msg, [question])] + + return [] + errors = [] + for pkg_name in pkgs: + pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) + for vname in pkg_cls.variant_names(): + for _, variant_def in pkg_cls.variant_definitions(vname): + errors.extend(check_variant(pkg_cls, variant_def, vname)) return errors @@ -999,11 +1005,11 @@ def _ensure_variants_have_descriptions(pkgs, error_cls): errors = [] for pkg_name in pkgs: pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) - for variant_name, entry in pkg_cls.variants.items(): - variant, _ = entry - if not variant.description: - error_msg = "Variant '{}' in package '{}' is missing a description" - errors.append(error_cls(error_msg.format(variant_name, pkg_name), [])) + for name in pkg_cls.variant_names(): + for when, variant in pkg_cls.variant_definitions(name): + if not variant.description: + msg = f"Variant '{name}' in package '{pkg_name}' is missing a description" + errors.append(error_cls(msg, [])) return errors @@ -1060,29 +1066,26 @@ def _version_constraints_are_satisfiable_by_some_version_in_repo(pkgs, error_cls def _analyze_variants_in_directive(pkg, constraint, directive, error_cls): - variant_exceptions = ( - spack.variant.InconsistentValidationError, - spack.variant.MultipleValuesInExclusiveVariantError, - spack.variant.InvalidVariantValueError, - KeyError, - ) errors = [] - for name, v in constraint.variants.items(): - try: - variant, _ = pkg.variants[name] - variant.validate_or_raise(v, pkg_cls=pkg) - except variant_exceptions as e: - summary = pkg.name + ': wrong variant in "{0}" directive' - summary = summary.format(directive) - filename = spack.repo.PATH.filename_for_package_name(pkg.name) - - error_msg = str(e).strip() - if isinstance(e, KeyError): - error_msg = "the variant {0} does not exist".format(error_msg) + variant_names = pkg.variant_names() + summary = f"{pkg.name}: wrong variant in '{directive}' directive" + filename = spack.repo.PATH.filename_for_package_name(pkg.name) - err = error_cls(summary=summary, details=[error_msg, "in " + filename]) + for name, v in constraint.variants.items(): + if name not in variant_names: + msg = f"variant {name} does not exist in {pkg.name}" + errors.append(error_cls(summary=summary, details=[msg, f"in {filename}"])) + continue - errors.append(err) + try: + spack.variant.prevalidate_variant_value(pkg, v, constraint, strict=True) + except ( + spack.variant.InconsistentValidationError, + spack.variant.MultipleValuesInExclusiveVariantError, + spack.variant.InvalidVariantValueError, + ) as e: + msg = str(e).strip() + errors.append(error_cls(summary=summary, details=[msg, f"in {filename}"])) return errors @@ -1120,9 +1123,10 @@ def _named_specs_in_when_arguments(pkgs, error_cls): for dname in dnames ) - for vname, (variant, triggers) in pkg_cls.variants.items(): - summary = f"{pkg_name}: wrong 'when=' condition for the '{vname}' variant" - errors.extend(_extracts_errors(triggers, summary)) + for when, variants_by_name in pkg_cls.variants.items(): + for vname, variant in variants_by_name.items(): + summary = f"{pkg_name}: wrong 'when=' condition for the '{vname}' variant" + errors.extend(_extracts_errors([when], summary)) for when, providers, details in _error_items(pkg_cls.provided): errors.extend( diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index d5ddcea11a..47911271fe 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -687,9 +687,8 @@ To resolve this problem, please try the following: variant = variant or name - # Defensively look that the name passed as argument is among - # variants - if variant not in self.pkg.variants: + # Defensively look that the name passed as argument is among variants + if not self.pkg.has_variant(variant): msg = '"{0}" is not a variant of "{1}"' raise KeyError(msg.format(variant, self.pkg.name)) @@ -698,27 +697,19 @@ To resolve this problem, please try the following: # Create a list of pairs. Each pair includes a configuration # option and whether or not that option is activated - variant_desc, _ = self.pkg.variants[variant] - if set(variant_desc.values) == set((True, False)): + vdef = self.pkg.get_variant(variant) + if set(vdef.values) == set((True, False)): # BoolValuedVariant carry information about a single option. # Nonetheless, for uniformity of treatment we'll package them # in an iterable of one element. - condition = "+{name}".format(name=variant) - options = [(name, condition in spec)] + options = [(name, f"+{variant}" in spec)] else: - condition = "{variant}={value}" # "feature_values" is used to track values which correspond to # features which can be enabled or disabled as understood by the # package's build system. It excludes values which have special # meanings and do not correspond to features (e.g. "none") - feature_values = ( - getattr(variant_desc.values, "feature_values", None) or variant_desc.values - ) - - options = [ - (value, condition.format(variant=variant, value=value) in spec) - for value in feature_values - ] + feature_values = getattr(vdef.values, "feature_values", None) or vdef.values + options = [(value, f"{variant}={value}" in spec) for value in feature_values] # For each allowed value in the list of values for option_value, activated in options: diff --git a/lib/spack/spack/build_systems/cached_cmake.py b/lib/spack/spack/build_systems/cached_cmake.py index aff54d7559..3706943704 100644 --- a/lib/spack/spack/build_systems/cached_cmake.py +++ b/lib/spack/spack/build_systems/cached_cmake.py @@ -89,7 +89,7 @@ class CachedCMakeBuilder(CMakeBuilder): if variant is None: variant = cmake_var.lower() - if variant not in self.pkg.variants: + if not self.pkg.has_variant(variant): raise KeyError('"{0}" is not a variant of "{1}"'.format(variant, self.pkg.name)) if variant not in self.pkg.spec.variants: diff --git a/lib/spack/spack/build_systems/cmake.py b/lib/spack/spack/build_systems/cmake.py index dc833a10a6..93d3485ae0 100644 --- a/lib/spack/spack/build_systems/cmake.py +++ b/lib/spack/spack/build_systems/cmake.py @@ -146,6 +146,7 @@ def generator(*names: str, default: Optional[str] = None): default=default, values=_values, description="the build system generator to use", + when="build_system=cmake", ) for x in not_used: conflicts(f"generator={x}") @@ -505,7 +506,7 @@ class CMakeBuilder(BaseBuilder): if variant is None: variant = cmake_var.lower() - if variant not in self.pkg.variants: + if not self.pkg.has_variant(variant): raise KeyError('"{0}" is not a variant of "{1}"'.format(variant, self.pkg.name)) if variant not in self.pkg.spec.variants: diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py index 5c2a02f042..aef2d7a541 100644 --- a/lib/spack/spack/cmd/config.py +++ b/lib/spack/spack/cmd/config.py @@ -536,11 +536,11 @@ def config_prefer_upstream(args): # Get and list all the variants that differ from the default. variants = [] for var_name, variant in spec.variants.items(): - if var_name in ["patches"] or var_name not in spec.package.variants: + if var_name in ["patches"] or not spec.package.has_variant(var_name): continue - variant_desc, _ = spec.package.variants[var_name] - if variant.value != variant_desc.default: + vdef = spec.package.get_variant(var_name) + if variant.value != vdef.default: variants.append(str(variant)) variants.sort() variants = " ".join(variants) diff --git a/lib/spack/spack/cmd/info.py b/lib/spack/spack/cmd/info.py index 5ea99caa3a..a7cdf608a2 100644 --- a/lib/spack/spack/cmd/info.py +++ b/lib/spack/spack/cmd/info.py @@ -334,26 +334,6 @@ def _fmt_variant(variant, max_name_default_len, indent, when=None, out=None): out.write("\n") -def _variants_by_name_when(pkg): - """Adaptor to get variants keyed by { name: { when: { [Variant...] } }.""" - # TODO: replace with pkg.variants_by_name(when=True) when unified directive dicts are merged. - variants = {} - for name, (variant, whens) in sorted(pkg.variants.items()): - for when in whens: - variants.setdefault(name, {}).setdefault(when, []).append(variant) - return variants - - -def _variants_by_when_name(pkg): - """Adaptor to get variants keyed by { when: { name: Variant } }""" - # TODO: replace with pkg.variants when unified directive dicts are merged. - variants = {} - for name, (variant, whens) in pkg.variants.items(): - for when in whens: - variants.setdefault(when, {})[name] = variant - return variants - - def _print_variants_header(pkg): """output variants""" @@ -364,32 +344,22 @@ def _print_variants_header(pkg): color.cprint("") color.cprint(section_title("Variants:")) - variants_by_name = _variants_by_name_when(pkg) - # Calculate the max length of the "name [default]" part of the variant display # This lets us know where to print variant values. max_name_default_len = max( color.clen(_fmt_name_and_default(variant)) - for name, when_variants in variants_by_name.items() - for variants in when_variants.values() - for variant in variants + for name in pkg.variant_names() + for _, variant in pkg.variant_definitions(name) ) - return max_name_default_len, variants_by_name - - -def _unconstrained_ver_first(item): - """sort key that puts specs with open version ranges first""" - spec, _ = item - return (spack.version.any_version not in spec.versions, spec) + return max_name_default_len def print_variants_grouped_by_when(pkg): - max_name_default_len, _ = _print_variants_header(pkg) + max_name_default_len = _print_variants_header(pkg) indent = 4 - variants = _variants_by_when_name(pkg) - for when, variants_by_name in sorted(variants.items(), key=_unconstrained_ver_first): + for when, variants_by_name in pkg.variant_items(): padded_values = max_name_default_len + 4 start_indent = indent @@ -407,15 +377,14 @@ def print_variants_grouped_by_when(pkg): def print_variants_by_name(pkg): - max_name_default_len, variants_by_name = _print_variants_header(pkg) + max_name_default_len = _print_variants_header(pkg) max_name_default_len += 4 indent = 4 - for name, when_variants in variants_by_name.items(): - for when, variants in sorted(when_variants.items(), key=_unconstrained_ver_first): - for variant in variants: - _fmt_variant(variant, max_name_default_len, indent, when, out=sys.stdout) - sys.stdout.write("\n") + for name in pkg.variant_names(): + for when, variant in pkg.variant_definitions(name): + _fmt_variant(variant, max_name_default_len, indent, when, out=sys.stdout) + sys.stdout.write("\n") def print_variants(pkg, args): diff --git a/lib/spack/spack/cray_manifest.py b/lib/spack/spack/cray_manifest.py index f71cf272b6..41767bdf06 100644 --- a/lib/spack/spack/cray_manifest.py +++ b/lib/spack/spack/cray_manifest.py @@ -132,7 +132,7 @@ def spec_from_entry(entry): variant_strs = list() for name, value in entry["parameters"].items(): # TODO: also ensure that the variant value is valid? - if not (name in pkg_cls.variants): + if not pkg_cls.has_variant(name): tty.debug( "Omitting variant {0} for entry {1}/{2}".format( name, entry["name"], entry["hash"][:7] diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 7119339d5a..8f9e43bf8b 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -78,7 +78,6 @@ __all__ = [ "redistribute", ] - _patch_order_index = 0 @@ -674,22 +673,25 @@ def variant( def _execute_variant(pkg): when_spec = _make_when_spec(when) - when_specs = [when_spec] if not re.match(spack.spec.IDENTIFIER_RE, name): directive = "variant" msg = "Invalid variant name in {0}: '{1}'" raise DirectiveError(directive, msg.format(pkg.name, name)) - if name in pkg.variants: - # We accumulate when specs, but replace the rest of the variant - # with the newer values - _, orig_when = pkg.variants[name] - when_specs += orig_when - - pkg.variants[name] = ( - spack.variant.Variant(name, default, description, values, multi, validator, sticky), - when_specs, + # variants are stored by condition then by name (so only the last variant of a + # given name takes precedence *per condition*). + # NOTE: variant defaults and values can conflict if when conditions overlap. + variants_by_name = pkg.variants.setdefault(when_spec, {}) + variants_by_name[name] = spack.variant.Variant( + name=name, + default=default, + description=description, + values=values, + multi=multi, + validator=validator, + sticky=sticky, + precedence=pkg.num_variant_definitions(), ) return _execute_variant diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index c539cf1e0e..5be23ff124 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -451,10 +451,11 @@ def _by_name( else: all_by_name.setdefault(name, []).append(value) + # this needs to preserve the insertion order of whens return dict(sorted(all_by_name.items())) -def _names(when_indexed_dictionary): +def _names(when_indexed_dictionary: WhenDict) -> List[str]: """Get sorted names from dicts keyed by when/name.""" all_names = set() for when, by_name in when_indexed_dictionary.items(): @@ -464,6 +465,45 @@ def _names(when_indexed_dictionary): return sorted(all_names) +WhenVariantList = List[Tuple["spack.spec.Spec", "spack.variant.Variant"]] + + +def _remove_overridden_vdefs(variant_defs: WhenVariantList) -> None: + """Remove variant defs from the list if their when specs are satisfied by later ones. + + Any such variant definitions are *always* overridden by their successor, as it will + match everything the predecessor matches, and the solver will prefer it because of + its higher precedence. + + We can just remove these defs from variant definitions and avoid putting them in the + solver. This is also useful for, e.g., `spack info`, where we don't want to show a + variant from a superclass if it is always overridden by a variant defined in a + subclass. + + Example:: + + class ROCmPackage: + variant("amdgpu_target", ..., when="+rocm") + + class Hipblas: + variant("amdgpu_target", ...) + + The subclass definition *always* overrides the superclass definition here, but they + have different when specs and the subclass def won't just replace the one in the + superclass. In this situation, the subclass should *probably* also have + ``when="+rocm"``, but we can't guarantee that will always happen when a vdef is + overridden. So we use this method to remove any overrides we can know statically. + + """ + i = 0 + while i < len(variant_defs): + when, vdef = variant_defs[i] + if any(when.satisfies(successor) for successor, _ in variant_defs[i + 1 :]): + del variant_defs[i] + else: + i += 1 + + class RedistributionMixin: """Logic for determining whether a Package is source/binary redistributable. @@ -596,7 +636,7 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass provided: Dict["spack.spec.Spec", Set["spack.spec.Spec"]] provided_together: Dict["spack.spec.Spec", List[Set[str]]] patches: Dict["spack.spec.Spec", List["spack.patch.Patch"]] - variants: Dict[str, Tuple["spack.variant.Variant", "spack.spec.Spec"]] + variants: Dict["spack.spec.Spec", Dict[str, "spack.variant.Variant"]] languages: Dict["spack.spec.Spec", Set[str]] #: By default, packages are not virtual @@ -750,6 +790,72 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass def dependencies_by_name(cls, when: bool = False): return _by_name(cls.dependencies, when=when) + # Accessors for variants + # External code workingw with Variants should go through the methods below + + @classmethod + def variant_names(cls) -> List[str]: + return _names(cls.variants) + + @classmethod + def has_variant(cls, name) -> bool: + return any(name in dictionary for dictionary in cls.variants.values()) + + @classmethod + def num_variant_definitions(cls) -> int: + """Total number of variant definitions in this class so far.""" + return sum(len(variants_by_name) for variants_by_name in cls.variants.values()) + + @classmethod + def variant_definitions(cls, name: str) -> WhenVariantList: + """Iterator over (when_spec, Variant) for all variant definitions for a particular name.""" + # construct a list of defs sorted by precedence + defs: WhenVariantList = [] + for when, variants_by_name in cls.variants.items(): + variant_def = variants_by_name.get(name) + if variant_def: + defs.append((when, variant_def)) + + # With multiple definitions, ensure precedence order and simplify overrides + if len(defs) > 1: + defs.sort(key=lambda v: v[1].precedence) + _remove_overridden_vdefs(defs) + + return defs + + @classmethod + def variant_items( + cls, + ) -> Iterable[Tuple["spack.spec.Spec", Dict[str, "spack.variant.Variant"]]]: + """Iterate over ``cls.variants.items()`` with overridden definitions removed.""" + # Note: This is quadratic in the average number of variant definitions per name. + # That is likely close to linear in practice, as there are few variants with + # multiple definitions (but it matters when they are there). + exclude = { + name: [id(vdef) for _, vdef in cls.variant_definitions(name)] + for name in cls.variant_names() + } + + for when, variants_by_name in cls.variants.items(): + filtered_variants_by_name = { + name: vdef for name, vdef in variants_by_name.items() if id(vdef) in exclude[name] + } + + if filtered_variants_by_name: + yield when, filtered_variants_by_name + + def get_variant(self, name: str) -> "spack.variant.Variant": + """Get the highest precedence variant definition matching this package's spec. + + Arguments: + name: name of the variant definition to get + """ + try: + highest_to_lowest = reversed(self.variant_definitions(name)) + return next(vdef for when, vdef in highest_to_lowest if self.spec.satisfies(when)) + except StopIteration: + raise ValueError(f"No variant '{name}' on spec: {self.spec}") + @classmethod def possible_dependencies( cls, diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py index f655fbb885..0204e156f1 100644 --- a/lib/spack/spack/package_prefs.py +++ b/lib/spack/spack/package_prefs.py @@ -149,10 +149,12 @@ class PackagePrefs: # Only return variants that are actually supported by the package pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) - spec = spack.spec.Spec("%s %s" % (pkg_name, variants)) - return dict( - (name, variant) for name, variant in spec.variants.items() if name in pkg_cls.variants - ) + spec = spack.spec.Spec(f"{pkg_name} {variants}") + return { + name: variant + for name, variant in spec.variants.items() + if name in pkg_cls.variant_names() + } def is_spec_buildable(spec): diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 62557d311a..d8d052f311 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -44,7 +44,7 @@ import spack.util.crypto import spack.util.libc import spack.util.path import spack.util.timer -import spack.variant +import spack.variant as vt import spack.version as vn import spack.version.git_ref_lookup from spack import traverse @@ -127,8 +127,14 @@ class Provenance(enum.IntEnum): @contextmanager -def spec_with_name(spec, name): +def named_spec( + spec: Optional["spack.spec.Spec"], name: Optional[str] +) -> Iterator[Optional["spack.spec.Spec"]]: """Context manager to temporarily set the name of a spec""" + if spec is None or name is None: + yield spec + return + old_name = spec.name spec.name = name try: @@ -1128,6 +1134,7 @@ class SpackSolverSetup: self.default_targets: List = [] self.compiler_version_constraints: Set = set() self.post_facts: List = [] + self.variant_ids_by_def_id: Dict[int, int] = {} self.reusable_and_possible: ConcreteSpecsByHash = ConcreteSpecsByHash() @@ -1218,7 +1225,7 @@ class SpackSolverSetup: def conflict_rules(self, pkg): for when_spec, conflict_specs in pkg.conflicts.items(): when_spec_msg = "conflict constraint %s" % str(when_spec) - when_spec_id = self.condition(when_spec, name=pkg.name, msg=when_spec_msg) + when_spec_id = self.condition(when_spec, required_name=pkg.name, msg=when_spec_msg) for conflict_spec, conflict_msg in conflict_specs: conflict_spec = spack.spec.Spec(conflict_spec) @@ -1234,7 +1241,9 @@ class SpackSolverSetup: spec_for_msg = spack.spec.Spec(pkg.name) conflict_spec_msg = f"conflict is triggered when {str(spec_for_msg)}" conflict_spec_id = self.condition( - conflict_spec, name=conflict_spec.name or pkg.name, msg=conflict_spec_msg + conflict_spec, + required_name=conflict_spec.name or pkg.name, + msg=conflict_spec_msg, ) self.gen.fact( fn.pkg_fact( @@ -1248,7 +1257,7 @@ class SpackSolverSetup: condition_msg = f"{pkg.name} needs the {', '.join(sorted(languages))} language" if when_spec != spack.spec.Spec(): condition_msg += f" when {when_spec}" - condition_id = self.condition(when_spec, name=pkg.name, msg=condition_msg) + condition_id = self.condition(when_spec, required_name=pkg.name, msg=condition_msg) for language in sorted(languages): self.gen.fact(fn.pkg_fact(pkg.name, fn.language(condition_id, language))) self.gen.newline() @@ -1363,96 +1372,116 @@ class SpackSolverSetup: self.gen.newline() self._effect_cache.clear() - def variant_rules(self, pkg): - for name, entry in sorted(pkg.variants.items()): - variant, when = entry + def define_variant( + self, + pkg: "Type[spack.package_base.PackageBase]", + name: str, + when: spack.spec.Spec, + variant_def: vt.Variant, + ): + pkg_fact = lambda f: self.gen.fact(fn.pkg_fact(pkg.name, f)) - if spack.spec.Spec() in when: - # unconditional variant - self.gen.fact(fn.pkg_fact(pkg.name, fn.variant(name))) - else: - # conditional variant - for w in when: - msg = "%s has variant %s" % (pkg.name, name) - if str(w): - msg += " when %s" % w - - cond_id = self.condition(w, name=pkg.name, msg=msg) - self.gen.fact(fn.pkg_fact(pkg.name, fn.conditional_variant(cond_id, name))) - - single_value = not variant.multi - if single_value: - self.gen.fact(fn.pkg_fact(pkg.name, fn.variant_single_value(name))) - self.gen.fact( - fn.pkg_fact( - pkg.name, fn.variant_default_value_from_package_py(name, variant.default) - ) + # Every variant id has a unique definition (conditional or unconditional), and + # higher variant id definitions take precedence when variants intersect. + vid = next(self._id_counter) + + # used to find a variant id from its variant definition (for variant values on specs) + self.variant_ids_by_def_id[id(variant_def)] = vid + + if when == spack.spec.Spec(): + # unconditional variant + pkg_fact(fn.variant_definition(name, vid)) + else: + # conditional variant + msg = f"Package {pkg.name} has variant '{name}' when {when}" + cond_id = self.condition(when, required_name=pkg.name, msg=msg) + pkg_fact(fn.variant_condition(name, vid, cond_id)) + + # record type so we can construct the variant when we read it back in + self.gen.fact(fn.variant_type(vid, variant_def.variant_type.value)) + + if variant_def.sticky: + pkg_fact(fn.variant_sticky(vid)) + + # define defaults for this variant definition + defaults = variant_def.make_default().value if variant_def.multi else [variant_def.default] + for val in sorted(defaults): + pkg_fact(fn.variant_default_value_from_package_py(vid, val)) + + # define possible values for this variant definition + values = variant_def.values + if values is None: + values = [] + + elif isinstance(values, vt.DisjointSetsOfValues): + union = set() + for sid, s in enumerate(values.sets): + for value in s: + pkg_fact(fn.variant_value_from_disjoint_sets(vid, value, sid)) + union.update(s) + values = union + + # ensure that every variant has at least one possible value. + if not values: + values = [variant_def.default] + + for value in sorted(values): + pkg_fact(fn.variant_possible_value(vid, value)) + + # when=True means unconditional, so no need for conditional values + if getattr(value, "when", True) is True: + continue + + # now we have to handle conditional values + quoted_value = spack.parser.quote_if_needed(str(value)) + vstring = f"{name}={quoted_value}" + variant_has_value = spack.spec.Spec(vstring) + + if value.when: + # the conditional value is always "possible", but it imposes its when condition as + # a constraint if the conditional value is taken. This may seem backwards, but it + # ensures that the conditional can only occur when its condition holds. + self.condition( + required_spec=variant_has_value, + imposed_spec=value.when, + required_name=pkg.name, + imposed_name=pkg.name, + msg=f"{pkg.name} variant {name} has value '{quoted_value}' when {value.when}", ) else: - spec_variant = variant.make_default() - defaults = spec_variant.value - for val in sorted(defaults): - self.gen.fact( - fn.pkg_fact(pkg.name, fn.variant_default_value_from_package_py(name, val)) - ) + # We know the value is never allowed statically (when was false), but we can't just + # ignore it b/c it could come in as a possible value and we need a good error msg. + # So, it's a conflict -- if the value is somehow used, it'll trigger an error. + trigger_id = self.condition( + variant_has_value, + required_name=pkg.name, + msg=f"invalid variant value: {vstring}", + ) + constraint_id = self.condition( + spack.spec.Spec(), + required_name=pkg.name, + msg="empty (total) conflict constraint", + ) + msg = f"variant value {vstring} is conditionally disabled" + pkg_fact(fn.conflict(trigger_id, constraint_id, msg)) - values = variant.values - if values is None: - values = [] - elif isinstance(values, spack.variant.DisjointSetsOfValues): - union = set() - # Encode the disjoint sets in the logic program - for sid, s in enumerate(values.sets): - for value in s: - self.gen.fact( - fn.pkg_fact( - pkg.name, fn.variant_value_from_disjoint_sets(name, value, sid) - ) - ) - union.update(s) - values = union - - # make sure that every variant has at least one possible value - if not values: - values = [variant.default] - - for value in sorted(values): - if getattr(value, "when", True) is not True: # when=True means unconditional - condition_spec = spack.spec.Spec("{0}={1}".format(name, value)) - if value.when is False: - # This value is a conflict - # Cannot just prevent listing it as a possible value because it could - # also come in as a possible value from the command line - trigger_id = self.condition( - condition_spec, - name=pkg.name, - msg="invalid variant value {0}={1}".format(name, value), - ) - constraint_id = self.condition( - spack.spec.Spec(), - name=pkg.name, - msg="empty (total) conflict constraint", - ) - msg = "variant {0}={1} is conditionally disabled".format(name, value) - self.gen.fact( - fn.pkg_fact(pkg.name, fn.conflict(trigger_id, constraint_id, msg)) - ) - else: - imposed = spack.spec.Spec(value.when) - imposed.name = pkg.name - - self.condition( - required_spec=condition_spec, - imposed_spec=imposed, - name=pkg.name, - msg="%s variant %s value %s when %s" % (pkg.name, name, value, when), - ) - self.gen.fact(fn.pkg_fact(pkg.name, fn.variant_possible_value(name, value))) + self.gen.newline() - if variant.sticky: - self.gen.fact(fn.pkg_fact(pkg.name, fn.variant_sticky(name))) + def define_auto_variant(self, name: str, multi: bool): + self.gen.h3(f"Special variant: {name}") + vid = next(self._id_counter) + self.gen.fact(fn.auto_variant(name, vid)) + self.gen.fact( + fn.variant_type( + vid, vt.VariantType.MULTI.value if multi else vt.VariantType.SINGLE.value + ) + ) - self.gen.newline() + def variant_rules(self, pkg: "Type[spack.package_base.PackageBase]"): + for name in pkg.variant_names(): + self.gen.h3(f"Variant {name} in package {pkg.name}") + for when, variant_def in pkg.variant_definitions(name): + self.define_variant(pkg, name, when, variant_def) def _get_condition_id( self, @@ -1490,7 +1519,9 @@ class SpackSolverSetup: self, required_spec: spack.spec.Spec, imposed_spec: Optional[spack.spec.Spec] = None, - name: Optional[str] = None, + *, + required_name: Optional[str] = None, + imposed_name: Optional[str] = None, msg: Optional[str] = None, context: Optional[ConditionContext] = None, ): @@ -1499,22 +1530,30 @@ class SpackSolverSetup: Arguments: required_spec: the constraints that triggers this condition imposed_spec: the constraints that are imposed when this condition is triggered - name: name for `required_spec` (required if required_spec is anonymous, ignored if not) + required_name: name for ``required_spec`` + (required if required_spec is anonymous, ignored if not) + imposed_name: name for ``imposed_spec`` + (required if imposed_spec is anonymous, ignored if not) msg: description of the condition context: if provided, indicates how to modify the clause-sets for the required/imposed specs based on the type of constraint they are generated for (e.g. `depends_on`) Returns: int: id of the condition created by this function """ - name = required_spec.name or name - if not name: + required_name = required_spec.name or required_name + if not required_name: raise ValueError(f"Must provide a name for anonymous condition: '{required_spec}'") if not context: context = ConditionContext() context.transform_imposed = remove_node - with spec_with_name(required_spec, name): + if imposed_spec: + imposed_name = imposed_spec.name or imposed_name + if not imposed_name: + raise ValueError(f"Must provide a name for imposed constraint: '{imposed_spec}'") + + with named_spec(required_spec, required_name), named_spec(imposed_spec, imposed_name): # Check if we can emit the requirements before updating the condition ID counter. # In this way, if a condition can't be emitted but the exception is handled in the # caller, we won't emit partial facts. @@ -1562,7 +1601,7 @@ class SpackSolverSetup: continue msg = f"{pkg.name} provides {vpkg} when {when}" - condition_id = self.condition(when, vpkg, pkg.name, msg) + condition_id = self.condition(when, vpkg, required_name=pkg.name, msg=msg) self.gen.fact( fn.pkg_fact(when.name, fn.provider_condition(condition_id, vpkg.name)) ) @@ -1570,7 +1609,7 @@ class SpackSolverSetup: for when, sets_of_virtuals in pkg.provided_together.items(): condition_id = self.condition( - when, name=pkg.name, msg="Virtuals are provided together" + when, required_name=pkg.name, msg="Virtuals are provided together" ) for set_id, virtuals_together in enumerate(sets_of_virtuals): for name in virtuals_together: @@ -1622,7 +1661,7 @@ class SpackSolverSetup: context.transform_required = track_dependencies context.transform_imposed = dependency_holds - self.condition(cond, dep.spec, name=pkg.name, msg=msg, context=context) + self.condition(cond, dep.spec, required_name=pkg.name, msg=msg, context=context) self.gen.newline() @@ -1671,7 +1710,9 @@ class SpackSolverSetup: if rule.condition != spack.spec.Spec(): msg = f"condition to activate requirement {requirement_grp_id}" try: - main_condition_id = self.condition(rule.condition, name=pkg_name, msg=msg) + main_condition_id = self.condition( + rule.condition, required_name=pkg_name, msg=msg + ) except Exception as e: if rule.kind != RequirementKind.DEFAULT: raise RuntimeError( @@ -1712,7 +1753,7 @@ class SpackSolverSetup: member_id = self.condition( required_spec=when_spec, imposed_spec=spec, - name=pkg_name, + required_name=pkg_name, msg=f"{input_spec} is a requirement for package {pkg_name}", context=context, ) @@ -1765,8 +1806,8 @@ class SpackSolverSetup: if pkg_name == "all": continue - # This package does not appear in any repository - if pkg_name not in spack.repo.PATH: + # package isn't a possible dependency and can't be in the solution + if pkg_name not in self.pkgs: continue # This package is not among possible dependencies @@ -1846,23 +1887,19 @@ class SpackSolverSetup: for variant_name in sorted(preferred_variants): variant = preferred_variants[variant_name] - values = variant.value - - if not isinstance(values, tuple): - values = (values,) # perform validation of the variant and values - spec = spack.spec.Spec(pkg_name) try: - spec.update_variant_validate(variant_name, values) - except (spack.variant.InvalidVariantValueError, KeyError, ValueError) as e: + variant_defs = vt.prevalidate_variant_value(self.pkg_class(pkg_name), variant) + except (vt.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)) + for value in variant.value_as_tuple: + for variant_def in variant_defs: + self.variant_values_from_specs.add((pkg_name, id(variant_def), value)) self.gen.fact( fn.variant_default_value_from_packages_yaml(pkg_name, variant.name, value) ) @@ -1968,38 +2005,28 @@ class SpackSolverSetup: # variants for vname, variant in sorted(spec.variants.items()): - values = variant.value - if not isinstance(values, (list, tuple)): - values = [values] + # TODO: variant="*" means 'variant is defined to something', which used to + # be meaningless in concretization, as all variants had to be defined. But + # now that variants can be conditional, it should force a variant to exist. + if variant.value == ("*",): + continue - for value in values: - # * is meaningless for concretization -- just for matching - if value == "*": - continue + for value in variant.value_as_tuple: + # ensure that the value *can* be valid for the spec + if spec.name and not spec.concrete and not spec.virtual: + variant_defs = vt.prevalidate_variant_value( + self.pkg_class(spec.name), variant, spec + ) - # validate variant value only if spec not concrete - if not spec.concrete: - if not spec.virtual and vname not in spack.variant.reserved_names: - pkg_cls = self.pkg_class(spec.name) - try: - variant_def, _ = pkg_cls.variants[vname] - except KeyError: - msg = 'variant "{0}" not found in package "{1}"' - raise RuntimeError(msg.format(vname, spec.name)) - else: - variant_def.validate_or_raise( - variant, spack.repo.PATH.get_pkg_class(spec.name) - ) + # Record that that this is a valid possible value. Accounts for + # int/str/etc., where valid values can't be listed in the package + for variant_def in variant_defs: + self.variant_values_from_specs.add((spec.name, id(variant_def), value)) clauses.append(f.variant_value(spec.name, vname, value)) if variant.propagate: clauses.append(f.propagate(spec.name, fn.variant_value(vname, value))) - # Tell the concretizer that this is a possible value for the - # variant, to account for things like int/str values where we - # can't enumerate the valid values - self.variant_values_from_specs.add((spec.name, vname, value)) - # compiler and compiler version if spec.compiler: clauses.append(f.node_compiler(spec.name, spec.compiler.name)) @@ -2467,15 +2494,15 @@ class SpackSolverSetup: def define_variant_values(self): """Validate variant values from the command line. - Also add valid variant values from the command line to the - possible values for a variant. + Add valid variant values from the command line to the possible values for + variant definitions. """ - # Tell the concretizer about possible values from specs we saw in - # spec_clauses(). We might want to order these facts by pkg and name - # if we are debugging. - for pkg, variant, value in self.variant_values_from_specs: - self.gen.fact(fn.pkg_fact(pkg, fn.variant_possible_value(variant, value))) + # Tell the concretizer about possible values from specs seen in spec_clauses(). + # We might want to order these facts by pkg and name if we are debugging. + for pkg_name, variant_def_id, value in self.variant_values_from_specs: + vid = self.variant_ids_by_def_id[variant_def_id] + self.gen.fact(fn.pkg_fact(pkg_name, fn.variant_possible_value(vid, value))) def register_concrete_spec(self, spec, possible): # tell the solver about any installed packages that could @@ -2644,6 +2671,10 @@ class SpackSolverSetup: self.gen.h2("Package preferences: %s" % pkg) self.preferred_variants(pkg) + self.gen.h1("Special variants") + self.define_auto_variant("dev_path", multi=False) + self.define_auto_variant("patches", multi=True) + self.gen.h1("Develop specs") # Inject dev_path from environment for ds in dev_specs: @@ -2917,6 +2948,9 @@ class ProblemInstanceBuilder: def h2(self, header: str) -> None: self.title(header, "-") + def h3(self, header: str): + self.asp_problem.append(f"% {header}\n") + def newline(self): self.asp_problem.append("\n") @@ -3466,15 +3500,19 @@ class SpecBuilder: """ return NodeArgument(id="0", pkg=pkg) - def __init__(self, specs, hash_lookup=None): - self._specs = {} + def __init__( + self, specs: List[spack.spec.Spec], *, hash_lookup: Optional[ConcreteSpecsByHash] = None + ): + self._specs: Dict[NodeArgument, spack.spec.Spec] = {} self._result = None self._command_line_specs = specs - self._flag_sources = collections.defaultdict(lambda: set()) + self._flag_sources: Dict[Tuple[NodeArgument, str], Set[str]] = collections.defaultdict( + lambda: set() + ) # Pass in as arguments reusable specs and plug them in # from this dictionary during reconstruction - self._hash_lookup = hash_lookup or {} + self._hash_lookup = hash_lookup or ConcreteSpecsByHash() def hash(self, node, h): if node not in self._specs: @@ -3505,21 +3543,17 @@ class SpecBuilder: def node_target(self, node, target): self._arch(node).target = target - def variant_value(self, node, name, value): - # FIXME: is there a way not to special case 'dev_path' everywhere? - if name == "dev_path": - self._specs[node].variants.setdefault( - name, spack.variant.SingleValuedVariant(name, value) - ) - return - - if name == "patches": - self._specs[node].variants.setdefault( - name, spack.variant.MultiValuedVariant(name, value) + def variant_selected(self, node, name, value, variant_type, variant_id): + spec = self._specs[node] + variant = spec.variants.get(name) + if not variant: + spec.variants[name] = vt.VariantType(variant_type).variant_class(name, value) + else: + assert variant_type == vt.VariantType.MULTI.value, ( + f"Can't have multiple values for single-valued variant: " + f"{node}, {name}, {value}, {variant_type}, {variant_id}" ) - return - - self._specs[node].update_variant_validate(name, value) + variant.append(value) def version(self, node, version): self._specs[node].versions = vn.VersionList([vn.Version(version)]) @@ -3680,7 +3714,7 @@ class SpecBuilder: tty.warn(f'using "{node.pkg}@{version}" which is a deprecated version') @staticmethod - def sort_fn(function_tuple): + def sort_fn(function_tuple) -> Tuple[int, int]: """Ensure attributes are evaluated in the correct order. hash attributes are handled first, since they imply entire concrete specs @@ -3799,7 +3833,7 @@ def _develop_specs_from_env(spec, env): assert spec.variants["dev_path"].value == path, error_msg else: - spec.variants.setdefault("dev_path", spack.variant.SingleValuedVariant("dev_path", path)) + spec.variants.setdefault("dev_path", vt.SingleValuedVariant("dev_path", path)) assert spec.satisfies(dev_info["spec"]) diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 44e9e213be..2195cd6b08 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -819,58 +819,132 @@ error(10, Message) :- %----------------------------------------------------------------------------- % Variant semantics %----------------------------------------------------------------------------- -% a variant is a variant of a package if it is a variant under some condition -% and that condition holds -node_has_variant(node(NodeID, Package), Variant) :- - pkg_fact(Package, conditional_variant(ID, Variant)), - condition_holds(ID, node(NodeID, Package)). +% Packages define potentially several definitions for each variant, and depending +% on their attibutes, duplicate nodes for the same package may use different +% definitions. So the variant logic has several jobs: +% A. Associate a variant definition with a node, by VariantID +% B. Associate defaults and attributes (sticky, etc.) for the selected variant ID with the node. +% C. Once these rules are established for a node, select variant value(s) based on them. + +% A: Selecting a variant definition + +% Variant definitions come from package facts in two ways: +% 1. unconditional variants are always defined on all nodes for a given package +variant_definition(node(NodeID, Package), Name, VariantID) :- + pkg_fact(Package, variant_definition(Name, VariantID)), + attr("node", node(NodeID, Package)). + +% 2. conditional variants are only defined if the conditions hold for the node +variant_definition(node(NodeID, Package), Name, VariantID) :- + pkg_fact(Package, variant_condition(Name, VariantID, ConditionID)), + condition_holds(ConditionID, node(NodeID, Package)). + +% If there are any definitions for a variant on a node, the variant is "defined". +variant_defined(PackageNode, Name) :- variant_definition(PackageNode, Name, _). + +% We must select one definition for each defined variant on a node. +1 { + node_has_variant(PackageNode, Name, VariantID) : variant_definition(PackageNode, Name, VariantID) +} 1 :- + variant_defined(PackageNode, Name). + +% Solver must pick the variant definition with the highest id. When conditions hold +% for two or more variant definitions, this prefers the last one defined. +:- node_has_variant(node(NodeID, Package), Name, SelectedVariantID), + variant_definition(node(NodeID, Package), Name, VariantID), + VariantID > SelectedVariantID. + +% B: Associating applicable package rules with nodes -node_has_variant(node(ID, Package), Variant) :- - pkg_fact(Package, variant(Variant)), - attr("node", node(ID, Package)). +% The default value for a variant in a package is what is prescribed: +% 1. On the command line +% 2. In packages.yaml (if there's no command line settings) +% 3. In the package.py file (if there are no settings in packages.yaml and the command line) + +% -- Associate the definition's default values with the node +% note that the package.py variant defaults are associated with a particular definition, but +% packages.yaml and CLI are associated with just the variant name. +% Also, settings specified on the CLI apply to all duplicates, but always have +% `min_dupe_id` as their node id. +variant_default_value(node(NodeID, Package), VariantName, Value) :- + node_has_variant(node(NodeID, Package), VariantName, VariantID), + pkg_fact(Package, variant_default_value_from_package_py(VariantID, Value)), + not variant_default_value_from_packages_yaml(Package, VariantName, _), + not attr("variant_default_value_from_cli", node(min_dupe_id, Package), VariantName, _). + +variant_default_value(node(NodeID, Package), VariantName, Value) :- + node_has_variant(node(NodeID, Package), VariantName, _), + variant_default_value_from_packages_yaml(Package, VariantName, Value), + not attr("variant_default_value_from_cli", node(min_dupe_id, Package), VariantName, _). + +variant_default_value(node(NodeID, Package), VariantName, Value) :- + node_has_variant(node(NodeID, Package), VariantName, _), + attr("variant_default_value_from_cli", node(min_dupe_id, Package), VariantName, Value). + +% -- Associate the definition's possible values with the node +variant_possible_value(node(NodeID, Package), VariantName, Value) :- + node_has_variant(node(NodeID, Package), VariantName, VariantID), + pkg_fact(Package, variant_possible_value(VariantID, Value)). + +variant_value_from_disjoint_sets(node(NodeID, Package), VariantName, Value1, Set1) :- + node_has_variant(node(NodeID, Package), VariantName, VariantID), + pkg_fact(Package, variant_value_from_disjoint_sets(VariantID, Value1, Set1)). + +% -- Associate definition's arity with the node +variant_single_value(node(NodeID, Package), VariantName) :- + node_has_variant(node(NodeID, Package), VariantName, VariantID), + not variant_type(VariantID, "multi"). + +% C: Determining variant values on each node + +% if a variant is sticky, but not set, its value is the default value +attr("variant_selected", node(ID, Package), Variant, Value, VariantType, VariantID) :- + node_has_variant(node(ID, Package), Variant, VariantID), + variant_default_value(node(ID, Package), Variant, Value), + pkg_fact(Package, variant_sticky(VariantID)), + variant_type(VariantID, VariantType), + not attr("variant_set", node(ID, Package), Variant), + build(node(ID, Package)). + +% we can choose variant values from all the possible values for the node +{ + attr("variant_selected", node(ID, Package), Variant, Value, VariantType, VariantID) + : variant_possible_value(node(ID, Package), Variant, Value) +} :- + attr("node", node(ID, Package)), + node_has_variant(node(ID, Package), Variant, VariantID), + variant_type(VariantID, VariantType), + build(node(ID, Package)). + +% variant_selected is only needed for reconstruction on the python side, so we can ignore it here +attr("variant_value", PackageNode, Variant, Value) :- + attr("variant_selected", PackageNode, Variant, Value, VariantType, VariantID). % a variant cannot be set if it is not a variant on the package error(100, "Cannot set variant '{0}' for package '{1}' because the variant condition cannot be satisfied for the given spec", Variant, Package) - :- attr("variant_set", node(X, Package), Variant), - not node_has_variant(node(X, Package), Variant), - build(node(X, Package)). + :- attr("variant_set", node(ID, Package), Variant), + not node_has_variant(node(ID, Package), Variant, _), + build(node(ID, Package)). % a variant cannot take on a value if it is not a variant of the package error(100, "Cannot set variant '{0}' for package '{1}' because the variant condition cannot be satisfied for the given spec", Variant, Package) - :- attr("variant_value", node(X, Package), Variant, _), - not node_has_variant(node(X, Package), Variant), - build(node(X, Package)). - -% if a variant is sticky and not set its value is the default value -attr("variant_value", node(ID, Package), Variant, Value) :- - node_has_variant(node(ID, Package), Variant), - not attr("variant_set", node(ID, Package), Variant), - pkg_fact(Package, variant_sticky(Variant)), - variant_default_value(Package, Variant, Value), - build(node(ID, Package)). + :- attr("variant_value", node(ID, Package), Variant, _), + not node_has_variant(node(ID, Package), Variant, _), + build(node(ID, Package)). % at most one variant value for single-valued variants. -{ - attr("variant_value", node(ID, Package), Variant, Value) - : pkg_fact(Package, variant_possible_value(Variant, Value)) -} - :- attr("node", node(ID, Package)), - node_has_variant(node(ID, Package), Variant), - build(node(ID, Package)). - - error(100, "'{0}' required multiple values for single-valued variant '{1}'", Package, Variant) :- attr("node", node(ID, Package)), - node_has_variant(node(ID, Package), Variant), - pkg_fact(Package, variant_single_value(Variant)), + node_has_variant(node(ID, Package), Variant, _), + variant_single_value(node(ID, Package), Variant), build(node(ID, Package)), 2 { attr("variant_value", node(ID, Package), Variant, Value) }. error(100, "No valid value for variant '{1}' of package '{0}'", Package, Variant) - :- attr("node", node(X, Package)), - node_has_variant(node(X, Package), Variant), - build(node(X, Package)), - not attr("variant_value", node(X, Package), Variant, _). + :- attr("node", node(ID, Package)), + node_has_variant(node(ID, Package), Variant, _), + build(node(ID, Package)), + not attr("variant_value", node(ID, Package), Variant, _). % if a variant is set to anything, it is considered 'set'. attr("variant_set", PackageNode, Variant) :- attr("variant_set", PackageNode, Variant, _). @@ -880,17 +954,16 @@ attr("variant_set", PackageNode, Variant) :- attr("variant_set", PackageNode, Va % have been built w/different variants from older/different package versions. error(10, "'Spec({1}={2})' is not a valid value for '{0}' variant '{1}'", Package, Variant, Value) :- attr("variant_value", node(ID, Package), Variant, Value), - not pkg_fact(Package, variant_possible_value(Variant, Value)), + not variant_possible_value(node(ID, Package), Variant, Value), build(node(ID, Package)). -% Some multi valued variants accept multiple values from disjoint sets. -% Ensure that we respect that constraint and we don't pick values from more -% than one set at once +% Some multi valued variants accept multiple values from disjoint sets. Ensure that we +% respect that constraint and we don't pick values from more than one set at once error(100, "{0} variant '{1}' cannot have values '{2}' and '{3}' as they come from disjoint value sets", Package, Variant, Value1, Value2) :- attr("variant_value", node(ID, Package), Variant, Value1), attr("variant_value", node(ID, Package), Variant, Value2), - pkg_fact(Package, variant_value_from_disjoint_sets(Variant, Value1, Set1)), - pkg_fact(Package, variant_value_from_disjoint_sets(Variant, Value2, Set2)), + variant_value_from_disjoint_sets(node(ID, Package), Variant, Value1, Set1), + variant_value_from_disjoint_sets(node(ID, Package), Variant, Value2, Set2), Set1 < Set2, % see[1] build(node(ID, Package)). @@ -902,7 +975,7 @@ error(100, "{0} variant '{1}' cannot have values '{2}' and '{3}' as they come fr % specified in an external, we score it as if it was a default value. variant_not_default(node(ID, Package), Variant, Value) :- attr("variant_value", node(ID, Package), Variant, Value), - not variant_default_value(Package, Variant, Value), + not variant_default_value(node(ID, Package), Variant, Value), % variants set explicitly on the CLI don't count as non-default not attr("variant_set", node(ID, Package), Variant, Value), % variant values forced by propagation don't count as non-default @@ -913,11 +986,10 @@ variant_not_default(node(ID, Package), Variant, Value) not external_with_variant_set(node(ID, Package), Variant, Value), attr("node", node(ID, Package)). - % A default variant value that is not used variant_default_not_used(node(ID, Package), Variant, Value) - :- variant_default_value(Package, Variant, Value), - node_has_variant(node(ID, Package), Variant), + :- variant_default_value(node(ID, Package), Variant, Value), + node_has_variant(node(ID, Package), Variant, _), not attr("variant_value", node(ID, Package), Variant, Value), not propagate(node(ID, Package), variant_value(Variant, _)), attr("node", node(ID, Package)). @@ -931,25 +1003,6 @@ external_with_variant_set(node(NodeID, Package), Variant, Value) external(node(NodeID, Package)), attr("node", node(NodeID, Package)). -% The default value for a variant in a package is what is prescribed: -% -% 1. On the command line -% 2. In packages.yaml (if there's no command line settings) -% 3. In the package.py file (if there are no settings in -% packages.yaml and the command line) -% -variant_default_value(Package, Variant, Value) - :- pkg_fact(Package, variant_default_value_from_package_py(Variant, Value)), - not variant_default_value_from_packages_yaml(Package, Variant, _), - not attr("variant_default_value_from_cli", node(min_dupe_id, Package), Variant, _). - -variant_default_value(Package, Variant, Value) - :- variant_default_value_from_packages_yaml(Package, Variant, Value), - not attr("variant_default_value_from_cli", node(min_dupe_id, Package), Variant, _). - -variant_default_value(Package, Variant, Value) :- - attr("variant_default_value_from_cli", node(min_dupe_id, Package), Variant, Value). - % Treat 'none' in a special way - it cannot be combined with other % values even if the variant is multi-valued error(100, "{0} variant '{1}' cannot have values '{2}' and 'none'", Package, Variant, Value) @@ -958,23 +1011,26 @@ error(100, "{0} variant '{1}' cannot have values '{2}' and 'none'", Package, Var Value != "none", build(node(X, Package)). -% patches and dev_path are special variants -- they don't have to be -% declared in the package, so we just allow them to spring into existence -% when assigned a value. -auto_variant("dev_path"). -auto_variant("patches"). +% -- Auto variants +% These don't have to be declared in the package. We allow them to spring into +% existence when assigned a value. +variant_possible_value(PackageNode, Variant, Value) + :- attr("variant_set", PackageNode, Variant, Value), auto_variant(Variant, _). -node_has_variant(PackageNode, Variant) - :- attr("variant_set", PackageNode, Variant, _), auto_variant(Variant). +node_has_variant(PackageNode, Variant, VariantID) + :- attr("variant_set", PackageNode, Variant, _), auto_variant(Variant, VariantID). -pkg_fact(Package, variant_single_value("dev_path")) - :- attr("variant_set", node(ID, Package), "dev_path", _). +variant_single_value(PackageNode, Variant) + :- node_has_variant(PackageNode, Variant, VariantID), + auto_variant(Variant, VariantID), + not variant_type(VariantID, "multi"). % suppress warnings about this atom being unset. It's only set if some % spec or some package sets it, and without this, clingo will give % warnings like 'info: atom does not occur in any rule head'. #defined variant_default_value/3. #defined variant_default_value_from_packages_yaml/3. +#defined variant_default_value_from_package_py/3. %----------------------------------------------------------------------------- % Propagation semantics @@ -1004,11 +1060,12 @@ propagate(ChildNode, PropagatedAttribute, edge_types(DepType1, DepType2)) :- %---- % If a variant is propagated, and can be accepted, set its value -attr("variant_value", node(ID, Package), Variant, Value) :- - propagate(node(ID, Package), variant_value(Variant, Value)), - node_has_variant(node(ID, Package), Variant), - pkg_fact(Package, variant_possible_value(Variant, Value)), - not attr("variant_set", node(ID, Package), Variant). +attr("variant_selected", PackageNode, Variant, Value, VariantType, VariantID) :- + propagate(PackageNode, variant_value(Variant, Value)), + node_has_variant(PackageNode, Variant, VariantID), + variant_type(VariantID, VariantType), + variant_possible_value(PackageNode, Variant, Value), + not attr("variant_set", PackageNode, Variant). % If a variant is propagated, we cannot have extraneous values variant_is_propagated(PackageNode, Variant) :- @@ -1017,7 +1074,7 @@ variant_is_propagated(PackageNode, Variant) :- not attr("variant_set", PackageNode, Variant). :- variant_is_propagated(PackageNode, Variant), - attr("variant_value", PackageNode, Variant, Value), + attr("variant_selected", PackageNode, Variant, Value, _, _), not propagate(PackageNode, variant_value(Variant, Value)). %---- diff --git a/lib/spack/spack/solver/display.lp b/lib/spack/spack/solver/display.lp index fb3b2c41df..675a9d17d2 100644 --- a/lib/spack/spack/solver/display.lp +++ b/lib/spack/spack/solver/display.lp @@ -14,6 +14,7 @@ #show attr/3. #show attr/4. #show attr/5. +#show attr/6. % names of optimization criteria #show opt_criterion/2. @@ -39,7 +40,7 @@ #show condition_requirement/4. #show condition_requirement/5. #show condition_requirement/6. -#show node_has_variant/2. +#show node_has_variant/3. #show build/1. #show external/1. #show external_version/3. @@ -49,5 +50,6 @@ #show condition_nodes/3. #show trigger_node/3. #show imposed_nodes/3. +#show variant_single_value/2. % debug diff --git a/lib/spack/spack/solver/error_messages.lp b/lib/spack/spack/solver/error_messages.lp index 79a9b4b7eb..7bc9ed2e93 100644 --- a/lib/spack/spack/solver/error_messages.lp +++ b/lib/spack/spack/solver/error_messages.lp @@ -5,6 +5,10 @@ %============================================================================= % This logic program adds detailed error messages to Spack's concretizer +% +% Note that functions used in rule bodies here need to have a corresponding +% #show line in display.lp, otherwise they won't be passed through to the +% error solve. %============================================================================= #program error_messages. @@ -113,12 +117,11 @@ error(0, "Cannot find a valid provider for virtual {0}", Virtual, startcauses, C pkg_fact(TriggerPkg, condition_effect(Cause, EID)), condition_holds(Cause, node(CID, TriggerPkg)). - % At most one variant value for single-valued variants error(0, "'{0}' required multiple values for single-valued variant '{1}'\n Requested 'Spec({1}={2})' and 'Spec({1}={3})'", Package, Variant, Value1, Value2, startcauses, Cause1, X, Cause2, X) :- attr("node", node(X, Package)), - node_has_variant(node(X, Package), Variant), - pkg_fact(Package, variant_single_value(Variant)), + node_has_variant(node(X, Package), Variant, VariantID), + variant_single_value(node(X, Package), Variant), build(node(X, Package)), attr("variant_value", node(X, Package), Variant, Value1), imposed_constraint(EID1, "variant_set", Package, Variant, Value1), @@ -216,6 +219,11 @@ error(0, Msg, startcauses, TriggerID, ID1, ConstraintID, ID2) #defined error/4. #defined error/5. #defined error/6. +#defined error/7. +#defined error/8. +#defined error/9. +#defined error/10. +#defined error/11. #defined attr/2. #defined attr/3. #defined attr/4. @@ -225,6 +233,7 @@ error(0, Msg, startcauses, TriggerID, ID1, ConstraintID, ID2) #defined imposed_constraint/4. #defined imposed_constraint/5. #defined imposed_constraint/6. +#defined condition_cause/4. #defined condition_requirement/3. #defined condition_requirement/4. #defined condition_requirement/5. @@ -234,6 +243,7 @@ error(0, Msg, startcauses, TriggerID, ID1, ConstraintID, ID2) #defined external/1. #defined trigger_and_effect/3. #defined build/1. -#defined node_has_variant/2. +#defined node_has_variant/3. #defined provider/2. #defined external_version/3. +#defined variant_single_value/2. diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 492841f81a..28e11d68b8 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2367,14 +2367,16 @@ class Spec: package_cls = spack.repo.PATH.get_pkg_class(new_spec.name) if change_spec.versions and not change_spec.versions == vn.any_version: new_spec.versions = change_spec.versions - for variant, value in change_spec.variants.items(): - if variant in package_cls.variants: - if variant in new_spec.variants: + + for vname, value in change_spec.variants.items(): + if vname in package_cls.variant_names(): + if vname in new_spec.variants: new_spec.variants.substitute(value) else: - new_spec.variants[variant] = value + new_spec.variants[vname] = value else: - raise ValueError("{0} is not a variant of {1}".format(variant, new_spec.name)) + raise ValueError("{0} is not a variant of {1}".format(vname, new_spec.name)) + if change_spec.compiler: new_spec.compiler = change_spec.compiler if change_spec.compiler_flags: @@ -2962,48 +2964,14 @@ class Spec: return pkg_cls = spec.package_class - pkg_variants = pkg_cls.variants + pkg_variants = pkg_cls.variant_names() # reserved names are variants that may be set on any package # but are not necessarily recorded by the package's class not_existing = set(spec.variants) - (set(pkg_variants) | set(vt.reserved_names)) if not_existing: - raise vt.UnknownVariantError(spec, not_existing) - - def update_variant_validate(self, variant_name, values): - """If it is not already there, adds the variant named - `variant_name` to the spec `spec` based on the definition - contained in the package metadata. Validates the variant and - values before returning. - - Used to add values to a variant without being sensitive to the - variant being single or multi-valued. If the variant already - exists on the spec it is assumed to be multi-valued and the - values are appended. - - Args: - variant_name: the name of the variant to add or append to - values: the value or values (as a tuple) to add/append - to the variant - """ - if not isinstance(values, tuple): - values = (values,) - - pkg_variant, _ = self.package_class.variants[variant_name] - - for value in values: - if self.variants.get(variant_name): - msg = ( - f"cannot append the new value '{value}' to the single-valued " - f"variant '{self.variants[variant_name]}'" - ) - assert pkg_variant.multi, msg - self.variants[variant_name].append(value) - else: - variant = pkg_variant.make_variant(value) - self.variants[variant_name] = variant - - pkg_cls = spack.repo.PATH.get_pkg_class(self.name) - pkg_variant.validate_or_raise(self.variants[variant_name], pkg_cls) + raise vt.UnknownVariantError( + f"No such variant {not_existing} for spec: '{spec}'", list(not_existing) + ) def constrain(self, other, deps=True): """Intersect self with other in-place. Return True if self changed, False otherwise. @@ -4447,7 +4415,9 @@ class VariantMap(lang.HashableMap): Returns: bool: True or False """ - return self.spec._concrete or all(v in self for v in self.spec.package_class.variants) + return self.spec._concrete or all( + v in self for v in self.spec.package_class.variant_names() + ) def copy(self) -> "VariantMap": clone = VariantMap(self.spec) @@ -4485,7 +4455,7 @@ class VariantMap(lang.HashableMap): def substitute_abstract_variants(spec: Spec): """Uses the information in `spec.package` to turn any variant that needs - it into a SingleValuedVariant. + it into a SingleValuedVariant or BoolValuedVariant. This method is best effort. All variants that can be substituted will be substituted before any error is raised. @@ -4493,26 +4463,45 @@ def substitute_abstract_variants(spec: Spec): Args: spec: spec on which to operate the substitution """ - # This method needs to be best effort so that it works in matrix exlusion + # This method needs to be best effort so that it works in matrix exclusion # in $spack/lib/spack/spack/spec_list.py - failed = [] + unknown = [] 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) + + variant_defs = spec.package_class.variant_definitions(name) + valid_defs = [] + for when, vdef in variant_defs: + if when.intersects(spec): + valid_defs.append(vdef) + + if not valid_defs: + if name not in spec.package_class.variant_names(): + unknown.append(name) + else: + whens = [str(when) for when, _ in variant_defs] + raise InvalidVariantForSpecError(v.name, f"({', '.join(whens)})", spec) + continue + + pkg_variant, *rest = valid_defs + if rest: 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) + pkg_variant.validate_or_raise(new_variant, spec.name) spec.variants.substitute(new_variant) - # Raise all errors at once - if failed: - raise vt.UnknownVariantError(spec, failed) + if unknown: + variants = llnl.string.plural(len(unknown), "variant") + raise vt.UnknownVariantError( + f"Tried to set {variants} {llnl.string.comma_and(unknown)}. " + f"{spec.name} has no such {variants}", + unknown_variants=unknown, + ) def parse_with_version_concrete(spec_like: Union[str, Spec], compiler: bool = False): @@ -4942,6 +4931,15 @@ class SpecParseError(spack.error.SpecError): ) +class InvalidVariantForSpecError(spack.error.SpecError): + """Raised when an invalid conditional variant is specified.""" + + def __init__(self, variant, when, spec): + msg = f"Invalid variant {variant} for spec {spec}.\n" + msg += f"{variant} is only available for {spec.name} when satisfying one of {when}." + super().__init__(msg) + + class UnsupportedPropagationError(spack.error.SpecError): """Raised when propagation (==) is used with reserved variant names.""" diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 7aaa2bc7f5..ef004272a7 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -53,7 +53,7 @@ def check_spec(abstract, concrete): cflag = concrete.compiler_flags[flag] assert set(aflag) <= set(cflag) - for name in spack.repo.PATH.get_pkg_class(abstract.name).variants: + for name in spack.repo.PATH.get_pkg_class(abstract.name).variant_names(): assert name in concrete.variants for flag in concrete.compiler_flags.valid_compiler_flags(): @@ -931,7 +931,9 @@ class TestConcretize: ], ) def test_conditional_variants_fail(self, bad_spec): - with pytest.raises((spack.error.UnsatisfiableSpecError, vt.InvalidVariantForSpecError)): + with pytest.raises( + (spack.error.UnsatisfiableSpecError, spack.spec.InvalidVariantForSpecError) + ): _ = Spec("conditional-variant-pkg" + bad_spec).concretized() @pytest.mark.parametrize( @@ -1374,7 +1376,7 @@ class TestConcretize: ) def test_error_message_for_inconsistent_variants(self, spec_str): s = Spec(spec_str) - with pytest.raises(RuntimeError, match="not found in package"): + with pytest.raises(vt.UnknownVariantError): s.concretize() @pytest.mark.regression("22533") diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index a7f4383bc6..d6e7e168af 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -234,6 +234,16 @@ class TestSpecSemantics: 'libelf cflags="-O3" cppflags="-Wall"', 'libelf cflags="-O3" cppflags="-Wall"', ), + ( + "libelf patches=ba5e334fe247335f3a116decfb5284100791dc302b5571ff5e664d8f9a6806c2", + "libelf patches=ba5e3", # constrain by a patch sha256 prefix + # TODO: the result below is not ideal. Prefix satisfies() works for patches, but + # constrain() isn't similarly special-cased to do the same thing + ( + "libelf patches=ba5e3," + "ba5e334fe247335f3a116decfb5284100791dc302b5571ff5e664d8f9a6806c2" + ), + ), ], ) def test_abstract_specs_can_constrain_each_other(self, lhs, rhs, expected): @@ -1076,7 +1086,7 @@ class TestSpecSemantics: @pytest.mark.regression("13124") def test_error_message_unknown_variant(self): s = Spec("mpileaks +unknown") - with pytest.raises(UnknownVariantError, match=r"package has no such"): + with pytest.raises(UnknownVariantError): s.concretize() @pytest.mark.regression("18527") @@ -1115,6 +1125,20 @@ class TestSpecSemantics: assert new_spec.compiler_flags["cflags"] == ["-O2"] assert new_spec.compiler_flags["cxxflags"] == ["-O1"] + def test_spec_override_with_nonexisting_variant(self): + init_spec = Spec("pkg-a foo=baz foobar=baz cflags=-O3 cxxflags=-O1") + change_spec = Spec("pkg-a baz=fee") + with pytest.raises(ValueError): + Spec.override(init_spec, change_spec) + + def test_spec_override_with_variant_not_in_init_spec(self): + init_spec = Spec("pkg-a foo=baz foobar=baz cflags=-O3 cxxflags=-O1") + change_spec = Spec("pkg-a +bvv ~lorem_ipsum") + new_spec = Spec.override(init_spec, change_spec) + new_spec.concretize() + assert "+bvv" in new_spec + assert "~lorem_ipsum" in new_spec + @pytest.mark.parametrize( "spec_str,specs_in_dag", [ diff --git a/lib/spack/spack/test/variant.py b/lib/spack/spack/test/variant.py index d44aacee5e..c4c439b86f 100644 --- a/lib/spack/spack/test/variant.py +++ b/lib/spack/spack/test/variant.py @@ -7,9 +7,12 @@ import numbers import pytest import spack.error +import spack.repo +import spack.spec import spack.variant -from spack.spec import VariantMap +from spack.spec import Spec, VariantMap from spack.variant import ( + AbstractVariant, BoolValuedVariant, DuplicateVariantError, InconsistentValidationError, @@ -541,7 +544,7 @@ class TestVariant: ) # Valid vspec, shouldn't raise vspec = a.make_variant("bar") - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") # Multiple values are not allowed with pytest.raises(MultipleValuesInExclusiveVariantError): @@ -550,16 +553,16 @@ class TestVariant: # Inconsistent vspec vspec.name = "FOO" with pytest.raises(InconsistentValidationError): - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") # Valid multi-value vspec a.multi = True vspec = a.make_variant("bar,baz") - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") # Add an invalid value vspec.value = "bar,baz,barbaz" with pytest.raises(InvalidVariantValueError): - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") def test_callable_validator(self): def validator(x): @@ -570,12 +573,12 @@ class TestVariant: a = Variant("foo", default=1024, description="", values=validator, multi=False) vspec = a.make_default() - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") vspec.value = 2056 - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") vspec.value = "foo" with pytest.raises(InvalidVariantValueError): - a.validate_or_raise(vspec) + a.validate_or_raise(vspec, "test-package") def test_representation(self): a = Variant( @@ -583,11 +586,22 @@ class TestVariant: ) assert a.allowed_values == "bar, baz, foobar" + def test_str(self): + string = str( + Variant( + "foo", default="", description="", values=("bar", "baz", "foobar"), multi=False + ) + ) + assert "'foo'" in string + assert "default=''" in string + assert "description=''" in string + assert "values=('foo', 'bar', 'baz') in string" + class TestVariantMapTest: - def test_invalid_values(self): + def test_invalid_values(self) -> None: # Value with invalid type - a = VariantMap(None) + a = VariantMap(Spec()) with pytest.raises(TypeError): a["foo"] = 2 @@ -606,17 +620,17 @@ class TestVariantMapTest: with pytest.raises(KeyError): a["bar"] = MultiValuedVariant("foo", "bar") - def test_set_item(self): + def test_set_item(self) -> None: # Check that all the three types of variants are accepted - a = VariantMap(None) + a = VariantMap(Spec()) a["foo"] = BoolValuedVariant("foo", True) a["bar"] = SingleValuedVariant("bar", "baz") a["foobar"] = MultiValuedVariant("foobar", "a, b, c, d, e") - def test_substitute(self): + def test_substitute(self) -> None: # Check substitution of a key that exists - a = VariantMap(None) + a = VariantMap(Spec()) a["foo"] = BoolValuedVariant("foo", True) a.substitute(SingleValuedVariant("foo", "bar")) @@ -625,15 +639,15 @@ class TestVariantMapTest: with pytest.raises(KeyError): a.substitute(BoolValuedVariant("bar", True)) - def test_satisfies_and_constrain(self): + def test_satisfies_and_constrain(self) -> None: # foo=bar foobar=fee feebar=foo - a = VariantMap(None) + a = VariantMap(Spec()) a["foo"] = MultiValuedVariant("foo", "bar") a["foobar"] = SingleValuedVariant("foobar", "fee") a["feebar"] = SingleValuedVariant("feebar", "foo") # foo=bar,baz foobar=fee shared=True - b = VariantMap(None) + b = VariantMap(Spec()) b["foo"] = MultiValuedVariant("foo", "bar, baz") b["foobar"] = SingleValuedVariant("foobar", "fee") b["shared"] = BoolValuedVariant("shared", True) @@ -645,7 +659,7 @@ class TestVariantMapTest: assert not b.satisfies(a) # foo=bar,baz foobar=fee feebar=foo shared=True - c = VariantMap(None) + c = VariantMap(Spec()) c["foo"] = MultiValuedVariant("foo", "bar, baz") c["foobar"] = SingleValuedVariant("foobar", "fee") c["feebar"] = SingleValuedVariant("feebar", "foo") @@ -654,8 +668,8 @@ class TestVariantMapTest: assert a.constrain(b) assert a == c - def test_copy(self): - a = VariantMap(None) + def test_copy(self) -> None: + a = VariantMap(Spec()) a["foo"] = BoolValuedVariant("foo", True) a["bar"] = SingleValuedVariant("bar", "baz") a["foobar"] = MultiValuedVariant("foobar", "a, b, c, d, e") @@ -663,14 +677,31 @@ class TestVariantMapTest: c = a.copy() assert a == c - def test_str(self): - c = VariantMap(None) + def test_str(self) -> None: + c = VariantMap(Spec()) c["foo"] = MultiValuedVariant("foo", "bar, baz") c["foobar"] = SingleValuedVariant("foobar", "fee") c["feebar"] = SingleValuedVariant("feebar", "foo") c["shared"] = BoolValuedVariant("shared", True) assert str(c) == "+shared feebar=foo foo=bar,baz foobar=fee" + def test_concrete(self, mock_packages, config) -> None: + spec = Spec("pkg-a") + vm = VariantMap(spec) + assert not vm.concrete + + # concrete if associated spec is concrete + spec.concretize() + assert vm.concrete + + # concrete if all variants are present (even if spec not concrete) + spec._mark_concrete(False) + assert spec.variants.concrete + + # remove a variant to test the condition + del spec.variants["foo"] + assert not spec.variants.concrete + def test_disjoint_set_initialization_errors(): # Constructing from non-disjoint sets should raise an exception @@ -765,9 +796,154 @@ def test_wild_card_valued_variants_equivalent_to_str(): several_arbitrary_values = ("doe", "re", "mi") # "*" case wild_output = wild_var.make_variant(several_arbitrary_values) - wild_var.validate_or_raise(wild_output) + wild_var.validate_or_raise(wild_output, "test-package") # str case str_output = str_var.make_variant(several_arbitrary_values) - str_var.validate_or_raise(str_output) + str_var.validate_or_raise(str_output, "test-package") # equivalence each instance already validated assert str_output.value == wild_output.value + + +def test_variant_definitions(mock_packages): + pkg = spack.repo.PATH.get_pkg_class("variant-values") + + # two variant names + assert len(pkg.variant_names()) == 2 + assert "build_system" in pkg.variant_names() + assert "v" in pkg.variant_names() + + # this name doesn't exist + assert len(pkg.variant_definitions("no-such-variant")) == 0 + + # there are 4 definitions but one is completely shadowed by another + assert len(pkg.variants) == 4 + + # variant_items ignores the shadowed definition + assert len(list(pkg.variant_items())) == 3 + + # variant_definitions also ignores the shadowed definition + defs = [vdef for _, vdef in pkg.variant_definitions("v")] + assert len(defs) == 2 + assert defs[0].default == "foo" + assert defs[0].values == ("foo",) + + assert defs[1].default == "bar" + assert defs[1].values == ("foo", "bar") + + +@pytest.mark.parametrize( + "pkg_name,value,spec,def_ids", + [ + ("variant-values", "foo", "", [0, 1]), + ("variant-values", "bar", "", [1]), + ("variant-values", "foo", "@1.0", [0]), + ("variant-values", "foo", "@2.0", [1]), + ("variant-values", "foo", "@3.0", [1]), + ("variant-values", "foo", "@4.0", []), + ("variant-values", "bar", "@2.0", [1]), + ("variant-values", "bar", "@3.0", [1]), + ("variant-values", "bar", "@4.0", []), + # now with a global override + ("variant-values-override", "bar", "", [0]), + ("variant-values-override", "bar", "@1.0", [0]), + ("variant-values-override", "bar", "@2.0", [0]), + ("variant-values-override", "bar", "@3.0", [0]), + ("variant-values-override", "bar", "@4.0", [0]), + ("variant-values-override", "baz", "", [0]), + ("variant-values-override", "baz", "@2.0", [0]), + ("variant-values-override", "baz", "@3.0", [0]), + ("variant-values-override", "baz", "@4.0", [0]), + ], +) +def test_prevalidate_variant_value(mock_packages, pkg_name, value, spec, def_ids): + pkg = spack.repo.PATH.get_pkg_class(pkg_name) + + all_defs = [vdef for _, vdef in pkg.variant_definitions("v")] + + valid_defs = spack.variant.prevalidate_variant_value( + pkg, SingleValuedVariant("v", value), spack.spec.Spec(spec) + ) + assert len(valid_defs) == len(def_ids) + + for vdef, i in zip(valid_defs, def_ids): + assert vdef is all_defs[i] + + +@pytest.mark.parametrize( + "pkg_name,value,spec", + [ + ("variant-values", "baz", ""), + ("variant-values", "bar", "@1.0"), + ("variant-values", "bar", "@4.0"), + ("variant-values", "baz", "@3.0"), + ("variant-values", "baz", "@4.0"), + # and with override + ("variant-values-override", "foo", ""), + ("variant-values-override", "foo", "@1.0"), + ("variant-values-override", "foo", "@2.0"), + ("variant-values-override", "foo", "@3.0"), + ("variant-values-override", "foo", "@4.0"), + ], +) +def test_strict_invalid_variant_values(mock_packages, pkg_name, value, spec): + pkg = spack.repo.PATH.get_pkg_class(pkg_name) + + with pytest.raises(spack.variant.InvalidVariantValueError): + spack.variant.prevalidate_variant_value( + pkg, SingleValuedVariant("v", value), spack.spec.Spec(spec), strict=True + ) + + +@pytest.mark.parametrize( + "pkg_name,spec,satisfies,def_id", + [ + ("variant-values", "@1.0", "v=foo", 0), + ("variant-values", "@2.0", "v=bar", 1), + ("variant-values", "@3.0", "v=bar", 1), + ("variant-values-override", "@1.0", "v=baz", 0), + ("variant-values-override", "@2.0", "v=baz", 0), + ("variant-values-override", "@3.0", "v=baz", 0), + ], +) +def test_concretize_variant_default_with_multiple_defs( + mock_packages, config, pkg_name, spec, satisfies, def_id +): + pkg = spack.repo.PATH.get_pkg_class(pkg_name) + pkg_defs = [vdef for _, vdef in pkg.variant_definitions("v")] + + spec = spack.spec.Spec(f"{pkg_name}{spec}").concretized() + assert spec.satisfies(satisfies) + assert spec.package.get_variant("v") is pkg_defs[def_id] + + +@pytest.mark.parametrize( + "spec,variant_name,after", + [ + # dev_path is a special case + ("foo dev_path=/path/to/source", "dev_path", SingleValuedVariant), + # reserved name: won't be touched + ("foo patches=2349dc44", "patches", AbstractVariant), + # simple case -- one definition applies + ("variant-values@1.0 v=foo", "v", SingleValuedVariant), + # simple, but with bool valued variant + ("pkg-a bvv=true", "bvv", BoolValuedVariant), + # variant doesn't exist at version + ("variant-values@4.0 v=bar", "v", spack.spec.InvalidVariantForSpecError), + # multiple definitions, so not yet knowable + ("variant-values@2.0 v=bar", "v", AbstractVariant), + ], +) +def test_substitute_abstract_variants(mock_packages, spec, variant_name, after): + spec = Spec(spec) + + # all variants start out as AbstractVariant + assert isinstance(spec.variants[variant_name], AbstractVariant) + + if issubclass(after, Exception): + # if we're checking for an error, use pytest.raises + with pytest.raises(after): + spack.spec.substitute_abstract_variants(spec) + else: + # ensure that the type of the variant on the spec has been narrowed (or not) + spack.spec.substitute_abstract_variants(spec) + assert isinstance(spec.variants[variant_name], after) diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 83f3ecca83..e0ef7c0e30 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -7,17 +7,20 @@ variants both in packages and in specs. """ import collections.abc +import enum import functools import inspect import itertools import re +from typing import Any, Callable, Collection, Iterable, List, Optional, Tuple, Type, Union import llnl.util.lang as lang import llnl.util.tty.color -from llnl.string import comma_or import spack.error as error import spack.parser +import spack.repo +import spack.spec #: These are variant names used by Spack internally; packages can't use them reserved_names = [ @@ -35,36 +38,68 @@ reserved_names = [ special_variant_values = [None, "none", "*"] +class VariantType(enum.Enum): + """Enum representing the three concrete variant types.""" + + MULTI = "multi" + BOOL = "bool" + SINGLE = "single" + + @property + def variant_class(self) -> Type: + if self is self.MULTI: + return MultiValuedVariant + elif self is self.BOOL: + return BoolValuedVariant + else: + return SingleValuedVariant + + class Variant: - """Represents a variant in a package, as declared in the - variant directive. + """Represents a variant definition, created by the ``variant()`` directive. + + There can be multiple definitions of the same variant, and they are given precedence + by order of appearance in the package. Later definitions have higher precedence. + Similarly, definitions in derived classes have higher precedence than those in their + superclasses. + """ + name: str + default: Any + description: str + values: Optional[Collection] #: if None, valid values are defined only by validators + multi: bool + single_value_validator: Callable + group_validator: Optional[Callable] + sticky: bool + precedence: int + def __init__( self, - name, - default, - description, - values=(True, False), - multi=False, - validator=None, - sticky=False, + name: str, + *, + default: Any, + description: str, + values: Union[Collection, Callable] = (True, False), + multi: bool = False, + validator: Optional[Callable] = None, + sticky: bool = False, + precedence: int = 0, ): """Initialize a package variant. Args: - name (str): name of the variant - default (str): default value for the variant in case - nothing has been specified - description (str): purpose of the variant - values (sequence): sequence of allowed values or a callable - accepting a single value as argument and returning True if the - value is good, False otherwise - multi (bool): whether multiple CSV are allowed - validator (callable): optional callable used to enforce - additional logic on the set of values being validated - sticky (bool): if true the variant is set to the default value at - concretization time + name: name of the variant + default: default value for the variant, used when nothing is explicitly specified + description: purpose of the variant + values: sequence of allowed values or a callable accepting a single value as argument + and returning True if the value is good, False otherwise + multi: whether multiple values are allowed + validator: optional callable that can be used to perform additional validation + sticky: if true the variant is set to the default value at concretization time + precedence: int indicating precedence of this variant definition in the solve + (definition with highest precedence is used when multiple definitions are possible) """ self.name = name self.default = default @@ -73,7 +108,7 @@ class Variant: self.values = None if values == "*": # wildcard is a special case to make it easy to say any value is ok - self.single_value_validator = lambda x: True + self.single_value_validator = lambda v: True elif isinstance(values, type): # supplying a type means any value *of that type* @@ -92,21 +127,22 @@ class Variant: self.single_value_validator = values else: # Otherwise, assume values is the set of allowed explicit values - self.values = _flatten(values) - self.single_value_validator = lambda x: x in tuple(self.values) + values = _flatten(values) + self.values = values + self.single_value_validator = lambda v: v in values self.multi = multi self.group_validator = validator self.sticky = sticky + self.precedence = precedence - def validate_or_raise(self, vspec, pkg_cls=None): + def validate_or_raise(self, vspec: "AbstractVariant", pkg_name: str): """Validate a variant spec against this package variant. Raises an exception if any error is found. Args: - vspec (Variant): instance to be validated - pkg_cls (spack.package_base.PackageBase): the package class - that required the validation, if available + vspec: variant spec to be validated + pkg_name: the name of the package class that required this validation (for errors) Raises: InconsistentValidationError: if ``vspec.name != self.name`` @@ -121,25 +157,23 @@ class Variant: if self.name != vspec.name: raise InconsistentValidationError(vspec, self) - # Check the values of the variant spec - value = vspec.value - if isinstance(vspec.value, (bool, str)): - value = (vspec.value,) - # If the value is exclusive there must be at most one + value = vspec.value_as_tuple if not self.multi and len(value) != 1: - raise MultipleValuesInExclusiveVariantError(vspec, pkg_cls) + raise MultipleValuesInExclusiveVariantError(vspec, pkg_name) # Check and record the values that are not allowed - not_allowed_values = [ - x for x in value if x != "*" and self.single_value_validator(x) is False - ] - if not_allowed_values: - raise InvalidVariantValueError(self, not_allowed_values, pkg_cls) + invalid_vals = ", ".join( + f"'{v}'" for v in value if v != "*" and self.single_value_validator(v) is False + ) + if invalid_vals: + raise InvalidVariantValueError( + f"invalid values for variant '{self.name}' in package {pkg_name}: {invalid_vals}\n" + ) # Validate the group of values if needed if self.group_validator is not None and value != ("*",): - self.group_validator(pkg_cls.name, self.name, value) + self.group_validator(pkg_name, self.name, value) @property def allowed_values(self): @@ -168,7 +202,7 @@ class Variant: """ return self.make_variant(self.default) - def make_variant(self, value): + def make_variant(self, value) -> "AbstractVariant": """Factory that creates a variant holding the value passed as a parameter. @@ -179,30 +213,31 @@ class Variant: MultiValuedVariant or SingleValuedVariant or BoolValuedVariant: instance of the proper variant """ - return self.variant_cls(self.name, value) + return self.variant_type.variant_class(self.name, value) @property - def variant_cls(self): - """Proper variant class to be used for this configuration.""" + def variant_type(self) -> VariantType: + """String representation of the type of this variant (single/multi/bool)""" if self.multi: - return MultiValuedVariant + return VariantType.MULTI elif self.values == (True, False): - return BoolValuedVariant - return SingleValuedVariant + return VariantType.BOOL + else: + return VariantType.SINGLE - def __eq__(self, other): + def __str__(self): return ( - self.name == other.name - and self.default == other.default - and self.values == other.values - and self.multi == other.multi - and self.single_value_validator == other.single_value_validator - and self.group_validator == other.group_validator + f"Variant('{self.name}', " + f"default='{self.default}', " + f"description='{self.description}', " + f"values={self.values}, " + f"multi={self.multi}, " + f"single_value_validator={self.single_value_validator}, " + f"group_validator={self.group_validator}, " + f"sticky={self.sticky}, " + f"precedence={self.precedence})" ) - def __ne__(self, other): - return not self == other - def implicit_variant_conversion(method): """Converts other to type(self) and calls method(self, other) @@ -225,12 +260,12 @@ def implicit_variant_conversion(method): return convert -def _flatten(values): +def _flatten(values) -> Collection: """Flatten instances of _ConditionalVariantValues for internal representation""" if isinstance(values, DisjointSetsOfValues): return values - flattened = [] + flattened: List = [] for item in values: if isinstance(item, _ConditionalVariantValues): flattened.extend(item) @@ -241,6 +276,13 @@ def _flatten(values): return tuple(flattened) +#: Type for value of a variant +ValueType = Union[str, bool, Tuple[Union[str, bool], ...]] + +#: Type of variant value when output for JSON, YAML, etc. +SerializedValueType = Union[str, bool, List[Union[str, bool]]] + + @lang.lazy_lexicographic_ordering class AbstractVariant: """A variant that has not yet decided who it wants to be. It behaves like @@ -253,20 +295,20 @@ class AbstractVariant: values. """ - def __init__(self, name, value, propagate=False): + name: str + propagate: bool + _value: ValueType + _original_value: Any + + def __init__(self, name: str, value: Any, propagate: bool = False): self.name = name self.propagate = propagate - # Stores 'value' after a bit of massaging - # done by the property setter - self._value = None - self._original_value = None - # Invokes property setter self.value = value @staticmethod - def from_node_dict(name, value): + def from_node_dict(name: str, value: Union[str, List[str]]) -> "AbstractVariant": """Reconstruct a variant from a node dict.""" if isinstance(value, list): # read multi-value variants in and be faithful to the YAML @@ -280,16 +322,26 @@ class AbstractVariant: return SingleValuedVariant(name, value) - def yaml_entry(self): + def yaml_entry(self) -> Tuple[str, SerializedValueType]: """Returns a key, value tuple suitable to be an entry in a yaml dict. Returns: tuple: (name, value_representation) """ - return self.name, list(self.value) + return self.name, list(self.value_as_tuple) + + @property + def value_as_tuple(self) -> Tuple[Union[bool, str], ...]: + """Getter for self.value that always returns a Tuple (even for single valued variants). + + This makes it easy to iterate over possible values. + """ + if isinstance(self._value, (bool, str)): + return (self._value,) + return self._value @property - def value(self): + def value(self) -> ValueType: """Returns a tuple of strings containing the values stored in the variant. @@ -299,10 +351,10 @@ class AbstractVariant: return self._value @value.setter - def value(self, value): + def value(self, value: ValueType) -> None: self._value_setter(value) - def _value_setter(self, value): + def _value_setter(self, value: ValueType) -> None: # Store the original value self._original_value = value @@ -310,7 +362,7 @@ class AbstractVariant: # Store a tuple of CSV string representations # Tuple is necessary here instead of list because the # values need to be hashed - value = re.split(r"\s*,\s*", str(value)) + value = tuple(re.split(r"\s*,\s*", str(value))) for val in special_variant_values: if val in value and len(value) > 1: @@ -323,16 +375,11 @@ class AbstractVariant: # to a set self._value = tuple(sorted(set(value))) - def _cmp_iter(self): + def _cmp_iter(self) -> Iterable: yield self.name + yield from (str(v) for v in self.value_as_tuple) - value = self._value - if not isinstance(value, tuple): - value = (value,) - value = tuple(str(x) for x in value) - yield value - - def copy(self): + def copy(self) -> "AbstractVariant": """Returns an instance of a variant equivalent to self Returns: @@ -346,7 +393,7 @@ class AbstractVariant: return type(self)(self.name, self._original_value, self.propagate) @implicit_variant_conversion - def satisfies(self, other): + def satisfies(self, other: "AbstractVariant") -> bool: """Returns true if ``other.name == self.name``, because any value that other holds and is not in self yet **could** be added. @@ -360,13 +407,13 @@ class AbstractVariant: # (`foo=bar` will never satisfy `baz=bar`) return other.name == self.name - def intersects(self, other): + def intersects(self, other: "AbstractVariant") -> bool: """Returns True if there are variant matching both self and other, False otherwise.""" if isinstance(other, (SingleValuedVariant, BoolValuedVariant)): return other.intersects(self) return other.name == self.name - def compatible(self, other): + def compatible(self, other: "AbstractVariant") -> bool: """Returns True if self and other are compatible, False otherwise. As there is no semantic check, two VariantSpec are compatible if @@ -383,7 +430,7 @@ class AbstractVariant: return self.intersects(other) @implicit_variant_conversion - def constrain(self, other): + def constrain(self, other: "AbstractVariant") -> bool: """Modify self to match all the constraints for other if both instances are multi-valued. Returns True if self changed, False otherwise. @@ -399,23 +446,23 @@ class AbstractVariant: old_value = self.value - values = list(sorted(set(self.value + other.value))) + values = list(sorted(set(self.value_as_tuple + other.value_as_tuple))) # If we constraint wildcard by another value, just take value if "*" in values and len(values) > 1: values.remove("*") - self.value = ",".join(values) + self._value_setter(",".join(str(v) for v in values)) return old_value != self.value - def __contains__(self, item): - return item in self._value + def __contains__(self, item: Union[str, bool]) -> bool: + return item in self.value_as_tuple - def __repr__(self): + def __repr__(self) -> str: return f"{type(self).__name__}({repr(self.name)}, {repr(self._original_value)})" - def __str__(self): + def __str__(self) -> str: delim = "==" if self.propagate else "=" - values = spack.parser.quote_if_needed(",".join(str(v) for v in self.value)) + values = spack.parser.quote_if_needed(",".join(str(v) for v in self.value_as_tuple)) return f"{self.name}{delim}{values}" @@ -423,7 +470,7 @@ class MultiValuedVariant(AbstractVariant): """A variant that can hold multiple values at once.""" @implicit_variant_conversion - def satisfies(self, other): + def satisfies(self, other: AbstractVariant) -> bool: """Returns true if ``other.name == self.name`` and ``other.value`` is a strict subset of self. Does not try to validate. @@ -443,22 +490,25 @@ class MultiValuedVariant(AbstractVariant): # allow prefix find on patches if self.name == "patches": - return all(any(w.startswith(v) for w in self.value) for v in other.value) + return all( + any(str(w).startswith(str(v)) for w in self.value_as_tuple) + for v in other.value_as_tuple + ) # Otherwise we want all the values in `other` to be also in `self` - return all(v in self.value for v in other.value) + return all(v in self for v in other.value_as_tuple) - def append(self, value): + def append(self, value: Union[str, bool]) -> None: """Add another value to this multi-valued variant.""" - self._value = tuple(sorted((value,) + self._value)) - self._original_value = ",".join(self._value) + self._value = tuple(sorted((value,) + self.value_as_tuple)) + self._original_value = ",".join(str(v) for v in self._value) - def __str__(self): + def __str__(self) -> str: # Special-case patches to not print the full 64 character sha256 if self.name == "patches": - values_str = ",".join(x[:7] for x in self.value) + values_str = ",".join(str(x)[:7] for x in self.value_as_tuple) else: - values_str = ",".join(str(x) for x in self.value) + values_str = ",".join(str(x) for x in self.value_as_tuple) delim = "==" if self.propagate else "=" return f"{self.name}{delim}{spack.parser.quote_if_needed(values_str)}" @@ -467,35 +517,33 @@ class MultiValuedVariant(AbstractVariant): class SingleValuedVariant(AbstractVariant): """A variant that can hold multiple values, but one at a time.""" - def _value_setter(self, value): + def _value_setter(self, value: ValueType) -> None: # Treat the value as a multi-valued variant super()._value_setter(value) # Then check if there's only a single value - if len(self._value) != 1: - raise MultipleValuesInExclusiveVariantError(self, None) - self._value = str(self._value[0]) + values = self.value_as_tuple + if len(values) != 1: + raise MultipleValuesInExclusiveVariantError(self) - def __str__(self): - delim = "==" if self.propagate else "=" - return f"{self.name}{delim}{spack.parser.quote_if_needed(self.value)}" + self._value = values[0] @implicit_variant_conversion - def satisfies(self, other): + def satisfies(self, other: "AbstractVariant") -> bool: abstract_sat = super().satisfies(other) return abstract_sat and ( self.value == other.value or other.value == "*" or self.value == "*" ) - def intersects(self, other): + def intersects(self, other: "AbstractVariant") -> bool: return self.satisfies(other) - def compatible(self, other): + def compatible(self, other: "AbstractVariant") -> bool: return self.satisfies(other) @implicit_variant_conversion - def constrain(self, other): + def constrain(self, other: "AbstractVariant") -> bool: if self.name != other.name: raise ValueError("variants must have the same name") @@ -510,12 +558,17 @@ class SingleValuedVariant(AbstractVariant): raise UnsatisfiableVariantSpecError(other.value, self.value) return False - def __contains__(self, item): + def __contains__(self, item: ValueType) -> bool: return item == self.value - def yaml_entry(self): + def yaml_entry(self) -> Tuple[str, SerializedValueType]: + assert isinstance(self.value, (bool, str)) return self.name, self.value + def __str__(self) -> str: + delim = "==" if self.propagate else "=" + return f"{self.name}{delim}{spack.parser.quote_if_needed(str(self.value))}" + class BoolValuedVariant(SingleValuedVariant): """A variant that can hold either True or False. @@ -523,7 +576,7 @@ class BoolValuedVariant(SingleValuedVariant): BoolValuedVariant can also hold the value '*', for coerced comparisons between ``foo=*`` and ``+foo`` or ``~foo``.""" - def _value_setter(self, value): + def _value_setter(self, value: ValueType) -> None: # Check the string representation of the value and turn # it to a boolean if str(value).upper() == "TRUE": @@ -540,13 +593,14 @@ class BoolValuedVariant(SingleValuedVariant): msg += "a value that does not represent a bool" raise ValueError(msg.format(self.name)) - def __contains__(self, item): + def __contains__(self, item: ValueType) -> bool: return item is self.value - def __str__(self): + def __str__(self) -> str: + sigil = "+" if self.value else "~" if self.propagate: - return "{0}{1}".format("++" if self.value else "~~", self.name) - return "{0}{1}".format("+" if self.value else "~", self.name) + sigil *= 2 + return f"{sigil}{self.name}" # The class below inherit from Sequence to disguise as a tuple and comply @@ -720,12 +774,15 @@ def disjoint_sets(*sets): class Value: """Conditional value that might be used in variants.""" - def __init__(self, value, when): + value: Any + when: Optional["spack.spec.Spec"] # optional b/c we need to know about disabled values + + def __init__(self, value: Any, when: Optional["spack.spec.Spec"]): self.value = value self.when = when def __repr__(self): - return "Value({0.value}, when={0.when})".format(self) + return f"Value({self.value}, when={self.when})" def __str__(self): return str(self.value) @@ -745,15 +802,92 @@ class Value: return self.value < other.value +def prevalidate_variant_value( + pkg_cls: "Type[spack.package_base.PackageBase]", + variant: AbstractVariant, + spec: Optional["spack.spec.Spec"] = None, + strict: bool = False, +) -> List[Variant]: + """Do as much validation of a variant value as is possible before concretization. + + This checks that the variant value is valid for *some* definition of the variant, and + it raises if we know *before* concretization that the value cannot occur. On success + it returns the variant definitions for which the variant is valid. + + Arguments: + pkg_cls: package in which variant is (potentially multiply) defined + variant: variant spec with value to validate + spec: optionally restrict validation only to variants defined for this spec + strict: if True, raise an exception if no variant definition is valid for any + constraint on the spec. + + Return: + list of variant definitions that will accept the given value. List will be empty + only if the variant is a reserved variant. + """ + # don't validate wildcards or variants with reserved names + if variant.value == ("*",) or variant.name in reserved_names: + return [] + + # raise if there is no definition at all + if not pkg_cls.has_variant(variant.name): + raise UnknownVariantError( + f"No such variant '{variant.name}' in package {pkg_cls.name}", [variant.name] + ) + + # do as much prevalidation as we can -- check only those + # variants whose when constraint intersects this spec + errors = [] + possible_definitions = [] + valid_definitions = [] + + for when, pkg_variant_def in pkg_cls.variant_definitions(variant.name): + if spec and not spec.intersects(when): + continue + possible_definitions.append(pkg_variant_def) + + try: + pkg_variant_def.validate_or_raise(variant, pkg_cls.name) + valid_definitions.append(pkg_variant_def) + except spack.error.SpecError as e: + errors.append(e) + + # value is valid for at least one definition -- return them all + if valid_definitions: + return valid_definitions + + # no when spec intersected, so no possible definition for the variant in this configuration + if strict and not possible_definitions: + when_clause = f" when {spec}" if spec else "" + raise InvalidVariantValueError( + f"variant '{variant.name}' does not exist for '{pkg_cls.name}'{when_clause}" + ) + + # There are only no errors if we're not strict and there are no possible_definitions. + # We are strict for audits but not for specs on the CLI or elsewhere. Being strict + # in these cases would violate our rule of being able to *talk* about any configuration, + # regardless of what the package.py currently says. + if not errors: + return [] + + # if there is just one error, raise the specific error + if len(errors) == 1: + raise errors[0] + + # otherwise combine all the errors and raise them together + raise InvalidVariantValueError( + "multiple variant issues:", "\n".join(e.message for e in errors) + ) + + class _ConditionalVariantValues(lang.TypedMutableSequence): """A list, just with a different type""" -def conditional(*values, **kwargs): +def conditional(*values: List[Any], when: Optional["spack.directives.WhenType"] = None): """Conditional values that can be used in variant declarations.""" - if len(kwargs) != 1 and "when" not in kwargs: - raise ValueError('conditional statement expects a "when=" parameter only') - when = kwargs["when"] + # _make_when_spec returns None when the condition is statically false. + when = spack.directives._make_when_spec(when) return _ConditionalVariantValues([Value(x, when=when) for x in values]) @@ -764,15 +898,9 @@ class DuplicateVariantError(error.SpecError): class UnknownVariantError(error.SpecError): """Raised when an unknown variant occurs in a spec.""" - def __init__(self, spec, variants): - self.unknown_variants = variants - variant_str = "variant" if len(variants) == 1 else "variants" - msg = ( - 'trying to set {0} "{1}" in package "{2}", but the package' - " has no such {0} [happened when validating '{3}']" - ) - msg = msg.format(variant_str, comma_or(variants), spec.name, spec.root) + def __init__(self, msg: str, unknown_variants: List[str]): super().__init__(msg) + self.unknown_variants = unknown_variants class InconsistentValidationError(error.SpecError): @@ -788,11 +916,10 @@ class MultipleValuesInExclusiveVariantError(error.SpecError, ValueError): only one. """ - def __init__(self, variant, pkg): - msg = 'multiple values are not allowed for variant "{0.name}"{1}' - pkg_info = "" - if pkg is not None: - pkg_info = ' in package "{0}"'.format(pkg.name) + def __init__(self, variant: AbstractVariant, pkg_name: Optional[str] = None): + pkg_info = "" if pkg_name is None else f" in package '{pkg_name}'" + msg = f"multiple values are not allowed for variant '{variant.name}'{pkg_info}" + super().__init__(msg.format(variant, pkg_info)) @@ -801,23 +928,7 @@ class InvalidVariantValueCombinationError(error.SpecError): class InvalidVariantValueError(error.SpecError): - """Raised when a valid variant has at least an invalid value.""" - - def __init__(self, variant, invalid_values, pkg): - msg = 'invalid values for variant "{0.name}"{2}: {1}\n' - pkg_info = "" - if pkg is not None: - pkg_info = ' in package "{0}"'.format(pkg.name) - super().__init__(msg.format(variant, invalid_values, pkg_info)) - - -class InvalidVariantForSpecError(error.SpecError): - """Raised when an invalid conditional variant is specified.""" - - def __init__(self, variant, when, spec): - msg = "Invalid variant {0} for spec {1}.\n" - msg += "{0} is only available for {1.name} when satisfying one of {2}." - super().__init__(msg.format(variant, spec, when)) + """Raised when variants have invalid values.""" class UnsatisfiableVariantSpecError(error.UnsatisfiableSpecError): |