summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMassimiliano Culpo <massimiliano.culpo@gmail.com>2022-04-05 02:37:57 +0200
committerGitHub <noreply@github.com>2022-04-04 17:37:57 -0700
commitf2fc4ee9afcb1a159853b4037a8747949d65f7d9 (patch)
treef11fea6e5c5ec845c9cfe85ef416d5cbae150c38
parentd64de54ebe83853694e30e562b7f9edc996ccd2d (diff)
downloadspack-f2fc4ee9afcb1a159853b4037a8747949d65f7d9.tar.gz
spack-f2fc4ee9afcb1a159853b4037a8747949d65f7d9.tar.bz2
spack-f2fc4ee9afcb1a159853b4037a8747949d65f7d9.tar.xz
spack-f2fc4ee9afcb1a159853b4037a8747949d65f7d9.zip
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
-rw-r--r--lib/spack/docs/packaging_guide.rst31
-rw-r--r--lib/spack/llnl/util/lang.py40
-rw-r--r--lib/spack/spack/pkgkit.py7
-rw-r--r--lib/spack/spack/solver/asp.py7
-rw-r--r--lib/spack/spack/test/concretize.py30
-rw-r--r--lib/spack/spack/variant.py64
-rw-r--r--var/spack/repos/builtin.mock/packages/conditional-values-in-variant/package.py34
-rw-r--r--var/spack/repos/builtin/packages/boost/package.py14
8 files changed, 214 insertions, 13 deletions
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')