Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace BuildFileAddress with Address for HydratedTarget and TargetAdaptor #9100

Merged
merged 6 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async def create_python_awslambda(
# TODO: We must enforce that everything is built for Linux, no matter the local platform.
pex_filename = f'{lambda_tgt_adaptor.address.target_name}.pex'
pex_request = CreatePexFromTargetClosure(
addresses=Addresses((lambda_tgt_adaptor.address.to_address(),)),
addresses=Addresses([lambda_tgt_adaptor.address]),
entry_point=None,
output_filename=pex_filename
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async def create_python_binary(python_binary_adaptor: PythonBinaryAdaptor) -> Cr
entry_point = PythonBinary.translate_source_path_to_py_module_specifier(root_filename)

request = CreatePexFromTargetClosure(
addresses=Addresses((python_binary_adaptor.address.to_address(),)),
addresses=Addresses((python_binary_adaptor.address,)),
entry_point=entry_point,
output_filename=f'{python_binary_adaptor.address.target_name}.pex'
)
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/backend/python/rules/python_test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,13 @@ async def setup_pytest_for_target(
# TODO: Rather than consuming the TestOptions subsystem, the TestRunner should pass on coverage
# configuration via #7490.
transitive_hydrated_targets = await Get[TransitiveHydratedTargets](
Addresses((test_target.address.to_address(),))
Addresses((test_target.address,))
)
all_targets = transitive_hydrated_targets.closure

resolved_requirements_pex = await Get[Pex](
CreatePexFromTargetClosure(
addresses=Addresses((test_target.address.to_address(),)),
addresses=Addresses((test_target.address,)),
output_filename='pytest-with-requirements.pex',
entry_point="pytest:main",
additional_requirements=pytest.get_requirement_strings(),
Expand All @@ -132,7 +132,7 @@ async def setup_pytest_for_target(
# optimization, this ensures that any transitive sources, such as a test project file named
# test_fail.py, do not unintentionally end up being run as tests.
source_root_stripped_test_target_sources = await Get[SourceRootStrippedSources](
Address, test_target.address.to_address()
Address, test_target.address
)

coverage_args = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from pants.backend.python.targets.python_library import PythonLibrary
from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
from pants.backend.python.targets.python_tests import PythonTests
from pants.build_graph.address import BuildFileAddress
from pants.build_graph.address import Address
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.engine.fs import FileContent
from pants.engine.interactive_runner import InteractiveRunner
Expand Down Expand Up @@ -130,9 +130,7 @@ def run_pytest(self, *, passthrough_args: Optional[str] = None) -> TestResult:
if passthrough_args:
args.append(f"--pytest-args='{passthrough_args}'")
options_bootstrapper = create_options_bootstrapper(args=args)
target = PythonTestsAdaptor(
address=BuildFileAddress(rel_path=f"{self.source_root}/BUILD", target_name="target"),
)
target = PythonTestsAdaptor(address=Address.parse(f"{self.source_root}:target"))
test_result = self.request_single_product(TestResult, Params(target, options_bootstrapper))
debug_request = self.request_single_product(
TestDebugRequest, Params(target, options_bootstrapper),
Expand Down
12 changes: 6 additions & 6 deletions src/python/pants/backend/python/rules/run_setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ async def run_setup_pys(targets: HydratedTargets, options: SetupPyOptions, conso
for hydrated_target in targets:
if _is_exported(hydrated_target):
exported_targets.append(ExportedTarget(hydrated_target))
elif address_origin_map.is_single_address(hydrated_target.address.to_address()):
elif address_origin_map.is_single_address(hydrated_target.address):
explicit_nonexported_targets.append(hydrated_target)
if explicit_nonexported_targets:
raise TargetNotExported(
Expand All @@ -259,7 +259,7 @@ async def run_setup_pys(targets: HydratedTargets, options: SetupPyOptions, conso
if options.values.transitive:
# Expand out to all owners of the entire dep closure.
tht = await Get[TransitiveHydratedTargets](
Addresses(et.hydrated_target.address.to_address() for et in exported_targets))
Addresses(et.hydrated_target.address for et in exported_targets))
owners = await MultiGet(
Get[ExportedTarget](OwnedDependency(ht)) for ht in tht.closure if is_ownable_target(ht)
)
Expand Down Expand Up @@ -471,7 +471,7 @@ def _is_exported(target: HydratedTarget) -> bool:
@rule(name="Compute distribution's 3rd party requirements")
async def get_requirements(dep_owner: DependencyOwner) -> ExportedTargetRequirements:
tht = await Get[TransitiveHydratedTargets](
Addresses([dep_owner.exported_target.hydrated_target.address.to_address()]))
Addresses([dep_owner.exported_target.hydrated_target.address]))

ownable_tgts = [tgt for tgt in tht.closure if is_ownable_target(tgt)]
owners = await MultiGet(Get[ExportedTarget](OwnedDependency(ht)) for ht in ownable_tgts)
Expand Down Expand Up @@ -515,7 +515,7 @@ async def get_owned_dependencies(dependency_owner: DependencyOwner) -> OwnedDepe
Includes dependency_owner itself.
"""
tht = await Get[TransitiveHydratedTargets](
Addresses([dependency_owner.exported_target.hydrated_target.address.to_address()]))
Addresses([dependency_owner.exported_target.hydrated_target.address]))
ownable_targets = [tgt for tgt in tht.closure
if isinstance(tgt.adaptor, (PythonTargetAdaptor, ResourcesAdaptor))]
owners = await MultiGet(Get[ExportedTarget](OwnedDependency(ht)) for ht in ownable_targets)
Expand Down Expand Up @@ -547,15 +547,15 @@ async def get_exporting_owner(owned_dependency: OwnedDependency) -> ExportedTarg
[t for t in ancestor_tgts if _is_exported(t)], key=lambda t: t.address, reverse=True)
exported_ancestor_iter = iter(exported_ancestor_tgts)
for exported_ancestor in exported_ancestor_iter:
tht = await Get[TransitiveHydratedTargets](Addresses([exported_ancestor.address.to_address()]))
tht = await Get[TransitiveHydratedTargets](Addresses([exported_ancestor.address]))
if hydrated_target in tht.closure:
owner = exported_ancestor
# Find any exported siblings of owner that also depend on hydrated_target. They have the
# same spec_path as it, so they must immediately follow it in ancestor_iter.
sibling_owners = []
sibling = next(exported_ancestor_iter, None)
while sibling and sibling.address.spec_path == owner.address.spec_path:
tht = await Get[TransitiveHydratedTargets](Addresses([sibling.address.to_address()]))
tht = await Get[TransitiveHydratedTargets](Addresses([sibling.address]))
if hydrated_target in tht.closure:
sibling_owners.append(sibling)
sibling = next(exported_ancestor_iter, None)
Expand Down
9 changes: 3 additions & 6 deletions src/python/pants/engine/addressable.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from collections.abc import MutableMapping, MutableSequence
from dataclasses import dataclass
from functools import update_wrapper
from typing import Any, List, Set, Tuple, Type, Union
from typing import Any, Set, Tuple, Type, Union

from pants.base.exceptions import ResolveError
from pants.base.specs import AddressSpec, FilesystemResolvedSpec
Expand All @@ -31,7 +31,7 @@ def expect_single(self) -> Address:
@dataclass(frozen=True)
class AddressWithOrigin:
"""A BuildFileAddress along with the cmd-line spec it was generated from."""
address: BuildFileAddress
address: Address
origin: Union[AddressSpec, FilesystemResolvedSpec]


Expand All @@ -40,10 +40,7 @@ class AddressesWithOrigins(Collection[AddressWithOrigin]):


class BuildFileAddresses(Collection[BuildFileAddress]):
@property
def addresses(self) -> List[Address]:
"""Converts the BuildFileAddress objects in this collection to Address objects."""
return [bfa.to_address() for bfa in self]
"""NB: V2 should generally use Addresses instead of BuildFileAddresses."""


class NotSerializableError(TypeError):
Expand Down
37 changes: 17 additions & 20 deletions src/python/pants/engine/build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ async def find_build_file(address: Address) -> BuildFileAddress:
)


@rule
async def find_build_files(addresses: Addresses) -> BuildFileAddresses:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know which codepath this will still be used in? Will probably want to confirm that we don't see a performance impact.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used solely for LegacyAddressMapper, which is used by filter I think?

bfas = await MultiGet(Get[BuildFileAddress](Address, address) for address in addresses)
return BuildFileAddresses(bfas)


@rule
async def hydrate_struct(address_mapper: AddressMapper, address: Address) -> HydratedStruct:
"""Given an AddressMapper and an Address, resolve a Struct from a BUILD file.
Expand Down Expand Up @@ -183,7 +189,7 @@ def consume_dependencies(item, args=None):
hydrated_args[key] = maybe_consume(key, value)
return _hydrate(type(item), address.spec_path, **hydrated_args)

return HydratedStruct(consume_dependencies(struct, args={"address": build_file_address}))
return HydratedStruct(consume_dependencies(struct, args={"address": address}))


def _hydrate(item_type, spec_path, **kwargs):
Expand Down Expand Up @@ -225,7 +231,7 @@ async def addresses_with_origins_from_address_families(
address_family_by_directory = {af.namespace: af for af in address_families}

matched_addresses = OrderedSet()
bfaddr_to_origin: Dict[BuildFileAddress, AddressSpec] = {}
addr_to_origin: Dict[Address, AddressSpec] = {}

for address_spec in address_specs:
# NB: if an address spec is provided which expands to some number of targets, but those targets
Expand All @@ -242,31 +248,30 @@ async def addresses_with_origins_from_address_families(
addr_families_for_spec
)
for bfaddr, _ in all_bfaddr_tgt_pairs:
addr = bfaddr.to_address()
# A target might be covered by multiple specs, so we take the most specific one.
bfaddr_to_origin[bfaddr] = more_specific(bfaddr_to_origin.get(bfaddr), address_spec)
addr_to_origin[addr] = more_specific(addr_to_origin.get(addr), address_spec)
except AddressSpec.AddressResolutionError as e:
raise AddressLookupError(e) from e
except SingleAddress._SingleAddressResolutionError as e:
_raise_did_you_mean(e.single_address_family, e.name, source=e)

matched_addresses.update(
bfaddr
bfaddr.to_address()
for (bfaddr, tgt) in all_bfaddr_tgt_pairs
if address_specs.matcher.matches_target_address_pair(bfaddr, tgt)
)

# NB: This may be empty, as the result of filtering by tag and exclude patterns!
return AddressesWithOrigins(
AddressWithOrigin(address=bfaddr, origin=bfaddr_to_origin[bfaddr])
for bfaddr in matched_addresses
AddressWithOrigin(address=addr, origin=addr_to_origin[addr])
for addr in matched_addresses
)


@rule
def strip_address_origins(addresses_with_origins: AddressesWithOrigins) -> BuildFileAddresses:
return BuildFileAddresses(
address_with_origin.address for address_with_origin in addresses_with_origins
)
def strip_address_origins(addresses_with_origins: AddressesWithOrigins) -> Addresses:
return Addresses(address_with_origin.address for address_with_origin in addresses_with_origins)


@dataclass(frozen=True)
Expand All @@ -281,18 +286,12 @@ def is_single_address(self, address: Address) -> bool:
def address_origin_map(addresses_with_origins: AddressesWithOrigins) -> AddressOriginMap:
return AddressOriginMap(
addr_to_origin={
address_with_origin.address.to_address(): address_with_origin.origin
address_with_origin.address: address_with_origin.origin
for address_with_origin in addresses_with_origins
}
)


# TODO(#6657): Remove this once BFA is removed.
@rule
def bfas_to_addresses(build_file_addresses: BuildFileAddresses) -> Addresses:
return Addresses(bfa.to_address() for bfa in build_file_addresses)


def _address_spec_to_globs(address_mapper: AddressMapper, address_specs: AddressSpecs) -> PathGlobs:
"""Given an AddressSpecs object, return a PathGlobs object for the build files that it matches."""
patterns = set()
Expand All @@ -314,15 +313,13 @@ def address_mapper_singleton() -> AddressMapper:
hydrate_struct,
parse_address_family,
find_build_file,
find_build_files,
# AddressSpec handling: locate directories that contain build files, and request
# AddressFamilies for each of them.
addresses_with_origins_from_address_families,
strip_address_origins,
address_origin_map,
bfas_to_addresses,
# Root rules representing parameters that might be provided via root subjects.
RootRule(Address),
RootRule(BuildFileAddress),
RootRule(BuildFileAddresses),
RootRule(AddressSpecs),
]
Loading