diff --git a/src/python/pants/engine/internals/graph.py b/src/python/pants/engine/internals/graph.py index ce683913357..0de663d3e6e 100644 --- a/src/python/pants/engine/internals/graph.py +++ b/src/python/pants/engine/internals/graph.py @@ -1,9 +1,10 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import itertools import logging import os.path -from collections import defaultdict, deque +from collections import defaultdict from dataclasses import dataclass from pathlib import PurePath from typing import DefaultDict, List, Tuple, Union, cast @@ -39,7 +40,6 @@ Targets, TargetsWithOrigins, TargetWithOrigin, - TransitiveTarget, TransitiveTargets, UnrecognizedTargetTypeException, WrappedTarget, @@ -123,37 +123,24 @@ async def resolve_targets_with_origins( @rule -async def transitive_target(wrapped_root: WrappedTarget) -> TransitiveTarget: - root = wrapped_root.target - if not root.has_field(Dependencies): - return TransitiveTarget(root, ()) - dependency_addresses = await Get[Addresses](DependenciesRequest(root[Dependencies])) - dependencies = await MultiGet(Get[TransitiveTarget](Address, d) for d in dependency_addresses) - return TransitiveTarget(root, dependencies) +async def transitive_targets(targets: Targets) -> TransitiveTargets: + """Find all the targets transitively depended upon by the target roots. - -@rule -async def transitive_targets(addresses: Addresses) -> TransitiveTargets: - """Given Addresses, kicks off recursion on expansion of TransitiveTargets. - - The TransitiveTarget dataclass represents a structure-shared graph, which we walk and flatten - here. The engine memoizes the computation of TransitiveTarget, so when multiple - TransitiveTargets objects are being constructed for multiple roots, their structure will be - shared. + This uses iteration, rather than recursion, so that we can tolerate dependency cycles. Unlike a + traditional BFS algorithm, we batch each round of traversals via `MultiGet` for improved + performance / concurrency. """ - transitive_targets = await MultiGet(Get[TransitiveTarget](Address, a) for a in addresses) - - closure: OrderedSet[Target] = OrderedSet() - to_visit = deque(transitive_targets) - - while to_visit: - tt = to_visit.popleft() - if tt.root in closure: - continue - closure.add(tt.root) - to_visit.extend(tt.dependencies) - - return TransitiveTargets(tuple(tt.root for tt in transitive_targets), FrozenOrderedSet(closure)) + visited: OrderedSet[Target] = OrderedSet() + queued = FrozenOrderedSet(targets) + while queued: + visited.update(queued) + direct_dependencies = await MultiGet( + Get[Targets](DependenciesRequest(tgt.get(Dependencies))) for tgt in queued + ) + queued = FrozenOrderedSet(itertools.chain.from_iterable(direct_dependencies)).difference( + visited + ) + return TransitiveTargets(tuple(targets), FrozenOrderedSet(visited)) # ----------------------------------------------------------------------------------------------- @@ -313,7 +300,6 @@ def rules(): resolve_targets, resolve_target_with_origin, resolve_targets_with_origins, - transitive_target, transitive_targets, find_owners, addresses_with_origins_from_filesystem_specs, diff --git a/src/python/pants/engine/internals/graph_test.py b/src/python/pants/engine/internals/graph_test.py index 486cba93a63..fbc16edaac9 100644 --- a/src/python/pants/engine/internals/graph_test.py +++ b/src/python/pants/engine/internals/graph_test.py @@ -11,7 +11,6 @@ DependenciesRequest, Target, Targets, - TransitiveTarget, TransitiveTargets, WrappedTarget, ) @@ -44,6 +43,11 @@ def test_transitive_targets(self) -> None: {Dependencies.alias: [d1.address, d2.address, d3.address]}, address=Address.parse(":root"), ) + cycle1_addr = Address.parse(":cycle1") + cycle2_addr = Address.parse(":cycle2") + cycle1 = MockTarget({Dependencies.alias: [cycle2_addr]}, address=cycle1_addr) + # Also include a dependency on itself (self-cycle) + cycle2 = MockTarget({Dependencies.alias: [cycle1_addr, cycle2_addr]}, address=cycle2_addr) self.add_to_build_file( "", @@ -55,6 +59,8 @@ def test_transitive_targets(self) -> None: target(name='d2', dependencies=[':t2']) target(name='d3') target(name='root', dependencies=[':d1', ':d2', ':d3']) + target(name='cycle1', dependencies=[':cycle2']) + target(name='cycle2', dependencies=[':cycle1', ':cycle2']) """ ), ) @@ -64,17 +70,13 @@ def test_transitive_targets(self) -> None: ) assert direct_deps == Targets([d1, d2, d3]) - transitive_target = self.request_single_product( - TransitiveTarget, Params(WrappedTarget(root), create_options_bootstrapper()) - ) - assert transitive_target.root == root - assert { - dep_transitive_target.root for dep_transitive_target in transitive_target.dependencies - } == {d1, d2, d3} - transitive_targets = self.request_single_product( TransitiveTargets, - Params(Addresses([root.address, d2.address]), create_options_bootstrapper()), + Params( + Addresses([root.address, d2.address, cycle2_addr]), create_options_bootstrapper() + ), + ) + assert transitive_targets.roots == (root, d2, cycle2) + assert transitive_targets.closure == FrozenOrderedSet( + [root, d2, cycle2, d1, d3, t2, cycle1, t1] ) - assert transitive_targets.roots == (root, d2) - assert transitive_targets.closure == FrozenOrderedSet([root, d2, d1, d3, t2, t1]) diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index a0f0ec9d2a3..486a1f4d14d 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -537,14 +537,6 @@ def targets(self) -> Tuple[Target, ...]: return tuple(tgt_with_origin.target for tgt_with_origin in self) -@dataclass(frozen=True) -class TransitiveTarget: - """A recursive structure wrapping a Target root and TransitiveTarget deps.""" - - root: Target - dependencies: Tuple["TransitiveTarget", ...] - - @dataclass(frozen=True) class TransitiveTargets: """A set of Target roots, and their transitive, flattened, de-duped closure."""