From 9ae1317e795b42bdd8edce1a2d37a284ac967ea3 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 22 Aug 2023 21:07:51 +0200 Subject: ASP-based solver: use edge properties for reused specs (#39508) Since #34821 we are annotating virtual dependencies on DAG edges, and reconstructing virtuals in memory when we read a concrete spec from previous formats. Therefore, we can remove a TODO in asp.py, and rely on "virtual_on_edge" facts to be imposed. --- lib/spack/spack/solver/asp.py | 36 +++++------------------------------- lib/spack/spack/solver/concretize.lp | 13 +++++++++++++ lib/spack/spack/test/concretize.py | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 7710641cb5..8ce04cdc5b 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1827,37 +1827,11 @@ class SpackSolverSetup: # skip build dependencies of already-installed specs if concrete_build_deps or dtype != "build": clauses.append(fn.attr("depends_on", spec.name, dep.name, dtype)) - - # TODO: We have to look up info from package.py here, but we'd - # TODO: like to avoid this entirely. We should not need to look - # TODO: up potentially wrong info if we have virtual edge info. - try: - try: - pkg = dep.package - - except spack.repo.UnknownNamespaceError: - # Try to look up the package of the same name and use its - # providers. This is as good as we can do without edge info. - pkg_class = spack.repo.PATH.get_pkg_class(dep.name) - spec = spack.spec.Spec(f"{dep.name}@{dep.version}") - pkg = pkg_class(spec) - - virtuals = pkg.virtuals_provided - - except spack.repo.UnknownPackageError: - # Skip virtual node constriants for renamed/deleted packages, - # so their binaries can still be installed. - # NOTE: with current specs (which lack edge attributes) this - # can allow concretizations with two providers, but it's unlikely. - continue - - # Don't concretize with two providers of the same virtual. - # See above for exception for unknown packages. - # TODO: we will eventually record provider information on edges, - # TODO: which avoids the need for the package lookup above. - for virtual in virtuals: - clauses.append(fn.attr("virtual_node", virtual.name)) - clauses.append(fn.provider(dep.name, virtual.name)) + for virtual_name in dspec.virtuals: + clauses.append( + fn.attr("virtual_on_edge", spec.name, dep.name, virtual_name) + ) + clauses.append(fn.attr("virtual_node", virtual_name)) # imposing hash constraints for all but pure build deps of # already-installed concrete specs. diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 90b86cd160..9d1b48b379 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -113,6 +113,7 @@ unification_set(SetID, VirtualNode) % Node attributes that have multiple node arguments (usually, only the first argument is a node) multiple_nodes_attribute("node_flag_source"). multiple_nodes_attribute("depends_on"). +multiple_nodes_attribute("virtual_on_edge"). % Map constraint on the literal ID to facts on the node attr(Name, node(min_dupe_id, A1)) :- literal(LiteralID, Name, A1), solve_literal(LiteralID). @@ -367,6 +368,18 @@ attr("node_flag_source", node(X, A1), A2, node(Y, A3)) :- impose(ID, node(X, A1)), imposed_constraint(ID, "depends_on", A1, A2, A3). +% Reconstruct virtual dependencies for reused specs +attr("virtual_on_edge", node(X, A1), node(Y, A2), Virtual) + :- impose(ID, node(X, A1)), + depends_on(node(X, A1), node(Y, A2)), + imposed_constraint(ID, "virtual_on_edge", A1, A2, Virtual), + not build(node(X, A1)). + +virtual_condition_holds(node(Y, A2), Virtual) + :- impose(ID, node(X, A1)), + attr("virtual_on_edge", node(X, A1), node(Y, A2), Virtual), + not build(node(X, A1)). + % we cannot have additional variant values when we are working with concrete specs :- attr("node", node(ID, Package)), attr("hash", node(ID, Package), Hash), diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index ba84619d04..4cd22249ca 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -2099,6 +2099,21 @@ class TestConcretize: edges = spec.edges_to_dependencies(name="callpath") assert len(edges) == 1 and edges[0].virtuals == () + @pytest.mark.only_clingo("Use case not supported by the original concretizer") + @pytest.mark.db + @pytest.mark.parametrize( + "spec_str,mpi_name", + [("mpileaks", "mpich"), ("mpileaks ^mpich2", "mpich2"), ("mpileaks ^zmpi", "zmpi")], + ) + def test_virtuals_are_reconstructed_on_reuse(self, spec_str, mpi_name, database): + """Tests that when we reuse a spec, virtual on edges are reconstructed correctly""" + with spack.config.override("concretizer:reuse", True): + spec = Spec(spec_str).concretized() + assert spec.installed + mpi_edges = spec.edges_to_dependencies(mpi_name) + assert len(mpi_edges) == 1 + assert "mpi" in mpi_edges[0].virtuals + @pytest.fixture() def duplicates_test_repository(): -- cgit v1.2.3-60-g2f50