From 983fb11dee5801ed74fdbcb78493c0d31e3872cb Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Wed, 25 Nov 2020 22:03:42 +0100
Subject: concretizer: treat conditional providers correctly (#20086)

refers #20040

This modification emits rules like:

provides_virtual("netlib-lapack","blas") :- variant_value("netlib-lapack","external-blas","False").

for packages that provide virtual dependencies conditionally instead
of a fact that doesn't account for the condition.
---
 lib/spack/spack/solver/asp.py                          |  7 ++++++-
 lib/spack/spack/test/concretize.py                     | 12 ++++++++++++
 .../packages/conditional-provider/package.py           | 18 ++++++++++++++++++
 .../repos/builtin.mock/packages/v1-provider/package.py | 13 +++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 var/spack/repos/builtin.mock/packages/conditional-provider/package.py
 create mode 100644 var/spack/repos/builtin.mock/packages/v1-provider/package.py

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 9c48a77b28..5ef362a22d 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1322,13 +1322,18 @@ class SpackSolverSetup(object):
             self.gen.fact(fn.virtual(vspec))
             all_providers = sorted(spack.repo.path.providers_for(vspec))
             for idx, provider in enumerate(all_providers):
-                self.gen.fact(fn.provides_virtual(provider.name, vspec))
+                provides_atom = fn.provides_virtual(provider.name, vspec)
                 possible_provider_fn = fn.possible_provider(
                     vspec, provider.name, idx
                 )
                 item = (idx, provider, possible_provider_fn)
                 self.providers_by_vspec_name[vspec].append(item)
                 clauses = self.spec_clauses(provider, body=True)
+                clauses_but_node = [c for c in clauses if c.name != 'node']
+                if clauses_but_node:
+                    self.gen.rule(provides_atom, AspAnd(*clauses_but_node))
+                else:
+                    self.gen.fact(provides_atom)
                 for clause in clauses:
                     self.gen.rule(clause, possible_provider_fn)
                 self.gen.newline()
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 761641035f..b150d60426 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -883,3 +883,15 @@ class TestConcretize(object):
 
         # 'stuff' is provided by an external package, so check it's present
         assert 'externalvirtual' in s
+
+    @pytest.mark.regression('20040')
+    def test_conditional_provides_or_depends_on(self):
+        if spack.config.get('config:concretizer') == 'original':
+            pytest.xfail('Known failure of the original concretizer')
+
+        # Check that we can concretize correctly a spec that can either
+        # provide a virtual or depend on it based on the value of a variant
+        s = Spec('conditional-provider +disable-v1').concretized()
+        assert 'v1-provider' in s
+        assert s['v1'].name == 'v1-provider'
+        assert s['v2'].name == 'conditional-provider'
diff --git a/var/spack/repos/builtin.mock/packages/conditional-provider/package.py b/var/spack/repos/builtin.mock/packages/conditional-provider/package.py
new file mode 100644
index 0000000000..e95f2ad54c
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/conditional-provider/package.py
@@ -0,0 +1,18 @@
+# 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 ConditionalProvider(Package):
+    """Mimic the real netlib-lapack, that may be built on top of an
+    optimized blas.
+    """
+    homepage = "https://dev.null"
+
+    version('1.0')
+
+    variant('disable-v1', default=False, description='nope')
+
+    provides('v2')
+    provides('v1', when='~disable-v1')
+
+    depends_on('v1', when='+disable-v1')
diff --git a/var/spack/repos/builtin.mock/packages/v1-provider/package.py b/var/spack/repos/builtin.mock/packages/v1-provider/package.py
new file mode 100644
index 0000000000..46b7a99ac9
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/v1-provider/package.py
@@ -0,0 +1,13 @@
+# 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 V1Provider(Package):
+    """Mimic the real netlib-lapack, that may be built on top of an
+    optimized blas.
+    """
+    homepage = "https://dev.null"
+
+    version('1.0')
+
+    provides('v1')
-- 
cgit v1.2.3-60-g2f50