From 7fd777c3d93d7120c4d2ea3c13c1cd7e083181d2 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Tue, 1 Dec 2020 07:45:48 +0100
Subject: 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.
---
 lib/spack/spack/solver/concretize.lp | 29 ++++++++++++++++-------------
 lib/spack/spack/test/concretize.py   | 11 +++++++++++
 2 files changed, 27 insertions(+), 13 deletions(-)

(limited to 'lib')

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
-- 
cgit v1.2.3-70-g09d2