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
This commit is contained in:
		 Massimiliano Culpo
					Massimiliano Culpo
				
			
				
					committed by
					
						 GitHub
						GitHub
					
				
			
			
				
	
			
			
			 GitHub
						GitHub
					
				
			
						parent
						
							d64de54ebe
						
					
				
				
					commit
					f2fc4ee9af
				
			| @@ -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 | ||||
| ^^^^^^^^^^^^^^^^^^^^ | ||||
|   | ||||
| @@ -16,7 +16,7 @@ | ||||
| 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) | ||||
|   | ||||
| @@ -73,5 +73,10 @@ | ||||
| ) | ||||
| 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 | ||||
|   | ||||
| @@ -878,6 +878,13 @@ def pkg_rules(self, pkg, tests): | ||||
| 
 | ||||
|             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)) | ||||
|   | ||||
| @@ -1410,3 +1410,33 @@ def test_do_not_invent_new_concrete_versions_unless_necessary(self): | ||||
| 
 | ||||
|         # 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 | ||||
|   | ||||
| @@ -12,6 +12,7 @@ | ||||
| import itertools | ||||
| import re | ||||
| 
 | ||||
| import six | ||||
| from six import StringIO | ||||
| 
 | ||||
| import llnl.util.lang as lang | ||||
| @@ -79,10 +80,9 @@ def isa_type(v): | ||||
|             # 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 convert(self, other): | ||||
|     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.""" | ||||
| 
 | ||||
|   | ||||
		Reference in New Issue
	
	Block a user