Skip to content

Commit c452153

Browse files
alalazoscheibelp
alalazo
authored and
scheibelp
committed
Multi-valued variants: better support for combinations (#9481)
This enforces conventions that allow for correct handling of multi-valued variants where specifying no value is an option, and adds convenience functionality for specifying multi-valued variants with conflicting sets of values. This also adds a notion of "feature values" for variants, which are those that are understood by the build system (e.g. those that would appear as configure options). In more detail: * Add documentation on variants to the packaging guide * Forbid usage of '' or None as a possible variant value, in particular as a default. To indicate choosing no value, the user must explicitly define an option like 'none'. Without this, multi-valued variants with default set to None were not parsable from the command line (Fixes #6314) * Add "disjoint_sets" function to support the declaration of multi-valued variants with conflicting sets of options. For example a variant "foo" with possible values "a", "b", and "c" where "c" is exclusive of the other values ("foo=a,b" and "foo=c" are valid but "foo=a,c" is not). * Add "any_combination_of" function to support the declaration of multi-valued variants where it is valid to choose none of the values. This automatically defines "none" as an option (exclusive with all other choices); this value does not appear when iterating over the variant's values, for example in "with_or_without" (which constructs autotools option strings from variant values). * The "disjoint_sets" and "any_combination_of" methods return an object which tracks the possible values. It is also possible to indicate that some of these values do not correspond to options understood by the package's build system, such that methods like "with_or_without" will not define options for those values (this occurs automatically for "none") * Add documentation for usage of new functions for specifying multi-valued variants
1 parent 051057c commit c452153

24 files changed

Lines changed: 585 additions & 89 deletions

File tree

‎lib/spack/docs/packaging_guide.rst‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,174 @@ Go cannot be used to fetch a particular commit or branch, it always
10771077
downloads the head of the repository. This download method is untrusted,
10781078
and is not recommended. Use another fetch strategy whenever possible.
10791079

1080+
--------
1081+
Variants
1082+
--------
1083+
1084+
Many software packages can be configured to enable optional
1085+
features, which often come at the expense of additional dependencies or
1086+
longer build-times. To be flexible enough and support a wide variety of
1087+
use cases, Spack permits to expose to the end-user the ability to choose
1088+
which features should be activated in a package at the time it is installed.
1089+
The mechanism to be employed is the :py:func:`spack.directives.variant` directive.
1090+
1091+
^^^^^^^^^^^^^^^^
1092+
Boolean variants
1093+
^^^^^^^^^^^^^^^^
1094+
1095+
In their simplest form variants are boolean options specified at the package
1096+
level:
1097+
1098+
.. code-block:: python
1099+
1100+
class Hdf5(AutotoolsPackage):
1101+
...
1102+
variant(
1103+
'shared', default=True, description='Builds a shared version of the library'
1104+
)
1105+
1106+
with a default value and a description of their meaning / use in the package.
1107+
*Variants can be tested in any context where a spec constraint is expected.*
1108+
In the example above the ``shared`` variant is tied to the build of shared dynamic
1109+
libraries. To pass the right option at configure time we can branch depending on
1110+
its value:
1111+
1112+
.. code-block:: python
1113+
1114+
def configure_args(self):
1115+
...
1116+
if '+shared' in self.spec:
1117+
extra_args.append('--enable-shared')
1118+
else:
1119+
extra_args.append('--disable-shared')
1120+
extra_args.append('--enable-static-exec')
1121+
1122+
As explained in :ref:`basic-variants` the constraint ``+shared`` means
1123+
that the boolean variant is set to ``True``, while ``~shared`` means it is set
1124+
to ``False``.
1125+
Another common example is the optional activation of an extra dependency
1126+
which requires to use the variant in the ``when`` argument of
1127+
:py:func:`spack.directives.depends_on`:
1128+
1129+
.. code-block:: python
1130+
1131+
class Hdf5(AutotoolsPackage):
1132+
...
1133+
variant('szip', default=False, description='Enable szip support')
1134+
depends_on('szip', when='+szip')
1135+
1136+
as shown in the snippet above where ``szip`` is modeled to be an optional
1137+
dependency of ``hdf5``.
1138+
1139+
^^^^^^^^^^^^^^^^^^^^^
1140+
Multi-valued variants
1141+
^^^^^^^^^^^^^^^^^^^^^
1142+
1143+
If need be, Spack can go beyond Boolean variants and permit an arbitrary
1144+
number of allowed values. This might be useful when modeling
1145+
options that are tightly related to each other.
1146+
The values in this case are passed to the :py:func:`spack.directives.variant`
1147+
directive as a tuple:
1148+
1149+
.. code-block:: python
1150+
1151+
class Blis(Package):
1152+
...
1153+
variant(
1154+
'threads', default='none', description='Multithreading support',
1155+
values=('pthreads', 'openmp', 'none'), multi=False
1156+
)
1157+
1158+
In the example above the argument ``multi`` is set to ``False`` to indicate
1159+
that only one among all the variant values can be active at any time. This
1160+
constraint is enforced by the parser and an error is emitted if a user
1161+
specifies two or more values at the same time:
1162+
1163+
.. code-block:: console
1164+
1165+
$ spack spec blis threads=openmp,pthreads
1166+
Input spec
1167+
--------------------------------
1168+
blis threads=openmp,pthreads
1169+
1170+
Concretized
1171+
--------------------------------
1172+
==> Error: multiple values are not allowed for variant "threads"
1173+
1174+
Another useful note is that *Python's* ``None`` *is not allowed as a default value*
1175+
and therefore it should not be used to denote that no feature was selected.
1176+
Users should instead select another value, like ``'none'``, and handle it explicitly
1177+
within the package recipe if need be:
1178+
1179+
.. code-block:: python
1180+
1181+
if self.spec.variants['threads'].value == 'none':
1182+
options.append('--no-threads')
1183+
1184+
In cases where multiple values can be selected at the same time ``multi`` should
1185+
be set to ``True``:
1186+
1187+
.. code-block:: python
1188+
1189+
class Gcc(AutotoolsPackage):
1190+
...
1191+
variant(
1192+
'languages', default='c,c++,fortran',
1193+
values=('ada', 'brig', 'c', 'c++', 'fortran',
1194+
'go', 'java', 'jit', 'lto', 'objc', 'obj-c++'),
1195+
multi=True,
1196+
description='Compilers and runtime libraries to build'
1197+
)
1198+
1199+
Within a package recipe a multi-valued variant is tested using a ``key=value`` syntax:
1200+
1201+
.. code-block:: python
1202+
1203+
if 'languages=jit' in spec:
1204+
options.append('--enable-host-shared')
1205+
1206+
"""""""""""""""""""""""""""""""""""""""""""
1207+
Complex validation logic for variant values
1208+
"""""""""""""""""""""""""""""""""""""""""""
1209+
To cover complex use cases, the :py:func:`spack.directives.variant` directive
1210+
could accept as the ``values`` argument a full-fledged object which has
1211+
``default`` and other arguments of the directive embedded as attributes.
1212+
1213+
An example, already implemented in Spack's core, is :py:class:`spack.variant.DisjointSetsOfValues`.
1214+
This class is used to implement a few convenience functions, like
1215+
:py:func:`spack.variant.any_combination_of`:
1216+
1217+
.. code-block:: python
1218+
1219+
class Adios(AutotoolsPackage):
1220+
...
1221+
variant(
1222+
'staging',
1223+
values=any_combination_of('flexpath', 'dataspaces'),
1224+
description='Enable dataspaces and/or flexpath staging transports'
1225+
)
1226+
1227+
that allows any combination of the specified values, and also allows the
1228+
user to specify ``'none'`` (as a string) to choose none of them.
1229+
The objects returned by these functions can be modified at will by chaining
1230+
method calls to change the default value, customize the error message or
1231+
other similar operations:
1232+
1233+
.. code-block:: python
1234+
1235+
class Mvapich2(AutotoolsPackage):
1236+
...
1237+
variant(
1238+
'process_managers',
1239+
description='List of the process managers to activate',
1240+
values=disjoint_sets(
1241+
('auto',), ('slurm',), ('hydra', 'gforker', 'remshell')
1242+
).prohibit_empty_set().with_error(
1243+
"'slurm' or 'auto' cannot be activated along with "
1244+
"other process managers"
1245+
).with_default('auto').with_non_feature_values('auto'),
1246+
)
1247+
10801248
------------------------------------
10811249
Resources (expanding extra tarballs)
10821250
------------------------------------

‎lib/spack/spack/build_systems/autotools.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,9 +359,17 @@ def _activate_or_not(
359359
options = [(name, condition in spec)]
360360
else:
361361
condition = '{name}={value}'
362+
# "feature_values" is used to track values which correspond to
363+
# features which can be enabled or disabled as understood by the
364+
# package's build system. It excludes values which have special
365+
# meanings and do not correspond to features (e.g. "none")
366+
feature_values = getattr(
367+
self.variants[name].values, 'feature_values', None
368+
) or self.variants[name].values
369+
362370
options = [
363371
(value, condition.format(name=name, value=value) in spec)
364-
for value in self.variants[name].values
372+
for value in feature_values
365373
]
366374

367375
# For each allowed value in the list of values

‎lib/spack/spack/build_systems/cuda.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55

66
from spack.package import PackageBase
77
from spack.directives import depends_on, variant, conflicts
8+
89
import platform
910

11+
import spack.variant
12+
1013

1114
class CudaPackage(PackageBase):
1215
"""Auxiliary class which contains CUDA variant, dependencies and conflicts
@@ -19,11 +22,12 @@ class CudaPackage(PackageBase):
1922
description='Build with CUDA')
2023
# see http://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html#gpu-feature-list
2124
# https://developer.nvidia.com/cuda-gpus
22-
variant('cuda_arch', default=None,
25+
variant('cuda_arch',
2326
description='CUDA architecture',
24-
values=('20', '30', '32', '35', '50', '52', '53', '60', '61',
25-
'62', '70'),
26-
multi=True)
27+
values=spack.variant.any_combination_of(
28+
'20', '30', '32', '35', '50', '52', '53', '60', '61',
29+
'62', '70'
30+
))
2731

2832
# see http://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html#nvcc-examples
2933
# and http://llvm.org/docs/CompileCudaWithLLVM.html#compiling-cuda-code

‎lib/spack/spack/directives.py‎

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class OpenMpi(Package):
3434
from six import string_types
3535

3636
import llnl.util.lang
37+
import llnl.util.tty.color
3738

3839
import spack.error
3940
import spack.patch
@@ -439,7 +440,7 @@ def variant(
439440
default=None,
440441
description='',
441442
values=None,
442-
multi=False,
443+
multi=None,
443444
validator=None):
444445
"""Define a variant for the package. Packager can specify a default
445446
value as well as a text description.
@@ -456,23 +457,65 @@ def variant(
456457
multi (bool): if False only one value per spec is allowed for
457458
this variant
458459
validator (callable): optional group validator to enforce additional
459-
logic. It receives a tuple of values and should raise an instance
460-
of SpackError if the group doesn't meet the additional constraints
460+
logic. It receives the package name, the variant name and a tuple
461+
of values and should raise an instance of SpackError if the group
462+
doesn't meet the additional constraints
463+
464+
Raises:
465+
DirectiveError: if arguments passed to the directive are invalid
461466
"""
467+
def format_error(msg, pkg):
468+
msg += " @*r{{[{0}, variant '{1}']}}"
469+
return llnl.util.tty.color.colorize(msg.format(pkg.name, name))
470+
462471
if name in reserved_names:
463-
raise ValueError("The variant name '%s' is reserved by Spack" % name)
472+
def _raise_reserved_name(pkg):
473+
msg = "The name '%s' is reserved by Spack" % name
474+
raise DirectiveError(format_error(msg, pkg))
475+
return _raise_reserved_name
464476

477+
# Ensure we have a sequence of allowed variant values, or a
478+
# predicate for it.
465479
if values is None:
466-
if default in (True, False) or (type(default) is str and
467-
default.upper() in ('TRUE', 'FALSE')):
480+
if str(default).upper() in ('TRUE', 'FALSE'):
468481
values = (True, False)
469482
else:
470483
values = lambda x: True
471484

472-
if default is None:
473-
default = False if values == (True, False) else ''
485+
# The object defining variant values might supply its own defaults for
486+
# all the other arguments. Ensure we have no conflicting definitions
487+
# in place.
488+
for argument in ('default', 'multi', 'validator'):
489+
# TODO: we can consider treating 'default' differently from other
490+
# TODO: attributes and let a packager decide whether to use the fluent
491+
# TODO: interface or the directive argument
492+
if hasattr(values, argument) and locals()[argument] is not None:
493+
def _raise_argument_error(pkg):
494+
msg = "Remove specification of {0} argument: it is handled " \
495+
"by an attribute of the 'values' argument"
496+
raise DirectiveError(format_error(msg.format(argument), pkg))
497+
return _raise_argument_error
498+
499+
# Allow for the object defining the allowed values to supply its own
500+
# default value and group validator, say if it supports multiple values.
501+
default = getattr(values, 'default', default)
502+
validator = getattr(values, 'validator', validator)
503+
multi = getattr(values, 'multi', bool(multi))
504+
505+
# Here we sanitize against a default value being either None
506+
# or the empty string, as the former indicates that a default
507+
# was not set while the latter will make the variant unparsable
508+
# from the command line
509+
if default is None or default == '':
510+
def _raise_default_not_set(pkg):
511+
if default is None:
512+
msg = "either a default was not explicitly set, " \
513+
"or 'None' was used"
514+
elif default == '':
515+
msg = "the default cannot be an empty string"
516+
raise DirectiveError(format_error(msg, pkg))
517+
return _raise_default_not_set
474518

475-
default = default
476519
description = str(description).strip()
477520

478521
def _execute_variant(pkg):

‎lib/spack/spack/pkgkit.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,6 @@
4747
from spack.package import \
4848
install_dependency_symlinks, flatten_dependencies, \
4949
DependencyConflictError, InstallError, ExternalPackageError
50+
51+
from spack.variant import any_combination_of, auto_or_any_combination_of
52+
from spack.variant import disjoint_sets

‎lib/spack/spack/test/build_systems.py‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,11 @@ def test_with_or_without(self):
123123
s.concretize()
124124
pkg = spack.repo.get(s)
125125

126-
# Called without parameters
127126
options = pkg.with_or_without('foo')
127+
128+
# Ensure that values that are not representing a feature
129+
# are not used by with_or_without
130+
assert '--without-none' not in options
128131
assert '--with-bar' in options
129132
assert '--without-baz' in options
130133
assert '--no-fee' in options
@@ -133,14 +136,30 @@ def activate(value):
133136
return 'something'
134137

135138
options = pkg.with_or_without('foo', activation_value=activate)
139+
assert '--without-none' not in options
136140
assert '--with-bar=something' in options
137141
assert '--without-baz' in options
138142
assert '--no-fee' in options
139143

140144
options = pkg.enable_or_disable('foo')
145+
assert '--disable-none' not in options
141146
assert '--enable-bar' in options
142147
assert '--disable-baz' in options
143148
assert '--disable-fee' in options
144149

145150
options = pkg.with_or_without('bvv')
146151
assert '--with-bvv' in options
152+
153+
def test_none_is_allowed(self):
154+
s = Spec('a foo=none')
155+
s.concretize()
156+
pkg = spack.repo.get(s)
157+
158+
options = pkg.with_or_without('foo')
159+
160+
# Ensure that values that are not representing a feature
161+
# are not used by with_or_without
162+
assert '--with-none' not in options
163+
assert '--without-bar' in options
164+
assert '--without-baz' in options
165+
assert '--no-fee' in options

0 commit comments

Comments
 (0)