From f2fc4ee9afcb1a159853b4037a8747949d65f7d9 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 5 Apr 2022 02:37:57 +0200 Subject: Allow conditional possible values in variants (#29530) Allow declaring possible values for variants with an associated condition. If the variant takes one of those values, the condition is imposed as a further constraint. The idea of this PR is to implement part of the mechanisms needed for modeling [packages with multiple build-systems]( https://github.com/spack/seps/pull/3). After this PR the build-system directive can be implemented as: ```python variant( 'build-system', default='cmake', values=( 'autotools', conditional('cmake', when='@X.Y:') ), description='...', ) ``` Modifications: - [x] Allow conditional possible values in variants - [x] Add a unit-test for the feature - [x] Add documentation --- lib/spack/docs/packaging_guide.rst | 31 +++++++++++ lib/spack/llnl/util/lang.py | 40 +++++++++++++- lib/spack/spack/pkgkit.py | 7 ++- lib/spack/spack/solver/asp.py | 7 +++ lib/spack/spack/test/concretize.py | 30 ++++++++++ lib/spack/spack/variant.py | 64 ++++++++++++++++++++-- .../conditional-values-in-variant/package.py | 34 ++++++++++++ var/spack/repos/builtin/packages/boost/package.py | 14 ++--- 8 files changed, 214 insertions(+), 13 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/conditional-values-in-variant/package.py diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index eb62fa8bf8..4633ef25a8 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -1423,6 +1423,37 @@ other similar operations: ).with_default('auto').with_non_feature_values('auto'), ) +""""""""""""""""""""""""""" +Conditional Possible Values +""""""""""""""""""""""""""" + +There are cases where a variant may take multiple values, and the list of allowed values +expand over time. Think for instance at the C++ standard with which we might compile +Boost, which can take one of multiple possible values with the latest standards +only available from a certain version on. + +To model a similar situation we can use *conditional possible values* in the variant declaration: + +.. code-block:: python + + variant( + 'cxxstd', default='98', + values=( + '98', '11', '14', + # C++17 is not supported by Boost < 1.63.0. + conditional('17', when='@1.63.0:'), + # C++20/2a is not support by Boost < 1.73.0 + conditional('2a', '2b', when='@1.73.0:') + ), + multi=False, + description='Use the specified C++ standard when building.', + ) + +The snippet above allows ``98``, ``11`` and ``14`` as unconditional possible values for the +``cxxstd`` variant, while ``17`` requires a version greater or equal to ``1.63.0`` +and both ``2a`` and ``2b`` require a version greater or equal to ``1.73.0``. + + ^^^^^^^^^^^^^^^^^^^^ Conditional Variants ^^^^^^^^^^^^^^^^^^^^ diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py index 92e3a5c341..fb8b01aa84 100644 --- a/lib/spack/llnl/util/lang.py +++ b/lib/spack/llnl/util/lang.py @@ -16,7 +16,7 @@ from datetime import datetime, timedelta import six from six import string_types -from llnl.util.compat import MutableMapping, zip_longest +from llnl.util.compat import MutableMapping, MutableSequence, zip_longest # Ignore emacs backups when listing modules ignore_modules = [r'^\.#', '~$'] @@ -976,3 +976,41 @@ def enum(**kwargs): **kwargs: explicit dictionary of enums """ return type('Enum', (object,), kwargs) + + +class TypedMutableSequence(MutableSequence): + """Base class that behaves like a list, just with a different type. + + Client code can inherit from this base class: + + class Foo(TypedMutableSequence): + pass + + and later perform checks based on types: + + if isinstance(l, Foo): + # do something + """ + def __init__(self, iterable): + self.data = list(iterable) + + def __getitem__(self, item): + return self.data[item] + + def __setitem__(self, key, value): + self.data[key] = value + + def __delitem__(self, key): + del self.data[key] + + def __len__(self): + return len(self.data) + + def insert(self, index, item): + self.data.insert(index, item) + + def __repr__(self): + return repr(self.data) + + def __str__(self): + return str(self.data) diff --git a/lib/spack/spack/pkgkit.py b/lib/spack/spack/pkgkit.py index f4dd3dbe61..3fde349b9a 100644 --- a/lib/spack/spack/pkgkit.py +++ b/lib/spack/spack/pkgkit.py @@ -73,5 +73,10 @@ from spack.package import ( ) from spack.spec import InvalidSpecDetected, Spec from spack.util.executable import * -from spack.variant import any_combination_of, auto_or_any_combination_of, disjoint_sets +from spack.variant import ( + any_combination_of, + auto_or_any_combination_of, + conditional, + disjoint_sets, +) from spack.version import Version, ver diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 89c9108fc1..bbce46026d 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -878,6 +878,13 @@ class SpackSolverSetup(object): for value in sorted(values): self.gen.fact(fn.variant_possible_value(pkg.name, name, value)) + if hasattr(value, 'when'): + required = spack.spec.Spec('{0}={1}'.format(name, value)) + imposed = spack.spec.Spec(value.when) + imposed.name = pkg.name + self.condition( + required_spec=required, imposed_spec=imposed, name=pkg.name + ) if variant.sticky: self.gen.fact(fn.variant_sticky(pkg.name, name)) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index f47dc58073..92ff38ea34 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -1410,3 +1410,33 @@ class TestConcretize(object): # Here there is no known satisfying version - use the one on the spec. assert ver("2.7.21") == Spec("python@2.7.21").concretized().version + + @pytest.mark.parametrize('spec_str', [ + 'conditional-values-in-variant@1.62.0 cxxstd=17', + 'conditional-values-in-variant@1.62.0 cxxstd=2a', + 'conditional-values-in-variant@1.72.0 cxxstd=2a', + # Ensure disjoint set of values work too + 'conditional-values-in-variant@1.72.0 staging=flexpath', + ]) + def test_conditional_values_in_variants(self, spec_str): + if spack.config.get('config:concretizer') == 'original': + pytest.skip( + "Original concretizer doesn't resolve conditional values in variants" + ) + + s = Spec(spec_str) + with pytest.raises((RuntimeError, spack.error.UnsatisfiableSpecError)): + s.concretize() + + def test_conditional_values_in_conditional_variant(self): + """Test that conditional variants play well with conditional possible values""" + if spack.config.get('config:concretizer') == 'original': + pytest.skip( + "Original concretizer doesn't resolve conditional values in variants" + ) + + s = Spec('conditional-values-in-variant@1.50.0').concretized() + assert 'cxxstd' not in s.variants + + s = Spec('conditional-values-in-variant@1.60.0').concretized() + assert 'cxxstd' in s.variants diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index ff0f910adc..4ec016830e 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -12,6 +12,7 @@ import inspect import itertools import re +import six from six import StringIO import llnl.util.lang as lang @@ -79,10 +80,9 @@ class Variant(object): # If 'values' is a callable, assume it is a single value # validator and reset the values to be explicit during debug self.single_value_validator = values - else: - # Otherwise assume values is the set of allowed explicit values - self.values = values + # 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) self.multi = multi @@ -213,6 +213,22 @@ def implicit_variant_conversion(method): return convert +def _flatten(values): + """Flatten instances of _ConditionalVariantValues for internal representation""" + if isinstance(values, DisjointSetsOfValues): + return values + + flattened = [] + for item in values: + if isinstance(item, _ConditionalVariantValues): + flattened.extend(item) + else: + flattened.append(item) + # There are parts of the variant checking mechanism that expect to find tuples + # here, so it is important to convert the type once we flattened the values. + return tuple(flattened) + + @lang.lazy_lexicographic_ordering class AbstractVariant(object): """A variant that has not yet decided who it wants to be. It behaves like @@ -701,7 +717,7 @@ class DisjointSetsOfValues(Sequence): _empty_set = set(('none',)) def __init__(self, *sets): - self.sets = [set(x) for x in sets] + self.sets = [set(_flatten(x)) for x in sets] # 'none' is a special value and can appear only in a set of # a single element @@ -852,6 +868,46 @@ def disjoint_sets(*sets): return DisjointSetsOfValues(*sets).allow_empty_set().with_default('none') +@functools.total_ordering +class Value(object): + """Conditional value that might be used in variants.""" + def __init__(self, value, when): + self.value = value + self.when = when + + def __repr__(self): + return 'Value({0.value}, when={0.when})'.format(self) + + def __str__(self): + return str(self.value) + + def __hash__(self): + # Needed to allow testing the presence of a variant in a set by its value + return hash(self.value) + + def __eq__(self, other): + if isinstance(other, six.string_types): + return self.value == other + return self.value == other.value + + def __lt__(self, other): + if isinstance(other, six.string_types): + return self.value < other + return self.value < other.value + + +class _ConditionalVariantValues(lang.TypedMutableSequence): + """A list, just with a different type""" + + +def conditional(*values, **kwargs): + """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'] + return _ConditionalVariantValues([Value(x, when=when) for x in values]) + + class DuplicateVariantError(error.SpecError): """Raised when the same variant occurs in a spec twice.""" diff --git a/var/spack/repos/builtin.mock/packages/conditional-values-in-variant/package.py b/var/spack/repos/builtin.mock/packages/conditional-values-in-variant/package.py new file mode 100644 index 0000000000..74895cb7c0 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/conditional-values-in-variant/package.py @@ -0,0 +1,34 @@ +# Copyright 2013-2022 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 ConditionalValuesInVariant(Package): + """Package with conditional possible values in a variant""" + homepage = "https://dev.null" + + version('1.73.0') + version('1.72.0') + version('1.62.0') + version('1.60.0') + version('1.50.0') + + variant( + 'cxxstd', default='98', + values=( + '98', '11', '14', + # C++17 is not supported by Boost < 1.63.0. + conditional('17', when='@1.63.0:'), + # C++20/2a is not support by Boost < 1.73.0 + conditional('2a', when='@1.73.0:') + ), + multi=False, + description='Use the specified C++ standard when building.', + when='@1.60.0:' + ) + + variant( + 'staging', values=any_combination_of( + conditional('flexpath', 'dataspaces', when='@1.73.0:') + ), + description='Enable dataspaces and/or flexpath staging transports' + ) diff --git a/var/spack/repos/builtin/packages/boost/package.py b/var/spack/repos/builtin/packages/boost/package.py index 3b5e83813a..9ac8584288 100644 --- a/var/spack/repos/builtin/packages/boost/package.py +++ b/var/spack/repos/builtin/packages/boost/package.py @@ -140,7 +140,13 @@ class Boost(Package): variant('cxxstd', default='98', - values=('98', '11', '14', '17', '2a'), + values=( + '98', '11', '14', + # C++17 is not supported by Boost < 1.63.0. + conditional('17', when='@1.63.0:'), + # C++20/2a is not support by Boost < 1.73.0 + conditional('2a', when='@1.73.0:') + ), multi=False, description='Use the specified C++ standard when building.') variant('debug', default=False, @@ -193,12 +199,6 @@ class Boost(Package): conflicts('cxxstd=98', when='+fiber') # Fiber requires >=C++11. conflicts('~context', when='+fiber') # Fiber requires Context. - # C++20/2a is not support by Boost < 1.73.0 - conflicts('cxxstd=2a', when='@:1.72') - - # C++17 is not supported by Boost<1.63.0. - conflicts('cxxstd=17', when='@:1.62') - conflicts('+taggedlayout', when='+versionedlayout') conflicts('+numpy', when='~python') -- cgit v1.2.3-60-g2f50