diff options
Diffstat (limited to 'lib/spack/spack/spec.py')
| -rw-r--r-- | lib/spack/spack/spec.py | 349 |
1 files changed, 217 insertions, 132 deletions
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index eac51b0f46..030bb50913 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1739,19 +1739,28 @@ class Spec: self.add_dependency_edge(spec, depflag=depflag, virtuals=virtuals) return - # Keep the intersection of constraints when a dependency is added multiple times. - # The only restriction, currently, is keeping the same dependency type + # Keep the intersection of constraints when a dependency is added multiple times with + # the same deptype. Add a new dependency if it is added with a compatible deptype + # (for example, a build-only dependency is compatible with a link-only dependenyc). + # The only restrictions, currently, are that we cannot add edges with overlapping + # dependency types and we cannot add multiple edges that have link/run dependency types. + # See ``spack.deptypes.compatible``. orig = self._dependencies[spec.name] try: dspec = next(dspec for dspec in orig if depflag == dspec.depflag) except StopIteration: - edge_attrs = f"deptypes={dt.flag_to_chars(depflag).strip()}" - required_dep_str = f"^[{edge_attrs}] {str(spec)}" + # Error if we have overlapping or incompatible deptypes + if any(not dt.compatible(dspec.depflag, depflag) for dspec in orig): + edge_attrs = f"deptypes={dt.flag_to_chars(depflag).strip()}" + required_dep_str = f"^[{edge_attrs}] {str(spec)}" + + raise DuplicateDependencyError( + f"{spec.name} is a duplicate dependency, with conflicting dependency types\n" + f"\t'{str(self)}' cannot depend on '{required_dep_str}'" + ) - raise DuplicateDependencyError( - f"{spec.name} is a duplicate dependency, with conflicting dependency types\n" - f"\t'{str(self)}' cannot depend on '{required_dep_str}'" - ) + self.add_dependency_edge(spec, depflag=depflag, virtuals=virtuals) + return try: dspec.spec.constrain(spec) @@ -1776,7 +1785,10 @@ class Spec: for edge in selected: has_errors, details = False, [] msg = f"cannot update the edge from {edge.parent.name} to {edge.spec.name}" - if edge.depflag & depflag: + + # If the dependency is to an existing spec, we can update dependency + # types. If it is to a new object, check deptype compatibility. + if id(edge.spec) != id(dependency_spec) and not dt.compatible(edge.depflag, depflag): has_errors = True details.append( ( @@ -1785,14 +1797,13 @@ class Spec: ) ) - if any(v in edge.virtuals for v in virtuals): - has_errors = True - details.append( - ( - f"{edge.parent.name} has already an edge matching any" - f" of these virtuals {virtuals}" + if any(v in edge.virtuals for v in virtuals): + details.append( + ( + f"{edge.parent.name} has already an edge matching any" + f" of these virtuals {virtuals}" + ) ) - ) if has_errors: raise spack.error.SpecError(msg, "\n".join(details)) @@ -4168,6 +4179,144 @@ class Spec: new_dependencies.add(edge) spec._dependencies = new_dependencies + def _virtuals_provided(self, root): + """Return set of virtuals provided by self in the context of root""" + if root is self: + # Could be using any virtual the package can provide + return set(self.package.virtuals_provided) + + hashes = [s.dag_hash() for s in root.traverse()] + in_edges = set( + [edge for edge in self.edges_from_dependents() if edge.parent.dag_hash() in hashes] + ) + return set().union(*[edge.virtuals for edge in in_edges]) + + def _splice_match(self, other, self_root, other_root): + """Return True if other is a match for self in a splice of other_root into self_root + + Other is a splice match for self if it shares a name, or if self is a virtual provider + and other provides a superset of the virtuals provided by self. Virtuals provided are + evaluated in the context of a root spec (self_root for self, other_root for other). + + This is a slight oversimplification. Other could be a match for self in the context of + one edge in self_root and not in the context of another edge. This method could be + expanded in the future to account for these cases. + """ + if other.name == self.name: + return True + + return bool( + self._virtuals_provided(self_root) + and self._virtuals_provided(self_root) <= other._virtuals_provided(other_root) + ) + + def _splice_detach_and_add_dependents(self, replacement, context): + """Helper method for Spec._splice_helper. + + replacement is a node to splice in, context is the scope of dependents to consider relevant + to this splice.""" + # Update build_spec attributes for all transitive dependents + # before we start changing their dependencies + ancestors_in_context = [ + a + for a in self.traverse(root=False, direction="parents") + if a in context.traverse(deptype=dt.LINK | dt.RUN) + ] + for ancestor in ancestors_in_context: + # Only set it if it hasn't been spliced before + ancestor._build_spec = ancestor._build_spec or ancestor.copy() + ancestor.clear_cached_hashes(ignore=(ht.package_hash.attr,)) + + # For each direct dependent in the link/run graph, replace the dependency on + # node with one on replacement + # For each build dependent, restrict the edge to build-only + for edge in self.edges_from_dependents(): + if edge.parent not in ancestors_in_context: + continue + build_dep = edge.depflag & dt.BUILD + other_dep = edge.depflag & ~dt.BUILD + if build_dep: + parent_edge = [e for e in edge.parent._dependencies[self.name] if e.spec is self] + assert len(parent_edge) == 1 + + edge.depflag = dt.BUILD + parent_edge[0].depflag = dt.BUILD + else: + edge.parent._dependencies.edges[self.name].remove(edge) + self._dependents.edges[edge.parent.name].remove(edge) + + if other_dep: + edge.parent._add_dependency(replacement, depflag=other_dep, virtuals=edge.virtuals) + + def _splice_helper(self, replacement, self_root, other_root): + """Main loop of a transitive splice. + + The while loop around a traversal of self ensures that changes to self from previous + iterations are reflected in the traversal. This avoids evaluating irrelevant nodes + using topological traversal (all incoming edges traversed before any outgoing edge). + If any node will not be in the end result, its parent will be spliced and it will not + ever be considered. + For each node in self, find any analogous node in replacement and swap it in. + We assume all build deps are handled outside of this method + + Arguments: + replacement: The node that will replace any equivalent node in self + self_root: The root of the spec that self comes from. This provides the context for + evaluating whether ``replacement`` is a match for each node of ``self``. See + ``Spec._splice_match`` and ``Spec._virtuals_provided`` for details. + other_root: The root of the spec that replacement comes from. This provides the context + for evaluating whether ``replacement`` is a match for each node of ``self``. See + ``Spec._splice_match`` and ``Spec._virtuals_provided`` for details. + """ + ids = set(id(s) for s in replacement.traverse()) + + # Sort all possible replacements by name and virtual for easy access later + replacements_by_name = collections.defaultdict(list) + for node in replacement.traverse(): + replacements_by_name[node.name].append(node) + virtuals = node._virtuals_provided(root=replacement) + for virtual in virtuals: + # Virtual may be spec or str, get name or return str + replacements_by_name[getattr(virtual, "name", virtual)].append(node) + + changed = True + while changed: + changed = False + + # Intentionally allowing traversal to change on each iteration + # using breadth-first traversal to ensure we only reach nodes that will + # be in final result + for node in self.traverse(root=False, order="topo", deptype=dt.ALL & ~dt.BUILD): + # If this node has already been swapped in, don't consider it again + if id(node) in ids: + continue + + analogs = replacements_by_name[node.name] + if not analogs: + # If we have to check for matching virtuals, then we need to check that it + # matches all virtuals. Use `_splice_match` to validate possible matches + for virtual in node._virtuals_provided(root=self): + analogs += [ + r + for r in replacements_by_name[getattr(virtual, "name", virtual)] + if r._splice_match(node, self_root=self_root, other_root=other_root) + ] + + # No match, keep iterating over self + if not analogs: + continue + + # If there are multiple analogs, this package must satisfy the constraint + # that a newer version can always replace a lesser version. + analog = max(analogs, key=lambda s: s.version) + + # No splice needed here, keep checking + if analog == node: + continue + node._splice_detach_and_add_dependents(analog, context=self) + changed = True + break + def splice(self, other, transitive): """Splices dependency "other" into this ("target") Spec, and return the result as a concrete Spec. @@ -4188,134 +4337,70 @@ class Spec: the same way it was built, any such changes are tracked by setting the build_spec to point to the corresponding dependency from the original Spec. - TODO: Extend this for non-concrete Specs. """ assert self.concrete assert other.concrete - virtuals_to_replace = [v.name for v in other.package.virtuals_provided if v in self] - if virtuals_to_replace: - deps_to_replace = dict((self[v], other) for v in virtuals_to_replace) - # deps_to_replace = [self[v] for v in virtuals_to_replace] - else: - # TODO: sanity check and error raise here for other.name not in self - deps_to_replace = {self[other.name]: other} - # deps_to_replace = [self[other.name]] - - for d in deps_to_replace: - if not all( - v in other.package.virtuals_provided or v not in self - for v in d.package.virtuals_provided - ): - # There was something provided by the original that we don't - # get from its replacement. - raise SpliceError( - ("Splice between {0} and {1} will not provide " "the same virtuals.").format( - self.name, other.name - ) + if not any( + node._splice_match(other, self_root=self, other_root=other) + for node in self.traverse(root=False, deptype=dt.LINK | dt.RUN) + ): + other_str = other.format("{name}/{hash:7}") + self_str = self.format("{name}/{hash:7}") + msg = f"Cannot splice {other_str} into {self_str}." + msg += f" Either {self_str} cannot depend on {other_str}," + msg += f" or {other_str} fails to provide a virtual used in {self_str}" + raise SpliceError(msg) + + # Copies of all non-build deps, build deps will get added at the end + spec = self.copy(deps=dt.ALL & ~dt.BUILD) + replacement = other.copy(deps=dt.ALL & ~dt.BUILD) + + def make_node_pairs(orig_spec, copied_spec): + return list( + zip( + orig_spec.traverse(deptype=dt.ALL & ~dt.BUILD), + copied_spec.traverse(deptype=dt.ALL & ~dt.BUILD), ) - for n in d.traverse(root=False): - if not all( - any( - v in other_n.package.virtuals_provided - for other_n in other.traverse(root=False) - ) - or v not in self - for v in n.package.virtuals_provided - ): - raise SpliceError( - ( - "Splice between {0} and {1} will not provide " "the same virtuals." - ).format(self.name, other.name) - ) - - # For now, check that we don't have DAG with multiple specs from the - # same package - def multiple_specs(root): - counter = collections.Counter([node.name for node in root.traverse()]) - _, max_number = counter.most_common()[0] - return max_number > 1 - - if multiple_specs(self) or multiple_specs(other): - msg = ( - 'Either "{0}" or "{1}" contain multiple specs from the same ' - "package, which cannot be handled by splicing at the moment" ) - raise ValueError(msg.format(self, other)) - # Multiple unique specs with the same name will collide, so the - # _dependents of these specs should not be trusted. - # Variants may also be ignored here for now... - - # Keep all cached hashes because we will invalidate the ones that need - # invalidating later, and we don't want to invalidate unnecessarily - - def from_self(name, transitive): - if transitive: - if name in other: - return False - if any(v in other for v in self[name].package.virtuals_provided): - return False - return True - else: - if name == other.name: - return False - if any( - v in other.package.virtuals_provided - for v in self[name].package.virtuals_provided - ): - return False - return True - - self_nodes = dict( - (s.name, s.copy(deps=False)) - for s in self.traverse(root=True) - if from_self(s.name, transitive) - ) + def mask_build_deps(in_spec): + for edge in in_spec.traverse_edges(cover="edges"): + edge.depflag &= ~dt.BUILD if transitive: - other_nodes = dict((s.name, s.copy(deps=False)) for s in other.traverse(root=True)) - else: - # NOTE: Does not fully validate providers; loader races possible - other_nodes = dict( - (s.name, s.copy(deps=False)) - for s in other.traverse(root=True) - if s is other or s.name not in self - ) - - nodes = other_nodes.copy() - nodes.update(self_nodes) + # These pairs will allow us to reattach all direct build deps + # We need the list of pairs while the two specs still match + node_pairs = make_node_pairs(self, spec) - for name in nodes: - if name in self_nodes: - for edge in self[name].edges_to_dependencies(): - dep_name = deps_to_replace.get(edge.spec, edge.spec).name - nodes[name].add_dependency_edge( - nodes[dep_name], depflag=edge.depflag, virtuals=edge.virtuals - ) - if any(dep not in self_nodes for dep in self[name]._dependencies): - nodes[name].build_spec = self[name].build_spec - else: - for edge in other[name].edges_to_dependencies(): - nodes[name].add_dependency_edge( - nodes[edge.spec.name], depflag=edge.depflag, virtuals=edge.virtuals - ) - if any(dep not in other_nodes for dep in other[name]._dependencies): - nodes[name].build_spec = other[name].build_spec + # Ignore build deps in the modified spec while doing the splice + # They will be added back in at the end + mask_build_deps(spec) - ret = nodes[self.name] - - # Clear cached hashes for all affected nodes - # Do not touch unaffected nodes - for dep in ret.traverse(root=True, order="post"): - opposite = other_nodes if dep.name in self_nodes else self_nodes - if any(name in dep for name in opposite.keys()): - # package hash cannot be affected by splice - dep.clear_cached_hashes(ignore=["package_hash"]) - - dep.dag_hash() + # Transitively splice any relevant nodes from new into base + # This handles all shared dependencies between self and other + spec._splice_helper(replacement, self_root=self, other_root=other) + else: + # Do the same thing as the transitive splice, but reversed + node_pairs = make_node_pairs(other, replacement) + mask_build_deps(replacement) + replacement._splice_helper(spec, self_root=other, other_root=self) + + # Intransitively splice replacement into spec + # This is very simple now that all shared dependencies have been handled + for node in spec.traverse(order="topo", deptype=dt.LINK | dt.RUN): + if node._splice_match(other, self_root=spec, other_root=other): + node._splice_detach_and_add_dependents(replacement, context=spec) + + # Set up build dependencies for modified nodes + # Also modify build_spec because the existing ones had build deps removed + for orig, copy in node_pairs: + for edge in orig.edges_to_dependencies(depflag=dt.BUILD): + copy._add_dependency(edge.spec, depflag=dt.BUILD, virtuals=edge.virtuals) + if copy._build_spec: + copy._build_spec = orig.build_spec.copy() - return nodes[self.name] + return spec def clear_cached_hashes(self, ignore=()): """ |