Skip to content

Commit

Permalink
Fix memoization of CoarsenedTarget.closure (pantsbuild#17516)
Browse files Browse the repository at this point in the history
Memoization of `CoarsenedTargets.closure()` relies on shared memoization across a walk in each `CoarsenedTarget`. That memoization was not working.

Fixes pantsbuild#17509.
  • Loading branch information
stuhood authored Nov 9, 2022
1 parent 83b5f04 commit c1514b7
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ def coarsened_closure(
) -> Iterator[CoarsenedTarget]:
"""All CoarsenedTargets reachable from this root."""

visited = visited or set()
visited = set() if visited is None else visited
queue = deque([self])
while queue:
ct = queue.popleft()
Expand Down
24 changes: 23 additions & 1 deletion src/python/pants/engine/target_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from collections import namedtuple
from dataclasses import dataclass
from enum import Enum
from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, cast
from typing import Any, Dict, Iterable, List, Optional, Sequence, Set, Tuple, cast

import pytest

Expand All @@ -15,6 +15,7 @@
AsyncFieldMixin,
BoolField,
CoarsenedTarget,
CoarsenedTargets,
DictStringToStringField,
DictStringToStringSequenceField,
ExplicitlyProvidedDependencies,
Expand Down Expand Up @@ -498,6 +499,27 @@ def nested():
assert nested() == nested()


def test_coarsened_target_closure() -> None:
all_targets = [FortranTarget({}, Address(name)) for name in string.ascii_lowercase[:5]]
a, b, c, d, e = all_targets

def ct(members: List[Target], dependencies: List[CoarsenedTarget] = []) -> CoarsenedTarget:
return CoarsenedTarget(members, dependencies)

def assert_closure(cts: Sequence[CoarsenedTarget], expected: Sequence[Target]) -> None:
assert sorted(t.address for t in CoarsenedTargets(cts).closure()) == sorted(
t.address for t in expected
)

ct1 = ct([a], [])
ct2 = ct([b, c], [ct1])
ct3 = ct([d, e], [ct1, ct2])

assert_closure([ct1], [a])
assert_closure([ct1, ct2], [a, b, c])
assert_closure([ct1, ct2, ct3], all_targets)


# -----------------------------------------------------------------------------------------------
# Test file-level target generation
# -----------------------------------------------------------------------------------------------
Expand Down

0 comments on commit c1514b7

Please sign in to comment.