diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 0cca744359..4514bd0e96 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1922,7 +1922,7 @@ class Head: node_flag = fn.attr("node_flag_set") node_flag_source = fn.attr("node_flag_source") node_flag_propagate = fn.attr("node_flag_propagate") - variant_propagate = fn.attr("variant_propagate") + variant_propagation_candidate = fn.attr("variant_propagation_candidate") class Body: node = fn.attr("node") @@ -1936,7 +1936,7 @@ class Body: node_flag = fn.attr("node_flag") node_flag_source = fn.attr("node_flag_source") node_flag_propagate = fn.attr("node_flag_propagate") - variant_propagate = fn.attr("variant_propagate") + variant_propagation_candidate = fn.attr("variant_propagation_candidate") f = Body if body else Head @@ -1985,7 +1985,9 @@ class Body: clauses.append(f.variant_value(spec.name, vname, value)) if variant.propagate: - clauses.append(f.variant_propagate(spec.name, vname, value, spec.name)) + clauses.append( + f.variant_propagation_candidate(spec.name, vname, value, spec.name) + ) # Tell the concretizer that this is a possible value for the # variant, to account for things like int/str values where we diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 5e98e5cf11..d5f24ddc3b 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -757,23 +757,36 @@ node_has_variant(node(ID, Package), Variant) :- pkg_fact(Package, variant(Variant)), attr("node", node(ID, Package)). -attr("variant_propagate", PackageNode, Variant, Value, Source) :- +% Variant propagation is forwarded to dependencies +attr("variant_propagation_candidate", PackageNode, Variant, Value, Source) :- attr("node", PackageNode), depends_on(ParentNode, PackageNode), - attr("variant_propagate", ParentNode, Variant, Value, Source), - not attr("variant_set", PackageNode, Variant). + attr("variant_value", node(_, Source), Variant, Value), + attr("variant_propagation_candidate", ParentNode, Variant, _, Source). -attr("variant_value", node(ID, Package), Variant, Value) :- - attr("node", node(ID, Package)), +% If the node is a candidate, and it has the variant and value, +% then those variant and value should be propagated +attr("variant_propagate", node(ID, Package), Variant, Value, Source) :- + attr("variant_propagation_candidate", node(ID, Package), Variant, Value, Source), node_has_variant(node(ID, Package), Variant), - attr("variant_propagate", node(ID, Package), Variant, Value, _), - pkg_fact(Package, variant_possible_value(Variant, Value)). + pkg_fact(Package, variant_possible_value(Variant, Value)), + not attr("variant_set", node(ID, Package), Variant). +% Propagate the value, if there is the corresponding attribute +attr("variant_value", PackageNode, Variant, Value) :- attr("variant_propagate", PackageNode, Variant, Value, _). + +% If a variant is propagated, we cannot have extraneous values (this is for multi valued variants) +variant_is_propagated(PackageNode, Variant) :- attr("variant_propagate", PackageNode, Variant, _, _). +:- variant_is_propagated(PackageNode, Variant), + attr("variant_value", PackageNode, Variant, Value), + not attr("variant_propagate", PackageNode, Variant, Value, _). + +% Cannot receive different values from different sources on the same variant error(100, "{0} and {1} cannot both propagate variant '{2}' to package {3} with values '{4}' and '{5}'", Source1, Source2, Variant, Package, Value1, Value2) :- attr("variant_propagate", node(X, Package), Variant, Value1, Source1), attr("variant_propagate", node(X, Package), Variant, Value2, Source2), node_has_variant(node(X, Package), Variant), - Value1 < Value2. + Value1 < Value2, Source1 < Source2. % a variant cannot be set if it is not a variant on the package error(100, "Cannot set variant '{0}' for package '{1}' because the variant condition cannot be satisfied for the given spec", Variant, Package) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 0af689ddd5..eba86d14fc 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -349,6 +349,9 @@ def test_compiler_flags_differ_identical_compilers(self): spec.concretize() assert spec.satisfies("cflags=-O2") + @pytest.mark.only_clingo( + "Optional compiler propagation isn't deprecated for original concretizer" + ) def test_concretize_compiler_flag_propagate(self): spec = Spec("hypre cflags=='-g' ^openblas") spec.concretize() @@ -458,19 +461,54 @@ def test_concretize_two_virtuals_with_dual_provider_and_a_conflict(self): @pytest.mark.only_clingo( "Optional compiler propagation isn't deprecated for original concretizer" ) - def test_concretize_propagate_disabled_variant(self): - """Test a package variant value was passed from its parent.""" - spec = Spec("hypre~~shared ^openblas") - spec.concretize() - - assert spec.satisfies("^openblas~shared") + @pytest.mark.parametrize( + "spec_str,expected_propagation", + [ + ("hypre~~shared ^openblas+shared", [("hypre", "~shared"), ("openblas", "+shared")]), + # Propagates past a node that doesn't have the variant + ("hypre~~shared ^openblas", [("hypre", "~shared"), ("openblas", "~shared")]), + ( + "ascent~~shared +adios2", + [("ascent", "~shared"), ("adios2", "~shared"), ("bzip2", "~shared")], + ), + # Propagates below a node that uses the other value explicitly + ( + "ascent~~shared +adios2 ^adios2+shared", + [("ascent", "~shared"), ("adios2", "+shared"), ("bzip2", "~shared")], + ), + ( + "ascent++shared +adios2 ^adios2~shared", + [("ascent", "+shared"), ("adios2", "~shared"), ("bzip2", "+shared")], + ), + ], + ) + def test_concretize_propagate_disabled_variant(self, spec_str, expected_propagation): + """Tests various patterns of boolean variant propagation""" + spec = Spec(spec_str).concretized() + for key, expected_satisfies in expected_propagation: + spec[key].satisfies(expected_satisfies) + @pytest.mark.only_clingo( + "Optional compiler propagation isn't deprecated for original concretizer" + ) def test_concretize_propagated_variant_is_not_passed_to_dependent(self): """Test a package variant value was passed from its parent.""" - spec = Spec("hypre~~shared ^openblas+shared") + spec = Spec("ascent~~shared +adios2 ^adios2+shared") spec.concretize() - assert spec.satisfies("^openblas+shared") + assert spec.satisfies("^adios2+shared") + assert spec.satisfies("^bzip2~shared") + + @pytest.mark.only_clingo( + "Optional compiler propagation isn't deprecated for original concretizer" + ) + def test_concretize_propagate_specified_variant(self): + """Test that only the specified variant is propagated to the dependencies""" + spec = Spec("parent-foo-bar ~~foo") + spec.concretize() + + assert spec.satisfies("~foo") and spec.satisfies("^dependency-foo-bar~foo") + assert spec.satisfies("+bar") and not spec.satisfies("^dependency-foo-bar+bar") @pytest.mark.only_clingo("Original concretizer is allowed to forego variant propagation") def test_concretize_propagate_multivalue_variant(self): diff --git a/var/spack/repos/builtin.mock/packages/adios2/package.py b/var/spack/repos/builtin.mock/packages/adios2/package.py new file mode 100644 index 0000000000..fb2f43ea0e --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/adios2/package.py @@ -0,0 +1,22 @@ +# Copyright 2013-2023 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) + + +from spack.package import * + + +class Adios2(Package): + """This packagae has the variants shared and + bzip2, both defaulted to True""" + + homepage = "https://example.com" + url = "https://example.com/adios2.tar.gz" + + version("2.9.1", sha256="ddfa32c14494250ee8a48ef1c97a1bf6442c15484bbbd4669228a0f90242f4f9") + + variant("shared", default=True, description="Build shared libraries") + variant("bzip2", default=True, description="Enable BZip2 compression") + + depends_on("bzip2") diff --git a/var/spack/repos/builtin.mock/packages/ascent/package.py b/var/spack/repos/builtin.mock/packages/ascent/package.py new file mode 100644 index 0000000000..9a8db472dc --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/ascent/package.py @@ -0,0 +1,21 @@ +# Copyright 2013-2023 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) + +from spack.package import * + + +class Ascent(Package): + """This packagae has the variants shared, defaulted + to True and adios2 defaulted to False""" + + homepage = "https://github.com/Alpine-DAV/ascent" + url = "http://www.example.com/ascent-1.0.tar.gz" + + version("0.9.2", sha256="44cd954aa5db478ab40042cd54fd6fcedf25000c3bb510ca23fcff8090531b91") + + variant("adios2", default=False, description="Build Adios2 filter support") + variant("shared", default=True, description="Build Ascent as shared libs") + + depends_on("adios2", when="+adios2") diff --git a/var/spack/repos/builtin.mock/packages/bzip2/package.py b/var/spack/repos/builtin.mock/packages/bzip2/package.py new file mode 100644 index 0000000000..326533ac5e --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/bzip2/package.py @@ -0,0 +1,19 @@ +# Copyright 2013-2023 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) + + +from spack.package import * + + +class Bzip2(Package): + """This packagae has the variants shared + defaulted to True""" + + homepage = "https://example.com" + url = "https://example.com/bzip2-1.0.8tar.gz" + + version("1.0.8", sha256="ab5a03176ee106d3f0fa90e381da478ddae405918153cca248e682cd0c4a2269") + + variant("shared", default=True, description="Enables the build of shared libraries.") diff --git a/var/spack/repos/builtin.mock/packages/dependency-foo-bar/package.py b/var/spack/repos/builtin.mock/packages/dependency-foo-bar/package.py new file mode 100644 index 0000000000..21e67f8a61 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/dependency-foo-bar/package.py @@ -0,0 +1,20 @@ +# Copyright 2013-2023 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) + +from spack.package import * + + +class DependencyFooBar(Package): + """This package has a variant "bar", which is False by default, and + variant "foo" which is True by default. + """ + + homepage = "http://www.example.com" + url = "http://www.example.com/dependency-foo-bar-1.0.tar.gz" + + version("1.0", md5="1234567890abcdefg1234567890098765") + + variant("foo", default=True, description="") + variant("bar", default=False, description="") diff --git a/var/spack/repos/builtin.mock/packages/parent-foo-bar/package.py b/var/spack/repos/builtin.mock/packages/parent-foo-bar/package.py new file mode 100644 index 0000000000..14516566a9 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/parent-foo-bar/package.py @@ -0,0 +1,22 @@ +# Copyright 2013-2023 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) + +from spack.package import * + + +class ParentFooBar(Package): + """This package has a variant "bar", which is True by default, and depends on another + package which has the same variant defaulting to False. + """ + + homepage = "http://www.example.com" + url = "http://www.example.com/parent-foo-bar-1.0.tar.gz" + + version("1.0", md5="abcdefg0123456789abcdefghfedcba0") + + variant("foo", default=True, description="") + variant("bar", default=True, description="") + + depends_on("dependency-foo-bar")