ASP-based solver: fix issue with conditional requirements and trigger conditions (#42566)

The lack of a rule to avoid enforcing requirements on multi-valued variants, when the condition activating the environment was not met, resulted in multiple optimal solutions. The fix is to prevent imposing a requirement if the when= rule activating it is not met.

4 files changed, 92 insertions, 0 deletions

diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index c8d96eed9c..7f06d6f3a9 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -698,6 +698,14 @@ requirement_group_satisfied(node(ID, Package), X) :-
   activate_requirement(node(ID, Package), X),
   requirement_group(Package, X).
 
+% Do not impose requirements, if the conditional requirement is not active
+do_not_impose(EffectID, node(ID, Package)) :-
+  trigger_condition_holds(TriggerID, node(ID, Package)),
+  pkg_fact(Package, condition_trigger(ConditionID, TriggerID)),
+  pkg_fact(Package, condition_effect(ConditionID, EffectID)),
+  requirement_group_member(ConditionID , Package, RequirementID),
+  not activate_requirement(node(ID, Package), RequirementID).
+
 % 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
diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py
index 3ef143fd36..ea1dc526df 100644
--- a/lib/spack/spack/test/concretize_requirements.py
+++ b/lib/spack/spack/test/concretize_requirements.py
@@ -1133,3 +1133,46 @@ def test_conflict_packages_yaml(packages_yaml, spec_str, concretize_scope, mock_
     update_packages_config(packages_yaml)
     with pytest.raises(UnsatisfiableSpecError):
         Spec(spec_str).concretized()
+
+
+@pytest.mark.parametrize(
+    "spec_str,expected,not_expected",
+    [
+        (
+            "forward-multi-value +cuda cuda_arch=10 ^dependency-mv~cuda",
+            ["cuda_arch=10", "^dependency-mv~cuda"],
+            ["cuda_arch=11", "^dependency-mv cuda_arch=10", "^dependency-mv cuda_arch=11"],
+        ),
+        (
+            "forward-multi-value +cuda cuda_arch=10 ^dependency-mv+cuda",
+            ["cuda_arch=10", "^dependency-mv cuda_arch=10"],
+            ["cuda_arch=11", "^dependency-mv cuda_arch=11"],
+        ),
+        (
+            "forward-multi-value +cuda cuda_arch=11 ^dependency-mv+cuda",
+            ["cuda_arch=11", "^dependency-mv cuda_arch=11"],
+            ["cuda_arch=10", "^dependency-mv cuda_arch=10"],
+        ),
+        (
+            "forward-multi-value +cuda cuda_arch=10,11 ^dependency-mv+cuda",
+            ["cuda_arch=10,11", "^dependency-mv cuda_arch=10,11"],
+            [],
+        ),
+    ],
+)
+def test_forward_multi_valued_variant_using_requires(
+    spec_str, expected, not_expected, config, mock_packages
+):
+    """Tests that a package can forward multivalue variants to dependencies, using
+    `requires` directives of the form:
+
+        for _val in ("shared", "static"):
+            requires(f"^some-virtual-mv libs={_val}", when=f"libs={_val} ^some-virtual-mv")
+    """
+    s = Spec(spec_str).concretized()
+
+    for constraint in expected:
+        assert s.satisfies(constraint)
+
+    for constraint in not_expected:
+        assert not s.satisfies(constraint)
diff --git a/var/spack/repos/builtin.mock/packages/dependency-mv/package.py b/var/spack/repos/builtin.mock/packages/dependency-mv/package.py
new file mode 100644
index 0000000000..ed2cc5408a
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/dependency-mv/package.py
@@ -0,0 +1,18 @@
+# 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 DependencyMv(Package):
+    """Package providing a virtual dependency and with a multivalued variant."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/foo-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    variant("cuda", default=False, description="Build with CUDA")
+    variant("cuda_arch", values=any_combination_of("10", "11"), when="+cuda")
diff --git a/var/spack/repos/builtin.mock/packages/forward-multi-value/package.py b/var/spack/repos/builtin.mock/packages/forward-multi-value/package.py
new file mode 100644
index 0000000000..1bdd696c4e
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/forward-multi-value/package.py
@@ -0,0 +1,23 @@
+# 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 ForwardMultiValue(Package):
+    """A package that forwards the value of a multi-valued variant to a dependency"""
+
+    homepage = "http://www.llnl.gov"
+    url = "http://www.llnl.gov/mpileaks-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    variant("cuda", default=False, description="Build with CUDA")
+    variant("cuda_arch", values=any_combination_of("10", "11"), when="+cuda")
+
+    depends_on("dependency-mv")
+
+    requires("^dependency-mv cuda_arch=10", when="+cuda cuda_arch=10 ^dependency-mv+cuda")
+    requires("^dependency-mv cuda_arch=11", when="+cuda cuda_arch=11 ^dependency-mv+cuda")