From b23a829c4c27329e58770f6954dab54b0d10ec86 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Tue, 16 Jan 2024 11:50:33 +0100
Subject: [PATCH] Fix a bug when a required provider is requested for multiple
 virtuals (#42088)

---
 lib/spack/spack/solver/asp.py                 |  7 +++---
 lib/spack/spack/solver/concretize.lp          | 12 ++++++++++
 .../spack/test/concretize_requirements.py     | 23 +++++++++++++++++++
 .../packages/dla-future/package.py            | 21 +++++++++++++++++
 4 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 var/spack/repos/builtin.mock/packages/dla-future/package.py

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 2c1d45f9e3..96564d5898 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1676,9 +1676,10 @@ def provider_requirements(self):
             rules = self._rules_from_requirements(
                 virtual_str, requirements, kind=RequirementKind.VIRTUAL
             )
-            self.emit_facts_from_requirement_rules(rules)
-            self.trigger_rules()
-            self.effect_rules()
+            if rules:
+                self.emit_facts_from_requirement_rules(rules)
+                self.trigger_rules()
+                self.effect_rules()
 
     def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
         """Generate facts to enforce requirements.
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 686ca07619..3b5797997c 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -698,6 +698,18 @@ requirement_group_satisfied(node(ID, Package), X) :-
   activate_requirement(node(ID, Package), X),
   requirement_group(Package, X).
 
+% When we have a required provider, we need to ensure that the provider/2 facts respect
+% the requirement. This is particularly important for packages that could provide multiple
+% virtuals independently
+required_provider(Provider, Virtual)
+  :- requirement_group_member(ConditionID, Virtual, RequirementID),
+     condition_holds(ConditionID, _),
+     virtual(Virtual),
+     pkg_fact(Virtual, condition_effect(ConditionID, EffectID)),
+     imposed_constraint(EffectID, "node", Provider).
+
+:- provider(node(Y, Package), node(X, Virtual)), required_provider(Provider, Virtual), Package != Provider.
+
 % TODO: the following two choice rules allow the solver to add compiler
 % flags if their only source is from a requirement. This is overly-specific
 % and should use a more-generic approach like in https://github.com/spack/spack/pull/37180
diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py
index d5295691ce..0158c7602b 100644
--- a/lib/spack/spack/test/concretize_requirements.py
+++ b/lib/spack/spack/test/concretize_requirements.py
@@ -896,3 +896,26 @@ def test_requires_directive(concretize_scope, mock_packages):
     # This package can only be compiled with clang
     with pytest.raises(spack.error.SpackError, match="can only be compiled with Clang"):
         Spec("requires_clang").concretized()
+
+
+@pytest.mark.regression("42084")
+def test_requiring_package_on_multiple_virtuals(concretize_scope, mock_packages):
+    update_packages_config(
+        """
+    packages:
+      all:
+        providers:
+          scalapack: [netlib-scalapack]
+      blas:
+        require: intel-parallel-studio
+      lapack:
+        require: intel-parallel-studio
+      scalapack:
+        require: intel-parallel-studio
+    """
+    )
+    s = Spec("dla-future").concretized()
+
+    assert s["blas"].name == "intel-parallel-studio"
+    assert s["lapack"].name == "intel-parallel-studio"
+    assert s["scalapack"].name == "intel-parallel-studio"
diff --git a/var/spack/repos/builtin.mock/packages/dla-future/package.py b/var/spack/repos/builtin.mock/packages/dla-future/package.py
new file mode 100644
index 0000000000..03a22544de
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/dla-future/package.py
@@ -0,0 +1,21 @@
+# Copyright 2013-2024 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 DlaFuture(Package):
+    """A package that depends on 3 different virtuals, that might or might not be provided
+    by the same node.
+    """
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/dla-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    depends_on("blas")
+    depends_on("lapack")
+    depends_on("scalapack")