diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 32db03f5cf906be30a47102f116e36f9cf702b93..5310c4e3a84c894742628d0e5b83c20f120e6e8e 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -3839,12 +3839,21 @@ def splice_at_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 in size order. This ensures that all dependencies are resolved
+ """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 = {}
- for node, spec in sorted(self._specs.items(), key=lambda x: len(x[1])):
+
+ # 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()
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 63a5a711758120d234592cb821643925721a6232..9bb237754e1d88fab630f3fe25aca466cc041dd7 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -1455,7 +1455,7 @@ attr("node_flag", PackageNode, NodeFlag) :- attr("node_flag_set", PackageNode, N
#defined installed_hash/2.
#defined abi_splice_conditions_hold/4.
-% These are the previously concretized attributes of the installed package as
+% These are the previously concretized attributes of the installed package as
% a hash. It has the general form:
% hash_attr(Hash, Attribute, PackageName, Args*)
#defined hash_attr/3.
@@ -1479,12 +1479,12 @@ hash_attr(Hash, "node_version_satisfies", PackageName, Constraint) :-
% This recovers the exact semantics for hash reuse hash and depends_on are where
% splices are decided, and virtual_on_edge can result in name-changes, which is
-% why they are all treated separately.
+% why they are all treated separately.
imposed_constraint(Hash, Attr, PackageName) :-
hash_attr(Hash, Attr, PackageName).
imposed_constraint(Hash, Attr, PackageName, A1) :-
hash_attr(Hash, Attr, PackageName, A1), Attr != "hash".
-imposed_constraint(Hash, Attr, PackageName, Arg1, Arg2) :-
+imposed_constraint(Hash, Attr, PackageName, Arg1, Arg2) :-
hash_attr(Hash, Attr, PackageName, Arg1, Arg2),
Attr != "depends_on",
Attr != "virtual_on_edge".
@@ -1492,16 +1492,16 @@ imposed_constraint(Hash, Attr, PackageName, A1, A2, A3) :-
hash_attr(Hash, Attr, PackageName, A1, A2, A3).
imposed_constraint(Hash, "hash", PackageName, Hash) :- installed_hash(PackageName, Hash).
% Without splicing, we simply recover the exact semantics
-imposed_constraint(ParentHash, "hash", ChildName, ChildHash) :-
+imposed_constraint(ParentHash, "hash", ChildName, ChildHash) :-
hash_attr(ParentHash, "hash", ChildName, ChildHash),
ChildHash != ParentHash,
not abi_splice_conditions_hold(_, _, ChildName, ChildHash).
-
+
imposed_constraint(Hash, "depends_on", PackageName, DepName, Type) :-
hash_attr(Hash, "depends_on", PackageName, DepName, Type),
hash_attr(Hash, "hash", DepName, DepHash),
not attr("splice_at_hash", _, _, DepName, DepHash).
-
+
imposed_constraint(Hash, "virtual_on_edge", PackageName, DepName, VirtName) :-
hash_attr(Hash, "virtual_on_edge", PackageName, DepName, VirtName),
not attr("splice_at_hash", _, _, DepName,_).
@@ -1511,7 +1511,7 @@ imposed_constraint(Hash, "virtual_on_edge", PackageName, DepName, VirtName) :-
impose(Hash, PackageNode) :- attr("hash", PackageNode, Hash), attr("node", PackageNode).
-% If there is not a hash for a package, we build it.
+% If there is not a hash for a package, we build it.
build(PackageNode) :- attr("node", PackageNode), not concrete(PackageNode).
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 03a372be4e5284a2583034268eb87b396ddf8c63..926f35a70451c251265c106b3b7505f41639618f 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1431,8 +1431,6 @@ def tree(
class Spec:
#: Cache for spec's prefix, computed lazily in the corresponding property
_prefix = None
- #: Cache for spec's length, computed lazily in the corresponding property
- _length = None
abstract_hash = None
@staticmethod
@@ -3702,18 +3700,6 @@ def __getitem__(self, name: str):
return child
- def __len__(self):
- if not self.concrete:
- raise spack.error.SpecError(f"Cannot get length of abstract spec: {self}")
-
- if not self._length:
- self._length = 1 + sum(len(dep) for dep in self.dependencies())
- return self._length
-
- def __bool__(self):
- # Need to define this so __len__ isn't used by default
- return True
-
def __contains__(self, spec):
"""True if this spec or some dependency satisfies the spec.
@@ -4472,7 +4458,7 @@ def clear_caches(self, ignore=()):
if h.attr not in ignore:
if hasattr(self, h.attr):
setattr(self, h.attr, None)
- for attr in ("_dunder_hash", "_prefix", "_length"):
+ for attr in ("_dunder_hash", "_prefix"):
if attr not in ignore:
setattr(self, attr, None)
diff --git a/lib/spack/spack/test/abi_splicing.py b/lib/spack/spack/test/abi_splicing.py
index b36c285b134923e04f2c59d04364f8a224bbe864..d647647797571f11dad0f057623c4ca8a21f9a30 100644
--- a/lib/spack/spack/test/abi_splicing.py
+++ b/lib/spack/spack/test/abi_splicing.py
@@ -229,3 +229,19 @@ def test_spliced_build_deps_only_in_build_spec(splicing_setup):
assert _has_build_dependency(build_spec, "splice-z")
# Spliced build dependencies are removed
assert len(concr_goal.dependencies(None, dt.BUILD)) == 0
+
+
+def test_spliced_transitive_dependency(splicing_setup):
+ cache = ["splice-depends-on-t@1.0 ^splice-h@1.0.1"]
+ goal_spec = Spec("splice-depends-on-t^splice-h@1.0.2")
+
+ with CacheManager(cache):
+ spack.config.set("packages", _make_specs_non_buildable(["splice-depends-on-t"]))
+ _enable_splicing()
+ concr_goal = goal_spec.concretized()
+ # Spec has been spliced
+ assert concr_goal._build_spec is not None
+ assert concr_goal["splice-t"]._build_spec is not None
+ assert concr_goal.satisfies(goal_spec)
+ # Spliced build dependencies are removed
+ assert len(concr_goal.dependencies(None, dt.BUILD)) == 0
diff --git a/lib/spack/spack/test/cmd/pkg.py b/lib/spack/spack/test/cmd/pkg.py
index 1811b1a617fdddc381c1482b441cbf54951eba5b..8dfdf4773de3f3ad618411ea93195ee17ed1a276 100644
--- a/lib/spack/spack/test/cmd/pkg.py
+++ b/lib/spack/spack/test/cmd/pkg.py
@@ -315,6 +315,7 @@ def test_pkg_grep(mock_packages, capfd):
"depends-on-manyvariants",
"manyvariants",
"splice-a",
+ "splice-depends-on-t",
"splice-h",
"splice-t",
"splice-vh",
diff --git a/var/spack/repos/builtin.mock/packages/splice-depends-on-t/package.py b/var/spack/repos/builtin.mock/packages/splice-depends-on-t/package.py
new file mode 100644
index 0000000000000000000000000000000000000000..9f38da0daa04f0fdaac16fd0a58209eae9076c85
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/splice-depends-on-t/package.py
@@ -0,0 +1,22 @@
+# Copyright 2013-2024 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class SpliceDependsOnT(Package):
+ """Package that depends on splice-t"""
+
+ homepage = "http://www.example.com"
+ url = "http://www.example.com/splice-depends-on-t-1.0.tar.gz"
+
+ version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+ depends_on("splice-t")
+
+ def install(self, spec, prefix):
+ with open(prefix.join("splice-depends-on-t"), "w") as f:
+ f.write("splice-depends-on-t: {0}".format(prefix))
+ f.write("splice-t: {0}".format(spec["splice-t"].prefix))