Review feedback.

[ci skip-rust-tests]

src/python/pants/engine/internals/graph.py

@@ -143,7 +143,7 @@ def __init__(self, subject: Address, path: Tuple[Address, ...]) -> None:
 
 
 def _detect_cycles(
-    roots: Tuple[Address, ...], dependencies: Dict[Address, Tuple[Address, ...]]
+    roots: Tuple[Address, ...], dependency_mapping: Dict[Address, Tuple[Address, ...]]
 ) -> None:
     path_stack: OrderedSet[Address] = OrderedSet()
     visited: Set[Address] = set()
@@ -152,12 +152,12 @@ def visit(address: Address):
         if address in visited:
             # NB: File-level dependencies are cycle tolerant.
             if not address.generated_base_target_name and address in path_stack:
-                raise CycleException(address, tuple(path_stack) + (address,))
+                raise CycleException(address, (*path_stack, address))
             return
         path_stack.add(address)
         visited.add(address)
 
-        for dep_address in dependencies[address]:
+        for dep_address in dependency_mapping[address]:
             visit(dep_address)
 
         path_stack.remove(address)

src/python/pants/engine/internals/graph_test.py

@@ -5,7 +5,7 @@
 from dataclasses import dataclass
 from pathlib import PurePath
 from textwrap import dedent
-from typing import Iterable, List, Sequence, Type
+from typing import Iterable, List, Sequence, Tuple, Type
 
 import pytest
 
@@ -29,6 +29,7 @@
 from pants.engine.internals.graph import (
     AmbiguousCodegenImplementationsException,
     AmbiguousImplementationsException,
+    CycleException,
     InvalidFileDependencyException,
     NoValidTargetsException,
     Owners,
@@ -153,15 +154,18 @@ def test_transitive_targets_tolerates_subtarget_cycles(self) -> None:
             Address("", target_name="t2.txt", generated_base_target_name="t2"),
         ]
 
-    def assert_failed_cycle(self, address_str: str, cyclic_address_str: str) -> None:
-        with self.assertRaisesRegex(
-            Exception,
-            f"(?ms)Dependency graph contained a cycle:.*-> {cyclic_address_str}.*-> {cyclic_address_str}.*",
-        ):
+    def assert_failed_cycle(
+        self, root_addr: str, subject_addr: str, path_addrs: Tuple[str, ...]
+    ) -> None:
+        with self.assertRaises(ExecutionError) as e:
             self.request_single_product(
                 TransitiveTargets,
-                Params(Addresses([Address.parse(address_str)]), create_options_bootstrapper()),
+                Params(Addresses([Address.parse(root_addr)]), create_options_bootstrapper()),
             )
+        (cycle_exception,) = e.exception.wrapped_exceptions
+        assert isinstance(cycle_exception, CycleException)
+        assert cycle_exception.subject == Address.parse(subject_addr)
+        assert cycle_exception.path == tuple(Address.parse(p) for p in path_addrs)
 
     def test_cycle_self(self) -> None:
         self.add_to_build_file(
@@ -172,7 +176,7 @@ def test_cycle_self(self) -> None:
                 """
             ),
         )
-        self.assert_failed_cycle("//:t1", "//:t1")
+        self.assert_failed_cycle("//:t1", "//:t1", ("//:t1", "//:t1"))
 
     def test_cycle_direct(self) -> None:
         self.add_to_build_file(
@@ -184,8 +188,8 @@ def test_cycle_direct(self) -> None:
                 """
             ),
         )
-        self.assert_failed_cycle("//:t1", "//:t1")
-        self.assert_failed_cycle("//:t2", "//:t2")
+        self.assert_failed_cycle("//:t1", "//:t1", ("//:t1", "//:t2", "//:t1"))
+        self.assert_failed_cycle("//:t2", "//:t2", ("//:t2", "//:t1", "//:t2"))
 
     def test_cycle_indirect(self) -> None:
         self.add_to_build_file(
@@ -198,8 +202,8 @@ def test_cycle_indirect(self) -> None:
                 """
             ),
         )
-        self.assert_failed_cycle("//:t1", "//:t2")
-        self.assert_failed_cycle("//:t2", "//:t2")
+        self.assert_failed_cycle("//:t1", "//:t2", ("//:t1", "//:t2", "//:t3", "//:t2"))
+        self.assert_failed_cycle("//:t2", "//:t2", ("//:t2", "//:t3", "//:t2"))
 
     def test_resolve_generated_subtarget(self) -> None:
         self.add_to_build_file("demo", "target(sources=['f1.txt', 'f2.txt'])")

src/python/pants/engine/target.py

@@ -524,14 +524,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 dependencies.