From 26c5f5265dac1d136173b281d9a7843783789925 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Fri, 16 May 2025 16:56:43 -0700 Subject: [PATCH] tests for conditional deps in requirements Signed-off-by: Gregory Becker --- lib/spack/spack/spec.py | 23 +++------- .../spack/test/concretization/requirements.py | 44 +++++++++++++++++++ 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 36ea507bd61..ffd319b5560 100644 --- a/lib/spack/spack/spec.py +++ b/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 ( - rhs_edge.when != Spec() - and not rhs_edge.parent.satisfies(rhs_edge.when) - and not self.satisfies(rhs_edge.when) - ): + if rhs_edge.when != Spec() and not self.satisfies(rhs_edge.when): + # TODO: this misses the case that the rhs statically satisfies its own condition 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 = [ - e - for e in rhs.edges_from_dependents() - if e.parent.satisfies(e.when) or any(lhp.satisfies(e.when) for lhp in lhs_parents) - ] - if not in_edges: + if not any(self.satisfies(e.when) for e in rhs.edges_from_dependents()): + # TODO: This technically misses the case that the edge is analogous + # to an edge lower in the DAG, and could give a false negative in that case continue # If there is no matching lhs for this rhs node diff --git a/lib/spack/spack/test/concretization/requirements.py b/lib/spack/spack/test/concretization/requirements.py index cae5cb760a0..13d58573c38 100644 --- a/lib/spack/spack/test/concretization/requirements.py +++ b/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