summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew W Elble <aweits@rit.edu>2020-12-08 09:46:52 -0500
committerTamara Dahlgren <dahlgren1@llnl.gov>2021-02-17 17:07:24 -0800
commitab3f1b10db7e9a64338f44ccca515a8356b4c43c (patch)
treeb99703e340e137bd03ec3a781f4344c650540b6b
parent30a9e6462f73aa0a50a9f1455dca3f3657cd9d8e (diff)
downloadspack-ab3f1b10db7e9a64338f44ccca515a8356b4c43c.tar.gz
spack-ab3f1b10db7e9a64338f44ccca515a8356b4c43c.tar.bz2
spack-ab3f1b10db7e9a64338f44ccca515a8356b4c43c.tar.xz
spack-ab3f1b10db7e9a64338f44ccca515a8356b4c43c.zip
concretizer: try hard to obtain all needed variant_possible_value()'s (#20102)
Track all the variant values mentioned when emitting constraints, validate them and emit a fact that allows them as possible values. This modification ensures that open-ended variants (variants accepting any string or any integer) are projected to the finite set of values that are relevant for this concretization.
-rw-r--r--lib/spack/spack/solver/asp.py17
-rw-r--r--lib/spack/spack/solver/concretize.lp3
-rw-r--r--lib/spack/spack/spec.py34
-rw-r--r--lib/spack/spack/test/concretize.py4
-rw-r--r--lib/spack/spack/test/concretize_preferences.py4
-rw-r--r--var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent-type/package.py16
-rw-r--r--var/spack/repos/builtin.mock/packages/singlevalue-variant/package.py19
7 files changed, 87 insertions, 10 deletions
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 13bcfd2e91..2d4598bd32 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1104,7 +1104,14 @@ class SpackSolverSetup(object):
if not isinstance(values, tuple):
values = (values,)
+ # perform validation of the variant and values
+ spec = spack.spec.Spec(pkg_name)
+ spec.update_variant_validate(variant_name, values)
+
for value in values:
+ self.variant_values_from_specs.add(
+ (pkg_name, variant.name, value)
+ )
self.gen.fact(fn.variant_default_value_from_packages_yaml(
pkg_name, variant.name, value
))
@@ -1692,15 +1699,7 @@ class SpecBuilder(object):
)
return
- pkg_class = spack.repo.path.get_pkg_class(pkg)
-
- variant = self._specs[pkg].variants.get(name)
- if variant:
- # it's multi-valued
- variant.append(value)
- else:
- variant = pkg_class.variants[name].make_variant(value)
- self._specs[pkg].variants[name] = variant
+ self._specs[pkg].update_variant_validate(name, value)
def version(self, pkg, version):
self._specs[pkg].versions = ver([version])
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 96bf23a45c..728331d91c 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -183,6 +183,9 @@ external_spec(Package, ID) :-
% if a variant is set to anything, it is considered 'set'.
variant_set(Package, Variant) :- variant_set(Package, Variant, _).
+% A variant cannot have a value that is not also a possible value
+:- variant_value(Package, Variant, Value), not variant_possible_value(Package, Variant, Value).
+
% variant_set is an explicitly set variant value. If it's not 'set',
% we revert to the default value. If it is set, we force the set value
variant_value(Package, Variant, Value)
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 46400859d3..88516470fb 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -2893,6 +2893,40 @@ class Spec(object):
if not_existing:
raise vt.UnknownVariantError(spec, not_existing)
+ def update_variant_validate(self, variant_name, values):
+ """If it is not already there, adds the variant named
+ `variant_name` to the spec `spec` based on the definition
+ contained in the package metadata. Validates the variant and
+ values before returning.
+
+ Used to add values to a variant without being sensitive to the
+ variant being single or multi-valued. If the variant already
+ exists on the spec it is assumed to be multi-valued and the
+ values are appended.
+
+ Args:
+ variant_name: the name of the variant to add or append to
+ values: the value or values (as a tuple) to add/append
+ to the variant
+ """
+ if not isinstance(values, tuple):
+ values = (values,)
+
+ pkg_variant = self.package_class.variants[variant_name]
+
+ for value in values:
+ if self.variants.get(variant_name):
+ msg = ("Cannot append a value to a single-valued "
+ "variant with an already set value")
+ assert pkg_variant.multi, msg
+ self.variants[variant_name].append(value)
+ else:
+ variant = pkg_variant.make_variant(value)
+ self.variants[variant_name] = variant
+
+ pkg_variant.validate_or_raise(
+ self.variants[variant_name], self.package)
+
def constrain(self, other, deps=True):
"""Merge the constraints of other with self.
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index f5f4ac4ad9..f98d9140fa 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -264,6 +264,10 @@ class TestConcretize(object):
s.concretize()
assert s['mpi'].version == ver('1.10.3')
+ def test_concretize_dependent_with_singlevalued_variant_type(self):
+ s = Spec('singlevalue-variant-dependent-type')
+ s.concretize()
+
@pytest.mark.parametrize("spec,version", [
('dealii', 'develop'),
('xsdk', '0.4.0'),
diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py
index 9ec304e624..9abac7221d 100644
--- a/lib/spack/spack/test/concretize_preferences.py
+++ b/lib/spack/spack/test/concretize_preferences.py
@@ -82,7 +82,9 @@ class TestConcretizePreferences(object):
{'debug': True, 'opt': True, 'shared': False, 'static': False}),
# Check a multivalued variant with multiple values set
('multivalue-variant', ['foo=bar,baz', 'fee=bar'],
- {'foo': ('bar', 'baz'), 'fee': 'bar'})
+ {'foo': ('bar', 'baz'), 'fee': 'bar'}),
+ ('singlevalue-variant', ['fum=why'],
+ {'fum': 'why'})
])
def test_preferred_variants(
self, package_name, variant_value, expected_results
diff --git a/var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent-type/package.py b/var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent-type/package.py
new file mode 100644
index 0000000000..8b42a82b8d
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent-type/package.py
@@ -0,0 +1,16 @@
+# Copyright 2013-2020 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 SinglevalueVariantDependentType(Package):
+ """Simple package with one dependency that has a single-valued
+ variant with values=str"""
+
+ homepage = "http://www.example.com"
+ url = "http://www.example.com/archive-1.0.tar.gz"
+
+ version('1.0', '0123456789abcdef0123456789abcdef')
+
+ depends_on('singlevalue-variant fum=nope')
diff --git a/var/spack/repos/builtin.mock/packages/singlevalue-variant/package.py b/var/spack/repos/builtin.mock/packages/singlevalue-variant/package.py
new file mode 100644
index 0000000000..fa6eca1527
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/singlevalue-variant/package.py
@@ -0,0 +1,19 @@
+# Copyright 2013-2020 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 SinglevalueVariant(Package):
+ homepage = "http://www.llnl.gov"
+ url = "http://www.llnl.gov/mpileaks-1.0.tar.gz"
+
+ version(1.0, 'foobarbaz')
+
+ variant(
+ 'fum',
+ description='Single-valued variant with type in values',
+ default='bar',
+ values=str,
+ multi=False
+ )