summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMassimiliano Culpo <massimiliano.culpo@gmail.com>2020-12-01 07:45:48 +0100
committerTamara Dahlgren <dahlgren1@llnl.gov>2021-02-17 17:07:18 -0800
commit22d7937c50fa56e8db3819ae1039c0bb26383bd1 (patch)
tree7c68f6028ed1e2722bd1565557af580a586b1b83
parent96283867d616f05db701b9256cb631eb0b4cc1c8 (diff)
downloadspack-22d7937c50fa56e8db3819ae1039c0bb26383bd1.tar.gz
spack-22d7937c50fa56e8db3819ae1039c0bb26383bd1.tar.bz2
spack-22d7937c50fa56e8db3819ae1039c0bb26383bd1.tar.xz
spack-22d7937c50fa56e8db3819ae1039c0bb26383bd1.zip
concretizer: swap priority of selecting provider and default variant (#20182)
refers #20040 Before this PR optimization rules would have selected default providers at a higher priority than default variants. Here we swap this priority and we consider variants that are forced by any means (root spec or spec in depends_on clause) the same as if they were with a default value. This prevents the solver from avoiding expected configurations just because they contain directives like: depends_on('pkg+foo') and `+foo` is not the default variant value for pkg.
-rw-r--r--lib/spack/spack/solver/concretize.lp29
-rw-r--r--lib/spack/spack/test/concretize.py11
-rw-r--r--var/spack/repos/builtin.mock/packages/conditional-constrained-dependencies/package.py16
-rw-r--r--var/spack/repos/builtin.mock/packages/dep-with-variants/package.py15
-rw-r--r--var/spack/repos/builtin.mock/packages/ecp-viz-sdk/package.py14
5 files changed, 72 insertions, 13 deletions
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index d012bc3cf3..d8b7c125be 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -194,6 +194,7 @@ variant_value(Package, Variant, Value)
variant_not_default(Package, Variant, Value, 1)
:- variant_value(Package, Variant, Value),
not variant_default_value(Package, Variant, Value),
+ not variant_set(Package, Variant, Value),
node(Package).
variant_not_default(Package, Variant, Value, 0)
@@ -201,6 +202,12 @@ variant_not_default(Package, Variant, Value, 0)
variant_default_value(Package, Variant, Value),
node(Package).
+variant_not_default(Package, Variant, Value, 0)
+ :- variant_value(Package, Variant, Value),
+ variant_set(Package, Variant, Value),
+ node(Package).
+
+
% The default value for a variant in a package is what is written
% in the package.py file, unless some preference is set in packages.yaml
variant_default_value(Package, Variant, Value)
@@ -508,28 +515,24 @@ root(Dependency, 1) :- not root(Dependency), node(Dependency).
: provider_weight(Provider, Weight), root(Provider)
}.
-% Next, we want to minimize the weights of the providers
-% i.e. use as much as possible the most preferred providers
-#minimize{
- Weight@11,Provider
- : provider_weight(Provider, Weight), not root(Provider)
-}.
-
% For external packages it's more important than for others
% to match the compiler with their parent node
#maximize{
- Weight@10,Package
+ Weight@12,Package
: compiler_version_match(Package, Weight), external(Package)
}.
-% Then try to use as much as possible:
-% 1. Default variants
-% 2. Latest versions
-% of all the other nodes in the DAG
+% Try to use default variants or variants that have been set
#minimize {
- Weight@9,Package,Variant,Value
+ Weight@11,Package,Variant,Value
: variant_not_default(Package, Variant, Value, Weight), not root(Package)
}.
+% Minimize the weights of the providers, i.e. use as much as
+% possible the most preferred providers
+#minimize{
+ Weight@9,Provider
+ : provider_weight(Provider, Weight), not root(Provider)
+}.
% If the value is a multivalued variant there could be multiple
% values set as default. Since a default value has a weight of 0 we
% need to maximize their number below to ensure they're all set
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 39799fc1bc..fa08eb8097 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -943,3 +943,14 @@ class TestConcretize(object):
def test_target_ranges_in_conflicts(self):
with pytest.raises(spack.error.SpackError):
Spec('impossible-concretization').concretized()
+
+ @pytest.mark.regression('20040')
+ def test_variant_not_default(self):
+ s = Spec('ecp-viz-sdk').concretized()
+
+ # Check default variant value for the package
+ assert '+dep' in s['conditional-constrained-dependencies']
+
+ # Check that non-default variant values are forced on the dependency
+ d = s['dep-with-variants']
+ assert '+foo+bar+baz' in d
diff --git a/var/spack/repos/builtin.mock/packages/conditional-constrained-dependencies/package.py b/var/spack/repos/builtin.mock/packages/conditional-constrained-dependencies/package.py
new file mode 100644
index 0000000000..68fee3e9c7
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/conditional-constrained-dependencies/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 ConditionalConstrainedDependencies(Package):
+ """Package that has a variant which adds a dependency forced to
+ use non default values.
+ """
+ homepage = "https://dev.null"
+
+ version('1.0')
+
+ # This variant is on by default and attaches a dependency
+ # with a lot of variants set at their non-default values
+ variant('dep', default=True, description='nope')
+ depends_on('dep-with-variants+foo+bar+baz', when='+dep')
diff --git a/var/spack/repos/builtin.mock/packages/dep-with-variants/package.py b/var/spack/repos/builtin.mock/packages/dep-with-variants/package.py
new file mode 100644
index 0000000000..d1b08cd5df
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/dep-with-variants/package.py
@@ -0,0 +1,15 @@
+# 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 DepWithVariants(Package):
+ """Package that has a variant which adds a dependency forced to
+ use non default values.
+ """
+ homepage = "https://dev.null"
+
+ version('1.0')
+
+ variant('foo', default=False, description='nope')
+ variant('bar', default=False, description='nope')
+ variant('baz', default=False, description='nope')
diff --git a/var/spack/repos/builtin.mock/packages/ecp-viz-sdk/package.py b/var/spack/repos/builtin.mock/packages/ecp-viz-sdk/package.py
new file mode 100644
index 0000000000..76e2718c6f
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/ecp-viz-sdk/package.py
@@ -0,0 +1,14 @@
+# 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 EcpVizSdk(Package):
+ """Package that has a dependency with a variant which
+ adds a transitive dependency forced to use non default
+ values.
+ """
+ homepage = "https://dev.null"
+
+ version('1.0')
+
+ depends_on('conditional-constrained-dependencies')