tests for conditional deps in requirements

Signed-off-by: Gregory Becker <becker33@llnl.gov>

lib/spack/spack/spec.py

@@ -3442,11 +3442,8 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
         mock_nodes_from_old_specfiles = set()
         for rhs_edge in other.traverse_edges(root=False, cover="edges"):
             # Skip checking any conditional edge that is not satisfied
-            if (
+            if rhs_edge.when != Spec() and not self.satisfies(rhs_edge.when):
-                rhs_edge.when != Spec()
+                # TODO: this misses the case that the rhs statically satisfies its own condition
-                and not rhs_edge.parent.satisfies(rhs_edge.when)
-                and not self.satisfies(rhs_edge.when)
-            ):
                 continue
 
             # If we are checking for ^mpi we need to verify if there is any edge
@@ -3506,6 +3503,7 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
                 for lhs_edge in self.traverse_edges(
                     root=False, cover="edges", deptype=("link", "run")
                 ):
+                    # TODO: do we need to avoid conditional edges here
                     lhs_edges[lhs_edge.spec.name].add(lhs_edge)
                     for virtual_name in lhs_edge.virtuals:
                         lhs_edges[virtual_name].add(lhs_edge)
@@ -3522,6 +3520,7 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
                 return False
 
             for virtual in rhs_edge.virtuals:
+                # TODO: consider how this could apply to conditional edges
                 has_virtual = any(
                     virtual in edge.virtuals for edge in lhs_edges[current_dependency_name]
                 )
@@ -3533,19 +3532,11 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
         for rhs in other.traverse(root=False):
             # Possible lhs nodes to match this rhs node
             lhss = [lhs for lhs in lhs_nodes if lhs.satisfies(rhs, deps=False)]
-            lhs_parents = [
-                lhs
-                for lhs in lhs_nodes
-                if any(lhs.satisfies(parent, deps=False) for parent in rhs.dependents())
-            ]
 
             # Check whether the node needs matching (not a conditional that isn't satisfied)
-            in_edges = [
+            if not any(self.satisfies(e.when) for e in rhs.edges_from_dependents()):
-                e
+                # TODO: This technically misses the case that the edge is analogous
-                for e in rhs.edges_from_dependents()
+                # to an edge lower in the DAG, and could give a false negative in that case
-                if e.parent.satisfies(e.when) or any(lhp.satisfies(e.when) for lhp in lhs_parents)
-            ]
-            if not in_edges:
                 continue
 
             # If there is no matching lhs for this rhs node

lib/spack/spack/test/concretization/requirements.py

@@ -1301,3 +1301,47 @@ def test_requirements_on_compilers_and_reuse(
     assert is_pkgb_reused == expected_reuse
     for c in expected_contraints:
         assert pkga.satisfies(c)
+
+
+@pytest.mark.parametrize(
+    "abstract,req_is_noop",
+    [
+        ("hdf5+mpi", False),
+        ("hdf5~mpi", True),
+        ("conditional-languages+c", False),
+        ("conditional-languages+cxx", False),
+        ("conditional-languages+fortran", False),
+        ("conditional-languages~c~cxx~fortran", True),
+    ],
+)
+def test_requirements_conditional_deps(abstract, req_is_noop, mutable_config, mock_packages):
+    required_spec = "%[when='^c' virtuals=c]gcc@10.3.1 %[when='^cxx' virtuals=cxx]gcc@10.3.1 %[when='^fortran' virtuals=fortran]gcc@10.3.1 ^[when='^mpi' virtuals=mpi]zmpi"
+    abstract = spack.spec.Spec(abstract)
+
+    # Configure two gcc compilers that could be concretized to
+    # We will confirm concretization matches the less preferred one
+    extra_attributes_block = {
+        "compilers": {"c": "/path/to/gcc", "cxx": "/path/to/g++", "fortran": "/path/to/fortran"}
+    }
+    spack.config.CONFIG.set(
+        "packages:gcc:externals::",
+        [
+            {
+                "spec": "gcc@12.3.1 languages=c,c++,fortran",
+                "prefix": "/path",
+                "extra_attributes": extra_attributes_block,
+            },
+            {
+                "spec": "gcc@10.3.1 languages=c,c++,fortran",
+                "prefix": "/path",
+                "extra_attributes": extra_attributes_block,
+            },
+        ],
+    )
+
+    no_requirements = spack.concretize.concretize_one(abstract)
+    spack.config.CONFIG.set(f"packages:{abstract.name}", {"require": required_spec})
+    requirements = spack.concretize.concretize_one(abstract)
+
+    assert requirements.satisfies(required_spec)
+    assert (requirements == no_requirements) == req_is_noop  # show the reqs change concretization