diff options
-rw-r--r-- | lib/spack/docs/packaging_guide.rst | 54 | ||||
-rw-r--r-- | lib/spack/spack/build_systems/autotools.py | 7 | ||||
-rw-r--r-- | lib/spack/spack/cmd/config.py | 3 | ||||
-rw-r--r-- | lib/spack/spack/cmd/info.py | 37 | ||||
-rw-r--r-- | lib/spack/spack/concretize.py | 9 | ||||
-rw-r--r-- | lib/spack/spack/directives.py | 20 | ||||
-rw-r--r-- | lib/spack/spack/solver/asp.py | 12 | ||||
-rw-r--r-- | lib/spack/spack/solver/concretize.lp | 14 | ||||
-rw-r--r-- | lib/spack/spack/spec.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/test/concretize.py | 36 | ||||
-rw-r--r-- | lib/spack/spack/test/package_sanity.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/variant.py | 25 | ||||
-rw-r--r-- | var/spack/repos/builtin.mock/packages/conditional-variant-pkg/package.py | 24 |
13 files changed, 217 insertions, 32 deletions
diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index b9fbb18cf7..dbf1c83078 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -1419,6 +1419,60 @@ other similar operations: ).with_default('auto').with_non_feature_values('auto'), ) +^^^^^^^^^^^^^^^^^^^^ +Conditional Variants +^^^^^^^^^^^^^^^^^^^^ + +The variant directive accepts a ``when`` clause. The variant will only +be present on specs that otherwise satisfy the spec listed as the +``when`` clause. For example, the following class has a variant +``bar`` when it is at version 2.0 or higher. + +.. code-block:: python + + class Foo(Package): + ... + variant('bar', default=False, when='@2.0:', description='help message') + +The ``when`` clause follows the same syntax and accepts the same +values as the ``when`` argument of +:py:func:`spack.directives.depends_on` + +^^^^^^^^^^^^^^^^^^^ +Overriding Variants +^^^^^^^^^^^^^^^^^^^ + +Packages may override variants for several reasons, most often to +change the default from a variant defined in a parent class or to +change the conditions under which a variant is present on the spec. + +When a variant is defined multiple times, whether in the same package +file or in a subclass and a superclass, the last definition is used +for all attributes **except** for the ``when`` clauses. The ``when`` +clauses are accumulated through all invocations, and the variant is +present on the spec if any of the accumulated conditions are +satisfied. + +For example, consider the following package: + +.. code-block:: python + + class Foo(Package): + ... + variant('bar', default=False, when='@1.0', description='help1') + variant('bar', default=True, when='platform=darwin', description='help2') + ... + +This package ``foo`` has a variant ``bar`` when the spec satisfies +either ``@1.0`` or ``platform=darwin``, but not for other platforms at +other versions. The default for this variant, when it is present, is +always ``True``, regardless of which condition of the variant is +satisfied. This allows packages to override variants in packages or +build system classes from which they inherit, by modifying the variant +values without modifying the ``when`` clause. It also allows a package +to implement ``or`` semantics for a variant ``when`` clause by +duplicating the variant definition. + ------------------------------------ Resources (expanding extra tarballs) ------------------------------------ diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index f19d5fa54b..e0c8cf2e03 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -521,7 +521,8 @@ 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 - if set(self.variants[variant].values) == set((True, False)): + variant_desc, _ = self.variants[variant] + if set(variant_desc.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. @@ -534,8 +535,8 @@ To resolve this problem, please try the following: # package's build system. It excludes values which have special # meanings and do not correspond to features (e.g. "none") feature_values = getattr( - self.variants[variant].values, 'feature_values', None - ) or self.variants[variant].values + variant_desc.values, 'feature_values', None + ) or variant_desc.values options = [ (value, diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py index 0dfb40c04e..7bdefd290e 100644 --- a/lib/spack/spack/cmd/config.py +++ b/lib/spack/spack/cmd/config.py @@ -433,7 +433,8 @@ def config_prefer_upstream(args): or var_name not in spec.package.variants): continue - if variant.value != spec.package.variants[var_name].default: + variant_desc, _ = spec.package.variants[var_name] + if variant.value != variant_desc.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 b4994a7b2d..73c632907a 100644 --- a/lib/spack/spack/cmd/info.py +++ b/lib/spack/spack/cmd/info.py @@ -57,7 +57,7 @@ def variant(s): class VariantFormatter(object): def __init__(self, variants): self.variants = variants - self.headers = ('Name [Default]', 'Allowed values', 'Description') + self.headers = ('Name [Default]', 'When', 'Allowed values', 'Description') # Formats fmt_name = '{0} [{1}]' @@ -68,9 +68,11 @@ class VariantFormatter(object): self.column_widths = [len(x) for x in self.headers] # Expand columns based on max line lengths - for k, v in variants.items(): + for k, e in variants.items(): + v, w = e candidate_max_widths = ( len(fmt_name.format(k, self.default(v))), # Name [Default] + len(str(w)), len(v.allowed_values), # Allowed values len(v.description) # Description ) @@ -78,26 +80,29 @@ class VariantFormatter(object): self.column_widths = ( max(self.column_widths[0], candidate_max_widths[0]), max(self.column_widths[1], candidate_max_widths[1]), - max(self.column_widths[2], candidate_max_widths[2]) + max(self.column_widths[2], candidate_max_widths[2]), + max(self.column_widths[3], candidate_max_widths[3]) ) # Don't let name or possible values be less than max widths _, cols = tty.terminal_size() max_name = min(self.column_widths[0], 30) - max_vals = min(self.column_widths[1], 20) + max_when = min(self.column_widths[1], 30) + max_vals = min(self.column_widths[2], 20) # allow the description column to extend as wide as the terminal. max_description = min( - self.column_widths[2], + self.column_widths[3], # min width 70 cols, 14 cols of margins and column spacing max(cols, 70) - max_name - max_vals - 14, ) - self.column_widths = (max_name, max_vals, max_description) + self.column_widths = (max_name, max_when, max_vals, max_description) # Compute the format - self.fmt = "%%-%ss%%-%ss%%s" % ( + self.fmt = "%%-%ss%%-%ss%%-%ss%%s" % ( self.column_widths[0] + 4, - self.column_widths[1] + 4 + self.column_widths[1] + 4, + self.column_widths[2] + 4 ) def default(self, v): @@ -115,21 +120,27 @@ class VariantFormatter(object): underline = tuple([w * "=" for w in self.column_widths]) yield ' ' + self.fmt % underline yield '' - for k, v in sorted(self.variants.items()): + for k, e in sorted(self.variants.items()): + v, w = e name = textwrap.wrap( '{0} [{1}]'.format(k, self.default(v)), width=self.column_widths[0] ) + if len(w) == 1: + w = w[0] + if w == spack.spec.Spec(): + w = '--' + when = textwrap.wrap(str(w), width=self.column_widths[1]) allowed = v.allowed_values.replace('True, False', 'on, off') - allowed = textwrap.wrap(allowed, width=self.column_widths[1]) + allowed = textwrap.wrap(allowed, width=self.column_widths[2]) description = [] for d_line in v.description.split('\n'): description += textwrap.wrap( d_line, - width=self.column_widths[2] + width=self.column_widths[3] ) for t in zip_longest( - name, allowed, description, fillvalue='' + name, when, allowed, description, fillvalue='' ): yield " " + self.fmt % t @@ -232,7 +243,7 @@ def print_text_info(pkg): formatter = VariantFormatter(pkg.variants) for line in formatter.lines: - color.cprint(line) + color.cprint(color.cescape(line)) if hasattr(pkg, 'phases') and pkg.phases: color.cprint('') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 1b865c32cc..aed8fe7713 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -384,7 +384,8 @@ class Concretizer(object): changed = False preferred_variants = PackagePrefs.preferred_variants(spec.name) pkg_cls = spec.package_class - for name, variant in pkg_cls.variants.items(): + for name, entry in pkg_cls.variants.items(): + variant, when = entry var = spec.variants.get(name, None) if var and '*' in var: # remove variant wildcard before concretizing @@ -392,12 +393,16 @@ class Concretizer(object): # multivalue variant, a concrete variant cannot have the value # wildcard, and a wildcard does not constrain a variant spec.variants.pop(name) - if name not in spec.variants: + if name not in spec.variants and any(spec.satisfies(w) + for w in when): changed = True if name in preferred_variants: spec.variants[name] = preferred_variants.get(name) else: spec.variants[name] = variant.make_default() + if name in spec.variants and not any(spec.satisfies(w) + for w in when): + raise vt.InvalidVariantForSpecError(name, when, spec) return changed diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 863001a601..f7400f4da9 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -244,7 +244,7 @@ class DirectiveMeta(type): if DirectiveMeta._when_constraints_from_context: # Check that directives not yet supporting the when= argument # are not used inside the context manager - if decorated_function.__name__ in ('version', 'variant'): + if decorated_function.__name__ == 'version': msg = ('directive "{0}" cannot be used within a "when"' ' context since it does not support a "when=" ' 'argument') @@ -562,7 +562,8 @@ def variant( description='', values=None, multi=None, - validator=None): + validator=None, + when=None): """Define a variant for the package. Packager can specify a default value as well as a text description. @@ -581,6 +582,8 @@ def variant( logic. It receives the package name, the variant name and a tuple of values and should raise an instance of SpackError if the group doesn't meet the additional constraints + when (spack.spec.Spec, bool): optional condition on which the + variant applies Raises: DirectiveError: if arguments passed to the directive are invalid @@ -640,14 +643,23 @@ def variant( description = str(description).strip() 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)) - pkg.variants[name] = spack.variant.Variant( + 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 - ) + ), when_specs) return _execute_variant diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 2d70df210f..e83b38cf8d 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -721,8 +721,12 @@ class SpackSolverSetup(object): self.gen.newline() # variants - for name, variant in sorted(pkg.variants.items()): - self.gen.fact(fn.variant(pkg.name, name)) + for name, entry in sorted(pkg.variants.items()): + variant, when = entry + + for w in when: + cond_id = self.condition(w, name=pkg.name) + self.gen.fact(fn.variant_condition(cond_id, pkg.name, name)) single_value = not variant.multi if single_value: @@ -788,7 +792,7 @@ class SpackSolverSetup(object): Arguments: required_spec (spack.spec.Spec): the spec that triggers this condition - imposed_spec (spack.spec.Spec or None): the sepc with constraints that + imposed_spec (spack.spec.Spec or None): the spec with constraints that are imposed when this condition is triggered name (str or None): name for `required_spec` (required if required_spec is anonymous, ignored if not) @@ -1087,7 +1091,7 @@ class SpackSolverSetup(object): reserved_names = spack.directives.reserved_names if not spec.virtual and vname not in reserved_names: try: - variant_def = spec.package.variants[vname] + variant_def, _ = spec.package.variants[vname] except KeyError: msg = 'variant "{0}" not found in package "{1}"' raise RuntimeError(msg.format(vname, spec.name)) diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index a65a0d530e..00c8abe82d 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -350,6 +350,20 @@ external_conditions_hold(Package, LocalIndex) :- %----------------------------------------------------------------------------- % Variant semantics %----------------------------------------------------------------------------- +% a variant is a variant of a package if it is a variant under some condition +% and that condition holds +variant(Package, Variant) :- variant_condition(ID, Package, Variant), + condition_holds(ID). + +% a variant cannot be set if it is not a variant on the package +:- variant_set(Package, Variant), + not variant(Package, Variant), + error("Unsatisfied conditional variants cannot be set"). + +% a variant cannot take on a value if it is not a variant of the package +:- variant_value(Package, Variant, _), not variant(Package, Variant), + error("Unsatisfied conditional variants cannot take on a variant value"). + % one variant value for single-valued variants. 1 { variant_value(Package, Variant, Value) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 2c5e6086e1..ca3ad5f317 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3081,7 +3081,7 @@ class Spec(object): if not isinstance(values, tuple): values = (values,) - pkg_variant = self.package_class.variants[variant_name] + pkg_variant, _ = self.package_class.variants[variant_name] for value in values: if self.variants.get(variant_name): diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index b2ac1b6446..62d2af8bd3 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -16,6 +16,7 @@ import spack.concretize import spack.error import spack.platforms import spack.repo +import spack.variant as vt from spack.concretize import find_spec from spack.spec import Spec from spack.util.mock_package import MockPackageMultiRepo @@ -739,6 +740,41 @@ class TestConcretize(object): assert s.satisfies(expected_str) @pytest.mark.parametrize('spec_str,expected,unexpected', [ + ('conditional-variant-pkg@1.0', + ['two_whens'], + ['version_based', 'variant_based']), + ('conditional-variant-pkg@2.0', + ['version_based', 'variant_based'], + ['two_whens']), + ('conditional-variant-pkg@2.0~version_based', + ['version_based'], + ['variant_based', 'two_whens']), + ('conditional-variant-pkg@2.0+version_based+variant_based', + ['version_based', 'variant_based', 'two_whens'], + []) + ]) + def test_conditional_variants(self, spec_str, expected, unexpected): + s = Spec(spec_str).concretized() + + for var in expected: + assert s.satisfies('%s=*' % var) + for var in unexpected: + assert not s.satisfies('%s=*' % var) + + @pytest.mark.parametrize('bad_spec', [ + '@1.0~version_based', + '@1.0+version_based', + '@2.0~version_based+variant_based', + '@2.0+version_based~variant_based+two_whens', + ]) + def test_conditional_variants_fail(self, bad_spec): + with pytest.raises( + (spack.error.UnsatisfiableSpecError, + vt.InvalidVariantForSpecError) + ): + _ = Spec('conditional-variant-pkg' + bad_spec).concretized() + + @pytest.mark.parametrize('spec_str,expected,unexpected', [ ('py-extension3 ^python@3.5.1', [], ['py-extension1']), ('py-extension3 ^python@2.7.11', ['py-extension1'], []), ('py-extension3@1.0 ^python@2.7.11', ['patchelf@0.9'], []), diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py index f65e008ef4..35840d85da 100644 --- a/lib/spack/spack/test/package_sanity.py +++ b/lib/spack/spack/test/package_sanity.py @@ -247,7 +247,8 @@ def test_variant_defaults_are_parsable_from_cli(): """Ensures that variant defaults are parsable from cli.""" failing = [] for pkg in spack.repo.path.all_packages(): - for variant_name, variant in pkg.variants.items(): + for variant_name, entry in pkg.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 @@ -262,7 +263,8 @@ def test_variant_defaults_are_parsable_from_cli(): def test_variant_defaults_listed_explicitly_in_values(): failing = [] for pkg in spack.repo.path.all_packages(): - for variant_name, variant in pkg.variants.items(): + for variant_name, entry in pkg.variants.items(): + variant, _ = entry vspec = variant.make_default() try: variant.validate_or_raise(vspec, pkg=pkg) diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 3badd30c20..7550af95fa 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -181,6 +181,17 @@ class Variant(object): return BoolValuedVariant return SingleValuedVariant + def __eq__(self, other): + 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) + + def __ne__(self, other): + return not self == other + def implicit_variant_conversion(method): """Converts other to type(self) and calls method(self, other) @@ -645,10 +656,10 @@ def substitute_abstract_variants(spec): new_variant = SingleValuedVariant(name, v._original_value) spec.variants.substitute(new_variant) continue - pkg_variant = spec.package_class.variants.get(name, None) - if not pkg_variant: + if name not in spec.package_class.variants: failed.append(name) continue + pkg_variant, _ = spec.package_class.variants[name] new_variant = pkg_variant.make_variant(v._original_value) pkg_variant.validate_or_raise(new_variant, spec.package_class) spec.variants.substitute(new_variant) @@ -880,6 +891,16 @@ class InvalidVariantValueError(error.SpecError): ) +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(InvalidVariantForSpecError, self).__init__( + msg.format(variant, spec, when) + ) + + class UnsatisfiableVariantSpecError(error.UnsatisfiableSpecError): """Raised when a spec variant conflicts with package constraints.""" diff --git a/var/spack/repos/builtin.mock/packages/conditional-variant-pkg/package.py b/var/spack/repos/builtin.mock/packages/conditional-variant-pkg/package.py new file mode 100644 index 0000000000..1b7a2c3afe --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/conditional-variant-pkg/package.py @@ -0,0 +1,24 @@ +# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +class ConditionalVariantPkg(Package): + """This package is used to test conditional variants.""" + homepage = "http://www.example.com/conditional-variant-pkg" + url = "http://www.unit-test-should-replace-this-url/conditional-variant-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') + version('2.0', 'abcdef0123456789abcdef0123456789') + + variant('version_based', default=True, when='@2.0:', + description="Check that version constraints work") + + variant('variant_based', default=False, when='+version_based', + description="Check that variants can depend on variants") + + variant('two_whens', default=False, when='@1.0') + variant('two_whens', default=False, when='+variant_based') + + def install(self, spec, prefix): + assert False |