From ab3f1b10db7e9a64338f44ccca515a8356b4c43c Mon Sep 17 00:00:00 2001 From: Andrew W Elble Date: Tue, 8 Dec 2020 09:46:52 -0500 Subject: 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. --- lib/spack/spack/solver/asp.py | 17 ++++++------- lib/spack/spack/solver/concretize.lp | 3 +++ lib/spack/spack/spec.py | 34 ++++++++++++++++++++++++++ lib/spack/spack/test/concretize.py | 4 +++ lib/spack/spack/test/concretize_preferences.py | 4 ++- 5 files changed, 52 insertions(+), 10 deletions(-) (limited to 'lib') 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 -- cgit v1.2.3-70-g09d2