diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index b764775572d..a0d2f67a127 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -37,6 +37,7 @@ import spack.package_prefs import spack.platforms import spack.repo +import spack.solver.splicing import spack.spec import spack.store import spack.util.crypto @@ -1740,7 +1741,7 @@ def package_splice_rules(self, pkg): if any( v in cond.variants or v in spec_to_splice.variants for v in match_variants ): - raise Exception( + raise spack.error.PackageError( "Overlap between match_variants and explicitly set variants" ) variant_constraints = self._gen_match_variant_splice_constraints( @@ -3374,14 +3375,6 @@ def consume_facts(self): self._setup.effect_rules() -# This should be a dataclass, but dataclasses don't work on Python 3.6 -class Splice: - def __init__(self, splice_node: NodeArgument, child_name: str, child_hash: str): - self.splice_node = splice_node - self.child_name = child_name - self.child_hash = child_hash - - class SpecBuilder: """Class with actions to rebuild a spec from ASP results.""" @@ -3421,7 +3414,7 @@ def __init__(self, specs, hash_lookup=None): self._specs: Dict[NodeArgument, spack.spec.Spec] = {} # Matches parent nodes to splice node - self._splices: Dict[NodeArgument, List[Splice]] = {} + self._splices: Dict[spack.spec.Spec, List[spack.solver.splicing.Splice]] = {} self._result = None self._command_line_specs = specs self._flag_sources: Dict[Tuple[NodeArgument, str], Set[str]] = collections.defaultdict( @@ -3628,50 +3621,12 @@ def splice_at_hash( child_name: str, child_hash: str, ): - splice = Splice(splice_node, child_name=child_name, child_hash=child_hash) - self._splices.setdefault(parent_node, []).append(splice) - - def _resolve_automatic_splices(self): - """After all of the specs have been concretized, apply all immediate splices. - - Use reverse topological order to ensure that all dependencies are resolved - before their parents, allowing for maximal sharing and minimal copying. - - """ - fixed_specs = {} - - # create a mapping from dag hash to an integer representing position in reverse topo order. - specs = self._specs.values() - topo_order = list(traverse.traverse_nodes(specs, order="topo", key=traverse.by_dag_hash)) - topo_lookup = {spec.dag_hash(): index for index, spec in enumerate(reversed(topo_order))} - - # iterate over specs, children before parents - for node, spec in sorted(self._specs.items(), key=lambda x: topo_lookup[x[1].dag_hash()]): - immediate = self._splices.get(node, []) - if not immediate and not any( - edge.spec in fixed_specs for edge in spec.edges_to_dependencies() - ): - continue - new_spec = spec.copy(deps=False) - new_spec.clear_caches(ignore=("package_hash",)) - new_spec.build_spec = spec - for edge in spec.edges_to_dependencies(): - depflag = edge.depflag & ~dt.BUILD - if any(edge.spec.dag_hash() == splice.child_hash for splice in immediate): - splice = [s for s in immediate if s.child_hash == edge.spec.dag_hash()][0] - new_spec.add_dependency_edge( - self._specs[splice.splice_node], depflag=depflag, virtuals=edge.virtuals - ) - elif edge.spec in fixed_specs: - new_spec.add_dependency_edge( - fixed_specs[edge.spec], depflag=depflag, virtuals=edge.virtuals - ) - else: - new_spec.add_dependency_edge( - edge.spec, depflag=depflag, virtuals=edge.virtuals - ) - self._specs[node] = new_spec - fixed_specs[spec] = new_spec + parent_spec = self._specs[parent_node] + splice_spec = self._specs[splice_node] + splice = spack.solver.splicing.Splice( + splice_spec, child_name=child_name, child_hash=child_hash + ) + self._splices.setdefault(parent_spec, []).append(splice) @staticmethod def sort_fn(function_tuple) -> Tuple[int, int]: @@ -3764,7 +3719,15 @@ def build_specs(self, function_tuples): for root in roots.values(): root._finalize_concretization() - self._resolve_automatic_splices() + # Only attempt to resolve automatic splices if the solver produced any + if self._splices: + resolved_splices = spack.solver.splicing._resolve_collected_splices( + list(self._specs.values()), self._splices + ) + new_specs = {} + for node, spec in self._specs.items(): + new_specs[node] = resolved_splices.get(spec, spec) + self._specs = new_specs for s in self._specs.values(): spack.spec.Spec.ensure_no_deprecated(s) diff --git a/lib/spack/spack/solver/splicing.py b/lib/spack/spack/solver/splicing.py new file mode 100644 index 00000000000..b29bca1b6ec --- /dev/null +++ b/lib/spack/spack/solver/splicing.py @@ -0,0 +1,73 @@ +# Copyright Spack Project Developers. See COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) +from functools import cmp_to_key +from typing import Dict, List, NamedTuple + +import spack.deptypes as dt +from spack.spec import Spec +from spack.traverse import by_dag_hash, traverse_nodes + + +class Splice(NamedTuple): + #: The spec being spliced into a parent + splice_spec: Spec + #: The name of the child that splice spec is replacing + child_name: str + #: The hash of the child that `splice_spec` is replacing + child_hash: str + + +def _resolve_collected_splices( + specs: List[Spec], splices: Dict[Spec, List[Splice]] +) -> Dict[Spec, Spec]: + """After all of the specs have been concretized, apply all immediate splices. + Returns a dict mapping original specs to their resolved counterparts + """ + + def splice_cmp(s1: Spec, s2: Spec): + """This function can be used to sort a list of specs such that that any + spec which will be spliced into a parent comes after the parent it will + be spliced into. This order ensures that transitive splices will be + executed in the correct order. + """ + + s1_splices = splices.get(s1, []) + s2_splices = splices.get(s2, []) + if any([s2.dag_hash() == splice.splice_spec.dag_hash() for splice in s1_splices]): + return -1 + elif any([s1.dag_hash() == splice.splice_spec.dag_hash() for splice in s2_splices]): + return 1 + else: + return 0 + + splice_order = sorted(specs, key=cmp_to_key(splice_cmp)) + reverse_topo_order = reversed( + [x for x in traverse_nodes(splice_order, order="topo", key=by_dag_hash) if x in specs] + ) + + already_resolved: Dict[Spec, Spec] = {} + for spec in reverse_topo_order: + immediate = splices.get(spec, []) + if not immediate and not any( + edge.spec in already_resolved for edge in spec.edges_to_dependencies() + ): + continue + new_spec = spec.copy(deps=False) + new_spec.clear_caches(ignore=("package_hash",)) + new_spec.build_spec = spec + for edge in spec.edges_to_dependencies(): + depflag = edge.depflag & ~dt.BUILD + if any(edge.spec.dag_hash() == splice.child_hash for splice in immediate): + splice = [s for s in immediate if s.child_hash == edge.spec.dag_hash()][0] + # If the spec being splice in is also spliced + splice_spec = already_resolved.get(splice.splice_spec, splice.splice_spec) + new_spec.add_dependency_edge(splice_spec, depflag=depflag, virtuals=edge.virtuals) + elif edge.spec in already_resolved: + new_spec.add_dependency_edge( + already_resolved[edge.spec], depflag=depflag, virtuals=edge.virtuals + ) + else: + new_spec.add_dependency_edge(edge.spec, depflag=depflag, virtuals=edge.virtuals) + already_resolved[spec] = new_spec + return already_resolved diff --git a/lib/spack/spack/test/abi_splicing.py b/lib/spack/spack/test/abi_splicing.py index a1d2210a889..2e47fdc2224 100644 --- a/lib/spack/spack/test/abi_splicing.py +++ b/lib/spack/spack/test/abi_splicing.py @@ -103,7 +103,6 @@ def test_splice_build_splice_node(install_specs, mutable_config): assert concrete["splice-h"].build_spec.dag_hash() == concrete["splice-h"].dag_hash() -@pytest.mark.xfail(reason="the spliced splice-h has sometimes the original splice-h hash") def test_double_splice(install_specs, mutable_config): """Tests splicing two dependencies of an installed spec, for other installed specs""" splice_t, splice_h, splice_z = install_specs(