Skip to content

Commit

Permalink
Fix generated subtargets not working with dependees (#10355)
Browse files Browse the repository at this point in the history
Because it's tricky to get right and also not critical for Pants to work, we, for now, punt on `dependees` using generated subtargets and instead convert back to the original base target. #10354 tracks doing the better thing.

[ci skip-rust-tests]
  • Loading branch information
Eric-Arellano committed Jul 14, 2020
1 parent 506a856 commit 248a903
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 23 deletions.
10 changes: 6 additions & 4 deletions src/python/pants/backend/project_info/dependees.py
Expand Up @@ -28,14 +28,16 @@ class AddressToDependees:
@rule
async def map_addresses_to_dependees() -> AddressToDependees:
# Get every target in the project so that we can iterate over them to find their dependencies.
all_targets = await Get(Targets, AddressSpecs([DescendantAddresses("")]))
all_explicit_targets = await Get(Targets, AddressSpecs([DescendantAddresses("")]))
dependencies_per_target = await MultiGet(
Get(Addresses, DependenciesRequest(tgt.get(Dependencies))) for tgt in all_targets
Get(Addresses, DependenciesRequest(tgt.get(Dependencies))) for tgt in all_explicit_targets
)

address_to_dependees = defaultdict(set)
for tgt, dependencies in zip(all_targets, dependencies_per_target):
for tgt, dependencies in zip(all_explicit_targets, dependencies_per_target):
for dependency in dependencies:
# TODO(#10354): teach dependees how to work with generated subtargets.
dependency = dependency.maybe_convert_to_base_target()
address_to_dependees[dependency].add(tgt.address)
return AddressToDependees(
FrozenDict(
Expand All @@ -54,7 +56,7 @@ class DependeesRequest:
def __init__(
self, addresses: Iterable[Address], *, transitive: bool, include_roots: bool
) -> None:
self.addresses = FrozenOrderedSet(addresses)
self.addresses = FrozenOrderedSet(addr.maybe_convert_to_base_target() for addr in addresses)
self.transitive = transitive
self.include_roots = include_roots

Expand Down
14 changes: 14 additions & 0 deletions src/python/pants/build_graph/address.py
Expand Up @@ -244,6 +244,15 @@ def reference(self, referencing_path: Optional[str] = None) -> str:
return self.spec
return self._spec_path

def maybe_convert_to_base_target(self) -> "Address":
"""If this address is a generated subtarget, convert it back into its original base target.
Otherwise, return itself unmodified.
"""
if not self.generated_base_target_name:
return self
return self.__class__(self._spec_path, target_name=self.generated_base_target_name)

def __eq__(self, other):
if not isinstance(other, Address):
return False
Expand Down Expand Up @@ -319,6 +328,11 @@ def to_address(self) -> Address:
generated_base_target_name=self.generated_base_target_name,
)

def maybe_convert_to_base_target(self) -> "BuildFileAddress":
if not self.generated_base_target_name:
return self
return self.__class__(rel_path=self.rel_path, target_name=self.generated_base_target_name)

def __repr__(self) -> str:
prefix = f"BuildFileAddress({self.rel_path}, {self.target_name}"
return (
Expand Down
11 changes: 11 additions & 0 deletions src/python/pants/build_graph/address_test.py
Expand Up @@ -204,6 +204,14 @@ def test_address_spec() -> None:
assert generated_subdirectory_addr.path_safe_spec == "a.b.subdir.c.txt"


def test_address_maybe_convert_to_base_target() -> None:
generated_addr = Address("a/b", "c.txt", generated_base_target_name="c")
assert generated_addr.maybe_convert_to_base_target() == Address("a/b", "c")

normal_addr = Address("a/b", "c")
assert normal_addr.maybe_convert_to_base_target() is normal_addr


def test_address_parse_method() -> None:
def assert_parsed(spec_path: str, target_name: str, address: Address) -> None:
assert spec_path == address.spec_path
Expand Down Expand Up @@ -236,3 +244,6 @@ def test_build_file_address() -> None:
assert generated_bfa != BuildFileAddress(rel_path="dir/BUILD", target_name="example.txt")
assert generated_bfa == Address("dir", "example.txt", generated_base_target_name="original")
assert generated_bfa.spec == "dir/example.txt"
assert generated_bfa.maybe_convert_to_base_target() == BuildFileAddress(
rel_path="dir/BUILD", target_name="original"
)
6 changes: 1 addition & 5 deletions src/python/pants/engine/internals/build_files.py
Expand Up @@ -101,11 +101,7 @@ def _raise_did_you_mean(address_family: AddressFamily, name: str, source=None) -
@rule
async def find_build_file(address: Address) -> BuildFileAddress:
address_family = await Get(AddressFamily, Dir(address.spec_path))
owning_address = (
address
if not address.generated_base_target_name
else Address(address.spec_path, address.generated_base_target_name)
)
owning_address = address.maybe_convert_to_base_target()
if owning_address not in address_family.addressables:
_raise_did_you_mean(address_family=address_family, name=owning_address.target_name)
bfa = next(
Expand Down
20 changes: 6 additions & 14 deletions src/python/pants/engine/internals/graph.py
Expand Up @@ -63,9 +63,7 @@ async def resolve_target(
address: Address, registered_target_types: RegisteredTargetTypes
) -> WrappedTarget:
if address.generated_base_target_name:
base_target = await Get(
WrappedTarget, Address(address.spec_path, address.generated_base_target_name)
)
base_target = await Get(WrappedTarget, Address, address.maybe_convert_to_base_target())
subtarget = generate_subtarget(
base_target.target,
full_file_name=PurePath(address.spec_path, address.target_name).as_posix(),
Expand Down Expand Up @@ -271,20 +269,14 @@ async def resolve_addresses_with_origins(specs: Specs) -> AddressesWithOrigins:
# subtarget; if the user explicitly specified the original owning target, we should use the
# original target rather than its generated subtarget.
address_spec_addresses = FrozenOrderedSet(awo.address for awo in from_address_specs)

def in_address_specs(filesystem_spec_address: Address) -> bool:
if not filesystem_spec_address.generated_base_target_name:
return filesystem_spec_address in address_spec_addresses
original_address = Address(
filesystem_spec_address.spec_path,
target_name=filesystem_spec_address.generated_base_target_name,
)
return original_address in address_spec_addresses

return AddressesWithOrigins(
[
*from_address_specs,
*(awo for awo in from_filesystem_specs if not in_address_specs(awo.address)),
*(
awo
for awo in from_filesystem_specs
if awo.address.maybe_convert_to_base_target() not in address_spec_addresses
),
]
)

Expand Down

0 comments on commit 248a903

Please sign in to comment.