Skip to content

Commit 67cd92e

Browse files
becker33becker33
authored
Allow conditional variants (spack#24858)
A common question from users has been how to model variants that are new in new versions of a package, or variants that are dependent on other variants. Our stock answer so far has been an unsatisfying combination of "just have it do nothing in the old version" and "tell Spack it conflicts". This PR enables conditional variants, on any spec condition. The syntax is straightforward, and matches that of previous features.
1 parent 78c08fc commit 67cd92e

File tree

13 files changed

+217
-32
lines changed

13 files changed

+217
-32
lines changed

‎lib/spack/docs/packaging_guide.rst‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,6 +1419,60 @@ other similar operations:
14191419
).with_default('auto').with_non_feature_values('auto'),
14201420
)
14211421
1422+
^^^^^^^^^^^^^^^^^^^^
1423+
Conditional Variants
1424+
^^^^^^^^^^^^^^^^^^^^
1425+
1426+
The variant directive accepts a ``when`` clause. The variant will only
1427+
be present on specs that otherwise satisfy the spec listed as the
1428+
``when`` clause. For example, the following class has a variant
1429+
``bar`` when it is at version 2.0 or higher.
1430+
1431+
.. code-block:: python
1432+
1433+
class Foo(Package):
1434+
...
1435+
variant('bar', default=False, when='@2.0:', description='help message')
1436+
1437+
The ``when`` clause follows the same syntax and accepts the same
1438+
values as the ``when`` argument of
1439+
:py:func:`spack.directives.depends_on`
1440+
1441+
^^^^^^^^^^^^^^^^^^^
1442+
Overriding Variants
1443+
^^^^^^^^^^^^^^^^^^^
1444+
1445+
Packages may override variants for several reasons, most often to
1446+
change the default from a variant defined in a parent class or to
1447+
change the conditions under which a variant is present on the spec.
1448+
1449+
When a variant is defined multiple times, whether in the same package
1450+
file or in a subclass and a superclass, the last definition is used
1451+
for all attributes **except** for the ``when`` clauses. The ``when``
1452+
clauses are accumulated through all invocations, and the variant is
1453+
present on the spec if any of the accumulated conditions are
1454+
satisfied.
1455+
1456+
For example, consider the following package:
1457+
1458+
.. code-block:: python
1459+
1460+
class Foo(Package):
1461+
...
1462+
variant('bar', default=False, when='@1.0', description='help1')
1463+
variant('bar', default=True, when='platform=darwin', description='help2')
1464+
...
1465+
1466+
This package ``foo`` has a variant ``bar`` when the spec satisfies
1467+
either ``@1.0`` or ``platform=darwin``, but not for other platforms at
1468+
other versions. The default for this variant, when it is present, is
1469+
always ``True``, regardless of which condition of the variant is
1470+
satisfied. This allows packages to override variants in packages or
1471+
build system classes from which they inherit, by modifying the variant
1472+
values without modifying the ``when`` clause. It also allows a package
1473+
to implement ``or`` semantics for a variant ``when`` clause by
1474+
duplicating the variant definition.
1475+
14221476
------------------------------------
14231477
Resources (expanding extra tarballs)
14241478
------------------------------------

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,8 @@ def _activate_or_not(
521521

522522
# Create a list of pairs. Each pair includes a configuration
523523
# option and whether or not that option is activated
524-
if set(self.variants[variant].values) == set((True, False)):
524+
variant_desc, _ = self.variants[variant]
525+
if set(variant_desc.values) == set((True, False)):
525526
# BoolValuedVariant carry information about a single option.
526527
# Nonetheless, for uniformity of treatment we'll package them
527528
# in an iterable of one element.
@@ -534,8 +535,8 @@ def _activate_or_not(
534535
# package's build system. It excludes values which have special
535536
# meanings and do not correspond to features (e.g. "none")
536537
feature_values = getattr(
537-
self.variants[variant].values, 'feature_values', None
538-
) or self.variants[variant].values
538+
variant_desc.values, 'feature_values', None
539+
) or variant_desc.values
539540

540541
options = [
541542
(value,

‎lib/spack/spack/cmd/config.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,8 @@ def config_prefer_upstream(args):
433433
or var_name not in spec.package.variants):
434434
continue
435435

436-
if variant.value != spec.package.variants[var_name].default:
436+
variant_desc, _ = spec.package.variants[var_name]
437+
if variant.value != variant_desc.default:
437438
variants.append(str(variant))
438439
variants.sort()
439440
variants = ' '.join(variants)

‎lib/spack/spack/cmd/info.py‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def variant(s):
5757
class VariantFormatter(object):
5858
def __init__(self, variants):
5959
self.variants = variants
60-
self.headers = ('Name [Default]', 'Allowed values', 'Description')
60+
self.headers = ('Name [Default]', 'When', 'Allowed values', 'Description')
6161

6262
# Formats
6363
fmt_name = '{0} [{1}]'
@@ -68,36 +68,41 @@ def __init__(self, variants):
6868
self.column_widths = [len(x) for x in self.headers]
6969

7070
# Expand columns based on max line lengths
71-
for k, v in variants.items():
71+
for k, e in variants.items():
72+
v, w = e
7273
candidate_max_widths = (
7374
len(fmt_name.format(k, self.default(v))), # Name [Default]
75+
len(str(w)),
7476
len(v.allowed_values), # Allowed values
7577
len(v.description) # Description
7678
)
7779

7880
self.column_widths = (
7981
max(self.column_widths[0], candidate_max_widths[0]),
8082
max(self.column_widths[1], candidate_max_widths[1]),
81-
max(self.column_widths[2], candidate_max_widths[2])
83+
max(self.column_widths[2], candidate_max_widths[2]),
84+
max(self.column_widths[3], candidate_max_widths[3])
8285
)
8386

8487
# Don't let name or possible values be less than max widths
8588
_, cols = tty.terminal_size()
8689
max_name = min(self.column_widths[0], 30)
87-
max_vals = min(self.column_widths[1], 20)
90+
max_when = min(self.column_widths[1], 30)
91+
max_vals = min(self.column_widths[2], 20)
8892

8993
# allow the description column to extend as wide as the terminal.
9094
max_description = min(
91-
self.column_widths[2],
95+
self.column_widths[3],
9296
# min width 70 cols, 14 cols of margins and column spacing
9397
max(cols, 70) - max_name - max_vals - 14,
9498
)
95-
self.column_widths = (max_name, max_vals, max_description)
99+
self.column_widths = (max_name, max_when, max_vals, max_description)
96100

97101
# Compute the format
98-
self.fmt = "%%-%ss%%-%ss%%s" % (
102+
self.fmt = "%%-%ss%%-%ss%%-%ss%%s" % (
99103
self.column_widths[0] + 4,
100-
self.column_widths[1] + 4
104+
self.column_widths[1] + 4,
105+
self.column_widths[2] + 4
101106
)
102107

103108
def default(self, v):
@@ -115,21 +120,27 @@ def lines(self):
115120
underline = tuple([w * "=" for w in self.column_widths])
116121
yield ' ' + self.fmt % underline
117122
yield ''
118-
for k, v in sorted(self.variants.items()):
123+
for k, e in sorted(self.variants.items()):
124+
v, w = e
119125
name = textwrap.wrap(
120126
'{0} [{1}]'.format(k, self.default(v)),
121127
width=self.column_widths[0]
122128
)
129+
if len(w) == 1:
130+
w = w[0]
131+
if w == spack.spec.Spec():
132+
w = '--'
133+
when = textwrap.wrap(str(w), width=self.column_widths[1])
123134
allowed = v.allowed_values.replace('True, False', 'on, off')
124-
allowed = textwrap.wrap(allowed, width=self.column_widths[1])
135+
allowed = textwrap.wrap(allowed, width=self.column_widths[2])
125136
description = []
126137
for d_line in v.description.split('\n'):
127138
description += textwrap.wrap(
128139
d_line,
129-
width=self.column_widths[2]
140+
width=self.column_widths[3]
130141
)
131142
for t in zip_longest(
132-
name, allowed, description, fillvalue=''
143+
name, when, allowed, description, fillvalue=''
133144
):
134145
yield " " + self.fmt % t
135146

@@ -232,7 +243,7 @@ def print_text_info(pkg):
232243

233244
formatter = VariantFormatter(pkg.variants)
234245
for line in formatter.lines:
235-
color.cprint(line)
246+
color.cprint(color.cescape(line))
236247

237248
if hasattr(pkg, 'phases') and pkg.phases:
238249
color.cprint('')

‎lib/spack/spack/concretize.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,20 +384,25 @@ def concretize_variants(self, spec):
384384
changed = False
385385
preferred_variants = PackagePrefs.preferred_variants(spec.name)
386386
pkg_cls = spec.package_class
387-
for name, variant in pkg_cls.variants.items():
387+
for name, entry in pkg_cls.variants.items():
388+
variant, when = entry
388389
var = spec.variants.get(name, None)
389390
if var and '*' in var:
390391
# remove variant wildcard before concretizing
391392
# wildcard cannot be combined with other variables in a
392393
# multivalue variant, a concrete variant cannot have the value
393394
# wildcard, and a wildcard does not constrain a variant
394395
spec.variants.pop(name)
395-
if name not in spec.variants:
396+
if name not in spec.variants and any(spec.satisfies(w)
397+
for w in when):
396398
changed = True
397399
if name in preferred_variants:
398400
spec.variants[name] = preferred_variants.get(name)
399401
else:
400402
spec.variants[name] = variant.make_default()
403+
if name in spec.variants and not any(spec.satisfies(w)
404+
for w in when):
405+
raise vt.InvalidVariantForSpecError(name, when, spec)
401406

402407
return changed
403408

‎lib/spack/spack/directives.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ def _wrapper(*args, **kwargs):
244244
if DirectiveMeta._when_constraints_from_context:
245245
# Check that directives not yet supporting the when= argument
246246
# are not used inside the context manager
247-
if decorated_function.__name__ in ('version', 'variant'):
247+
if decorated_function.__name__ == 'version':
248248
msg = ('directive "{0}" cannot be used within a "when"'
249249
' context since it does not support a "when=" '
250250
'argument')
@@ -562,7 +562,8 @@ def variant(
562562
description='',
563563
values=None,
564564
multi=None,
565-
validator=None):
565+
validator=None,
566+
when=None):
566567
"""Define a variant for the package. Packager can specify a default
567568
value as well as a text description.
568569
@@ -581,6 +582,8 @@ def variant(
581582
logic. It receives the package name, the variant name and a tuple
582583
of values and should raise an instance of SpackError if the group
583584
doesn't meet the additional constraints
585+
when (spack.spec.Spec, bool): optional condition on which the
586+
variant applies
584587
585588
Raises:
586589
DirectiveError: if arguments passed to the directive are invalid
@@ -640,14 +643,23 @@ def _raise_default_not_set(pkg):
640643
description = str(description).strip()
641644

642645
def _execute_variant(pkg):
646+
when_spec = make_when_spec(when)
647+
when_specs = [when_spec]
648+
643649
if not re.match(spack.spec.identifier_re, name):
644650
directive = 'variant'
645651
msg = "Invalid variant name in {0}: '{1}'"
646652
raise DirectiveError(directive, msg.format(pkg.name, name))
647653

648-
pkg.variants[name] = spack.variant.Variant(
654+
if name in pkg.variants:
655+
# We accumulate when specs, but replace the rest of the variant
656+
# with the newer values
657+
_, orig_when = pkg.variants[name]
658+
when_specs += orig_when
659+
660+
pkg.variants[name] = (spack.variant.Variant(
649661
name, default, description, values, multi, validator
650-
)
662+
), when_specs)
651663
return _execute_variant
652664

653665

‎lib/spack/spack/solver/asp.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -721,8 +721,12 @@ def pkg_rules(self, pkg, tests):
721721
self.gen.newline()
722722

723723
# variants
724-
for name, variant in sorted(pkg.variants.items()):
725-
self.gen.fact(fn.variant(pkg.name, name))
724+
for name, entry in sorted(pkg.variants.items()):
725+
variant, when = entry
726+
727+
for w in when:
728+
cond_id = self.condition(w, name=pkg.name)
729+
self.gen.fact(fn.variant_condition(cond_id, pkg.name, name))
726730

727731
single_value = not variant.multi
728732
if single_value:
@@ -788,7 +792,7 @@ def condition(self, required_spec, imposed_spec=None, name=None):
788792
789793
Arguments:
790794
required_spec (spack.spec.Spec): the spec that triggers this condition
791-
imposed_spec (spack.spec.Spec or None): the sepc with constraints that
795+
imposed_spec (spack.spec.Spec or None): the spec with constraints that
792796
are imposed when this condition is triggered
793797
name (str or None): name for `required_spec` (required if
794798
required_spec is anonymous, ignored if not)
@@ -1087,7 +1091,7 @@ class Body(object):
10871091
reserved_names = spack.directives.reserved_names
10881092
if not spec.virtual and vname not in reserved_names:
10891093
try:
1090-
variant_def = spec.package.variants[vname]
1094+
variant_def, _ = spec.package.variants[vname]
10911095
except KeyError:
10921096
msg = 'variant "{0}" not found in package "{1}"'
10931097
raise RuntimeError(msg.format(vname, spec.name))

‎lib/spack/spack/solver/concretize.lp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,20 @@ external_conditions_hold(Package, LocalIndex) :-
350350
%-----------------------------------------------------------------------------
351351
% Variant semantics
352352
%-----------------------------------------------------------------------------
353+
% a variant is a variant of a package if it is a variant under some condition
354+
% and that condition holds
355+
variant(Package, Variant) :- variant_condition(ID, Package, Variant),
356+
condition_holds(ID).
357+
358+
% a variant cannot be set if it is not a variant on the package
359+
:- variant_set(Package, Variant),
360+
not variant(Package, Variant),
361+
error("Unsatisfied conditional variants cannot be set").
362+
363+
% a variant cannot take on a value if it is not a variant of the package
364+
:- variant_value(Package, Variant, _), not variant(Package, Variant),
365+
error("Unsatisfied conditional variants cannot take on a variant value").
366+
353367
% one variant value for single-valued variants.
354368
1 {
355369
variant_value(Package, Variant, Value)

‎lib/spack/spack/spec.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3081,7 +3081,7 @@ def update_variant_validate(self, variant_name, values):
30813081
if not isinstance(values, tuple):
30823082
values = (values,)
30833083

3084-
pkg_variant = self.package_class.variants[variant_name]
3084+
pkg_variant, _ = self.package_class.variants[variant_name]
30853085

30863086
for value in values:
30873087
if self.variants.get(variant_name):

‎lib/spack/spack/test/concretize.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import spack.error
1717
import spack.platforms
1818
import spack.repo
19+
import spack.variant as vt
1920
from spack.concretize import find_spec
2021
from spack.spec import Spec
2122
from spack.util.mock_package import MockPackageMultiRepo
@@ -738,6 +739,41 @@ def test_compiler_conflicts_in_package_py(self, spec_str, expected_str):
738739
s = Spec(spec_str).concretized()
739740
assert s.satisfies(expected_str)
740741

742+
@pytest.mark.parametrize('spec_str,expected,unexpected', [
743+
('conditional-variant-pkg@1.0',
744+
['two_whens'],
745+
['version_based', 'variant_based']),
746+
('conditional-variant-pkg@2.0',
747+
['version_based', 'variant_based'],
748+
['two_whens']),
749+
('conditional-variant-pkg@2.0~version_based',
750+
['version_based'],
751+
['variant_based', 'two_whens']),
752+
('conditional-variant-pkg@2.0+version_based+variant_based',
753+
['version_based', 'variant_based', 'two_whens'],
754+
[])
755+
])
756+
def test_conditional_variants(self, spec_str, expected, unexpected):
757+
s = Spec(spec_str).concretized()
758+
759+
for var in expected:
760+
assert s.satisfies('%s=*' % var)
761+
for var in unexpected:
762+
assert not s.satisfies('%s=*' % var)
763+
764+
@pytest.mark.parametrize('bad_spec', [
765+
'@1.0~version_based',
766+
'@1.0+version_based',
767+
'@2.0~version_based+variant_based',
768+
'@2.0+version_based~variant_based+two_whens',
769+
])
770+
def test_conditional_variants_fail(self, bad_spec):
771+
with pytest.raises(
772+
(spack.error.UnsatisfiableSpecError,
773+
vt.InvalidVariantForSpecError)
774+
):
775+
_ = Spec('conditional-variant-pkg' + bad_spec).concretized()
776+
741777
@pytest.mark.parametrize('spec_str,expected,unexpected', [
742778
('py-extension3 ^python@3.5.1', [], ['py-extension1']),
743779
('py-extension3 ^python@2.7.11', ['py-extension1'], []),

0 commit comments

Comments
 (0)