From 9bb5cffc73a555342665527f385aec1acdd11478 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 15 Aug 2023 11:24:09 +0200 Subject: [PATCH] Change semantic for providers If a possible provider is not used to satisfy a vdep, then it's not a provider of that vdep. --- lib/spack/spack/solver/concretize.lp | 36 +++++++++++++------ lib/spack/spack/test/env.py | 32 +++++++++++++++++ .../stacks/e4s-oneapi/spack.yaml | 2 +- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index bc24514115..3d35eea3ff 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -455,11 +455,16 @@ error(1, Msg) % if a package depends on a virtual, it's not external and we have a % provider for that virtual then it depends on the provider -1 { attr("depends_on", PackageNode, ProviderNode, Type) : provider(ProviderNode, node(VirtualID, Virtual)) } 1 +node_depends_on_virtual(PackageNode, Virtual, Type) :- dependency_holds(PackageNode, Virtual, Type), virtual(Virtual), not external(PackageNode). +node_depends_on_virtual(PackageNode, Virtual) :- node_depends_on_virtual(PackageNode, Virtual, Type). + +1 { attr("depends_on", PackageNode, ProviderNode, Type) : provider(ProviderNode, node(VirtualID, Virtual)) } 1 + :- node_depends_on_virtual(PackageNode, Virtual, Type). + attr("virtual_on_edge", PackageNode, ProviderNode, Virtual) :- dependency_holds(PackageNode, Virtual, Type), attr("depends_on", PackageNode, ProviderNode, Type), @@ -468,9 +473,7 @@ attr("virtual_on_edge", PackageNode, ProviderNode, Virtual) % dependencies on virtuals also imply that the virtual is a virtual node 1 { attr("virtual_node", node(0..X-1, Virtual)) : max_dupes(Virtual, X) } - :- dependency_holds(PackageNode, Virtual, Type), - virtual(Virtual), - not external(PackageNode). + :- node_depends_on_virtual(PackageNode, Virtual). % If there's a virtual node, we must select one and only one provider. % The provider must be selected among the possible providers. @@ -490,13 +493,24 @@ attr("virtual_node", VirtualNode) :- attr("virtual_root", VirtualNode). % then the provider is the root package. attr("root", PackageNode) :- attr("virtual_root", VirtualNode), provider(PackageNode, VirtualNode). -% If we asked for a root package and that root provides a virtual, -% the root is a provider for that virtual. This rule is mostly relevant -% for environments that are concretized together (e.g. where we -% asks to install "mpich" and "hdf5+mpi" and we want "mpich" to -% be the mpi provider) -1 { provider(PackageNode, node(0..X-1, Virtual)) : max_dupes(Virtual, X) } 1 :- attr("node", PackageNode), virtual_condition_holds(PackageNode, Virtual). -:- 2 { provider(PackageNode, VirtualNode) }, attr("virtual_node", VirtualNode). +% The provider is selected among the nodes for which the virtual condition holds +1 { provider(PackageNode, node(X, Virtual)) : + attr("node", PackageNode), virtual_condition_holds(PackageNode, Virtual) } 1 + :- attr("virtual_node", node(X, Virtual)). + +% If a spec is selected as a provider, it is for all the virtual it could provide +:- provider(PackageNode, node(X, Virtual1)), + virtual_condition_holds(PackageNode, Virtual2), + Virtual2 != Virtual1, + unification_set(SetID, PackageNode), + unification_set(SetID, node(X, Virtual2)), + not provider(PackageNode, node(X, Virtual2)). + +% If a spec is a dependency, and could provide a needed virtual, it must be a provider +:- node_depends_on_virtual(PackageNode, Virtual), + depends_on(PackageNode, PossibleProviderNode), + virtual_condition_holds(PossibleProviderNode, Virtual), + not attr("virtual_on_edge", PackageNode, PossibleProviderNode, Virtual). % The provider provides the virtual if some provider condition holds. virtual_condition_holds(node(ProviderID, Provider), Virtual) :- virtual_condition_holds(ID, node(ProviderID, Provider), Virtual). diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 203fa72d70..7c26fcf6d7 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -588,3 +588,35 @@ def test_conflicts_with_packages_that_are_not_dependencies( else: e.concretize() assert any(s.satisfies(expected_spec) for s in e.concrete_roots()) + + +def test_requires_on_virtual_and_potential_providers(tmp_path, mock_packages, config): + """Tests that in an environment we can add packages explicitly, even though they provide + a virtual package, and we require the provider of the same virtual to be another package, + if they are added explicitly by their name. + """ + if spack.config.get("config:concretizer") == "original": + pytest.xfail("Known failure of the original concretizer") + + manifest = tmp_path / "spack.yaml" + manifest.write_text( + """\ + spack: + specs: + - mpich + - mpich2 + - mpileaks + packages: + mpi: + require: mpich2 + """ + ) + with ev.Environment(manifest.parent) as e: + e.concretize() + assert e.matching_spec("mpich") + assert e.matching_spec("mpich2") + + mpileaks = e.matching_spec("mpileaks") + assert mpileaks.satisfies("^mpich2") + assert mpileaks["mpi"].satisfies("mpich2") + assert not mpileaks.satisfies("^mpich") diff --git a/share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml index 5af50df68b..715c182729 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml @@ -148,7 +148,7 @@ spack: - nco - netlib-scalapack - omega-h - # - openmpi + - openmpi - openpmd-api - papi - papyrus