Skip to content

Commit

Permalink
Use TransitiveTargetsRequest as input for resolving `TransitiveTarg…
Browse files Browse the repository at this point in the history
…ets` (pantsbuild#10915)

### Problem

As explained in pantsbuild#10888, we are not properly handling "special-cased" dependencies-like fields, such as `python_tests`'s `runtime_package_dependencies` field.

We need those fields' dependencies to show up in project introspection, such as `./pants dependencies`, but _not_ in other contexts like when resolving the transitive targets to determine what source files are used by Pytest.

That is, we need to be able to parametrize `await Get(TransitiveTargets)` so call sites can determine the behavior they want.

### Solution

Add `TransitiveTargetsRequest`, which will allow us to add new fields to it to change its behavior. This mirrors how we have `DependenciesRequest` for direct deps.
 
Note that this means you can no longer request `TransitiveTargets` as a parameter to your rule and have it be evaluated automatically by looking at the `Specs`. You must now request `Addresses`, and then use `await Get`. There's an argument for this being a good thing. It's a bit confusing (imo) how some types like `Addresses` can be in a rule signature and will be evaluated by using the Specs. This means that you can now only do that with `Addresses` and `Targets`, but not `TransitiveTargets`.

To work around pantsbuild#10917, we add a `DependenciesRequestLite` and `TransitiveTargetsLite` for use with codegen. These behave almost the same, but don't work with dep inference.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Oct 7, 2020
1 parent ee50791 commit ff02ef0
Show file tree
Hide file tree
Showing 16 changed files with 325 additions and 54 deletions.
16 changes: 13 additions & 3 deletions src/python/pants/backend/codegen/protobuf/python/rules.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pathlib import PurePath

from pants.backend.codegen.protobuf.protoc import Protoc
Expand All @@ -9,7 +10,6 @@
from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest
from pants.core.util_rules.source_files import SourceFilesRequest
from pants.core.util_rules.stripped_source_files import StrippedSourceFiles
from pants.engine.addresses import Addresses
from pants.engine.fs import (
AddPrefix,
CreateDigest,
Expand All @@ -22,7 +22,13 @@
from pants.engine.platform import Platform
from pants.engine.process import Process, ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import GeneratedSources, GenerateSourcesRequest, Sources, TransitiveTargets
from pants.engine.target import (
GeneratedSources,
GenerateSourcesRequest,
Sources,
TransitiveTargets,
TransitiveTargetsRequestLite,
)
from pants.engine.unions import UnionRule
from pants.source.source_root import SourceRoot, SourceRootRequest
from pants.util.logging import LogLevel
Expand All @@ -47,7 +53,11 @@ async def generate_python_from_protobuf(
# Protoc needs all transitive dependencies on `protobuf_libraries` to work properly. It won't
# actually generate those dependencies; it only needs to look at their .proto files to work
# with imports.
transitive_targets = await Get(TransitiveTargets, Addresses([request.protocol_target.address]))
# TODO(#10917): Use TransitiveTargets instead of TransitiveTargetsLite.
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequestLite([request.protocol_target.address])
)

# NB: By stripping the source roots, we avoid having to set the value `--proto_path`
# for Protobuf imports to be discoverable.
all_stripped_sources_request = Get(
Expand Down
10 changes: 8 additions & 2 deletions src/python/pants/backend/project_info/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@
from pants.engine.goal import Goal, GoalSubsystem, LineOriented
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import Dependencies as DependenciesField
from pants.engine.target import DependenciesRequest, Targets, TransitiveTargets, UnexpandedTargets
from pants.engine.target import (
DependenciesRequest,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
UnexpandedTargets,
)


class DependencyType(Enum):
Expand Down Expand Up @@ -64,7 +70,7 @@ async def dependencies(
console: Console, addresses: Addresses, dependencies_subsystem: DependenciesSubsystem
) -> Dependencies:
if dependencies_subsystem.transitive:
transitive_targets = await Get(TransitiveTargets, Addresses, addresses)
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(addresses))
targets = Targets(transitive_targets.dependencies)
else:
target_roots = await Get(UnexpandedTargets, Addresses, addresses)
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/backend/project_info/filedeps.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
Target,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
UnexpandedTargets,
)

Expand Down Expand Up @@ -83,7 +84,7 @@ async def file_deps(
) -> Filedeps:
targets: Iterable[Target]
if filedeps_subsystem.transitive:
transitive_targets = await Get(TransitiveTargets, Addresses, addresses)
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(addresses))
targets = transitive_targets.closure
elif filedeps_subsystem.globs:
targets = await Get(UnexpandedTargets, Addresses, addresses)
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/backend/python/goals/coverage_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
CoverageReports,
FilesystemCoverageReport,
)
from pants.engine.addresses import Address
from pants.engine.addresses import Address, Addresses
from pants.engine.fs import (
AddPrefix,
CreateDigest,
Expand All @@ -42,7 +42,7 @@
)
from pants.engine.process import ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import TransitiveTargets
from pants.engine.target import TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionRule
from pants.option.custom_types import file_option
from pants.util.logging import LogLevel
Expand Down Expand Up @@ -270,9 +270,10 @@ async def generate_coverage_reports(
coverage_setup: CoverageSetup,
coverage_config: CoverageConfig,
coverage_subsystem: CoverageSubsystem,
transitive_targets: TransitiveTargets,
all_used_addresses: Addresses,
) -> CoverageReports:
"""Takes all Python test results and generates a single coverage report."""
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(all_used_addresses))
sources = await Get(
PythonSourceFiles,
PythonSourceFilesRequest(transitive_targets.closure, include_resources=False),
Expand Down
11 changes: 6 additions & 5 deletions src/python/pants/backend/python/goals/pytest_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
TestSubsystem,
)
from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
from pants.engine.addresses import Addresses, UnparsedAddressInputs
from pants.engine.addresses import UnparsedAddressInputs
from pants.engine.fs import AddPrefix, Digest, DigestSubset, MergeDigests, PathGlobs, Snapshot
from pants.engine.process import FallibleProcessResult, InteractiveProcess, Process
from pants.engine.rules import Get, MultiGet, collect_rules, rule
Expand All @@ -50,6 +50,7 @@
FieldSetsPerTargetRequest,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
)
from pants.engine.unions import UnionRule
from pants.option.global_options import GlobalOptions
Expand Down Expand Up @@ -103,9 +104,9 @@ async def setup_pytest_for_target(
test_extra_env: TestExtraEnv,
global_options: GlobalOptions,
) -> TestSetup:
test_addresses = Addresses((request.field_set.address,))

transitive_targets = await Get(TransitiveTargets, Addresses, test_addresses)
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequest([request.field_set.address])
)
all_targets = transitive_targets.closure

interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
Expand All @@ -121,7 +122,7 @@ async def setup_pytest_for_target(
requirements_pex_request = Get(
Pex,
PexFromTargetsRequest,
PexFromTargetsRequest.for_requirements(test_addresses, internal_only=True),
PexFromTargetsRequest.for_requirements([request.field_set.address], internal_only=True),
)

pytest_pex_request = Get(
Expand Down
5 changes: 2 additions & 3 deletions src/python/pants/backend/python/goals/run_python_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@
PythonSourceFilesRequest,
)
from pants.core.goals.run import RunFieldSet, RunRequest
from pants.engine.addresses import Addresses
from pants.engine.fs import Digest, MergeDigests, PathGlobs, Paths
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import InvalidFieldException, TransitiveTargets
from pants.engine.target import InvalidFieldException, TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionRule
from pants.option.global_options import FilesNotFoundBehavior
from pants.source.source_root import SourceRoot, SourceRootRequest
Expand Down Expand Up @@ -49,7 +48,7 @@ async def create_python_binary_run_request(
entry_point = PythonBinarySources.translate_source_file_to_entry_point(
os.path.relpath(entry_point_path, source_root.path)
)
transitive_targets = await Get(TransitiveTargets, Addresses([field_set.address]))
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([field_set.address]))

# Note that we get an intermediate PexRequest here (instead of going straight to a Pex)
# so that we can get the interpreter constraints for use in runner_pex_request.
Expand Down
21 changes: 14 additions & 7 deletions src/python/pants/backend/python/goals/setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
)
from pants.core.target_types import FilesSources, ResourcesSources
from pants.core.util_rules.distdir import DistDir
from pants.engine.addresses import Address, Addresses, UnparsedAddressInputs
from pants.engine.addresses import Address, UnparsedAddressInputs
from pants.engine.collection import Collection, DeduplicatedCollection
from pants.engine.fs import (
AddPrefix,
Expand All @@ -65,6 +65,7 @@
Targets,
TargetsWithOrigins,
TransitiveTargets,
TransitiveTargetsRequest,
)
from pants.engine.unions import UnionMembership, union
from pants.option.custom_types import shell_str
Expand Down Expand Up @@ -395,7 +396,8 @@ async def run_setup_pys(
if setup_py_subsystem.transitive:
# Expand out to all owners of the entire dep closure.
transitive_targets = await Get(
TransitiveTargets, Addresses(et.target.address for et in exported_targets)
TransitiveTargets,
TransitiveTargetsRequest(et.target.address for et in exported_targets),
)
owners = await MultiGet(
Get(ExportedTarget, OwnedDependency(tgt))
Expand Down Expand Up @@ -521,7 +523,7 @@ async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot:

owned_deps, transitive_targets = await MultiGet(
Get(OwnedDependencies, DependencyOwner(exported_target)),
Get(TransitiveTargets, Addresses([exported_target.target.address])),
Get(TransitiveTargets, TransitiveTargetsRequest([exported_target.target.address])),
)

# files() targets aren't owned by a single exported target - they aren't code, so
Expand Down Expand Up @@ -632,7 +634,7 @@ async def get_requirements(
dep_owner: DependencyOwner, union_membership: UnionMembership
) -> ExportedTargetRequirements:
transitive_targets = await Get(
TransitiveTargets, Addresses([dep_owner.exported_target.target.address])
TransitiveTargets, TransitiveTargetsRequest([dep_owner.exported_target.target.address])
)

ownable_tgts = [
Expand Down Expand Up @@ -689,7 +691,8 @@ async def get_owned_dependencies(
Includes dependency_owner itself.
"""
transitive_targets = await Get(
TransitiveTargets, Addresses([dependency_owner.exported_target.target.address])
TransitiveTargets,
TransitiveTargetsRequest([dependency_owner.exported_target.target.address]),
)
ownable_targets = [
tgt for tgt in transitive_targets.closure if is_ownable_target(tgt, union_membership)
Expand Down Expand Up @@ -729,15 +732,19 @@ async def get_exporting_owner(owned_dependency: OwnedDependency) -> ExportedTarg
)
exported_ancestor_iter = iter(exported_ancestor_tgts)
for exported_ancestor in exported_ancestor_iter:
transitive_targets = await Get(TransitiveTargets, Addresses([exported_ancestor.address]))
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequest([exported_ancestor.address])
)
if target in transitive_targets.closure:
owner = exported_ancestor
# Find any exported siblings of owner that also depend on 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:
transitive_targets = await Get(TransitiveTargets, Addresses([sibling.address]))
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequest([sibling.address])
)
if target in transitive_targets.closure:
sibling_owners.append(sibling)
sibling = next(exported_ancestor_iter, None)
Expand Down
5 changes: 4 additions & 1 deletion src/python/pants/backend/python/lint/pylint/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
Target,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
)
from pants.engine.unions import UnionRule
from pants.python.python_setup import PythonSetup
Expand Down Expand Up @@ -233,7 +234,9 @@ async def pylint_lint(
return LintResults([], linter_name="Pylint")

plugin_target_addresses = await Get(Addresses, UnparsedAddressInputs, pylint.source_plugins)
plugin_targets_request = Get(TransitiveTargets, Addresses(plugin_target_addresses))
plugin_targets_request = Get(
TransitiveTargets, TransitiveTargetsRequest(plugin_target_addresses)
)
linted_targets_request = Get(
Targets, Addresses(field_set.address for field_set in request.field_sets)
)
Expand Down
9 changes: 6 additions & 3 deletions src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
)
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import FieldSet, Target, TransitiveTargets
from pants.engine.target import FieldSet, Target, TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionRule
from pants.python.python_setup import PythonSetup
from pants.util.logging import LogLevel
Expand Down Expand Up @@ -187,7 +187,9 @@ def determine_python_files(files: Iterable[str]) -> Tuple[str, ...]:
@rule
async def mypy_typecheck_partition(partition: MyPyPartition, mypy: MyPy) -> TypecheckResult:
plugin_target_addresses = await Get(Addresses, UnparsedAddressInputs, mypy.source_plugins)
plugin_transitive_targets_request = Get(TransitiveTargets, Addresses(plugin_target_addresses))
plugin_transitive_targets_request = Get(
TransitiveTargets, TransitiveTargetsRequest(plugin_target_addresses)
)
plugin_transitive_targets, launcher_script = await MultiGet(
plugin_transitive_targets_request, Get(Digest, CreateDigest([LAUNCHER_FILE]))
)
Expand Down Expand Up @@ -349,7 +351,8 @@ async def mypy_typecheck(
# transitive closure to get the final resulting constraints.
# TODO(#10863): Improve the performance of this.
transitive_targets_per_field_set = await MultiGet(
Get(TransitiveTargets, Addresses([field_set.address])) for field_set in request.field_sets
Get(TransitiveTargets, TransitiveTargetsRequest([field_set.address]))
for field_set in request.field_sets
)

interpreter_constraints_to_transitive_targets = defaultdict(set)
Expand Down
12 changes: 10 additions & 2 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@
PathGlobs,
)
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import Dependencies, DependenciesRequest, Targets, TransitiveTargets
from pants.engine.target import (
Dependencies,
DependenciesRequest,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
)
from pants.python.python_setup import PythonSetup, ResolveAllConstraintsOption
from pants.util.logging import LogLevel
from pants.util.meta import frozen_after_init
Expand Down Expand Up @@ -184,7 +190,9 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
)
all_targets = FrozenOrderedSet(itertools.chain(*direct_deps, targets))
else:
transitive_targets = await Get(TransitiveTargets, Addresses, request.addresses)
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequest(request.addresses)
)
all_targets = transitive_targets.closure

input_digests = []
Expand Down
9 changes: 7 additions & 2 deletions src/python/pants/core/goals/repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
from typing import ClassVar, Dict, Iterable, Mapping, Optional, Tuple, Type, cast

from pants.base.build_root import BuildRoot
from pants.engine.addresses import Addresses
from pants.engine.console import Console
from pants.engine.fs import Digest, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import InteractiveProcess, InteractiveRunner
from pants.engine.rules import Get, collect_rules, goal_rule
from pants.engine.target import Targets, TransitiveTargets
from pants.engine.target import Targets, TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionMembership, union
from pants.option.global_options import GlobalOptions
from pants.util.contextutil import temporary_dir
Expand Down Expand Up @@ -86,11 +87,15 @@ async def run_repl(
workspace: Workspace,
interactive_runner: InteractiveRunner,
repl_subsystem: ReplSubsystem,
transitive_targets: TransitiveTargets,
all_specified_addresses: Addresses,
build_root: BuildRoot,
union_membership: UnionMembership,
global_options: GlobalOptions,
) -> Repl:
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequest(all_specified_addresses)
)

# TODO: When we support multiple languages, detect the default repl to use based
# on the targets. For now we default to the python repl.
repl_shell_name = repl_subsystem.shell or "python"
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/core/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class RelocateFilesViaCodegenRequest(GenerateSourcesRequest):
async def relocate_files(request: RelocateFilesViaCodegenRequest) -> GeneratedSources:
# Unlike normal codegen, we operate the on the sources of the `files_targets` field, not the
# `sources` of the original `relocated_sources` target.
# TODO: using `await Get(Addresses, UnparsedAddressInputs)` causes a graph failure.
# TODO(#10915): using `await Get(Addresses, UnparsedAddressInputs)` causes a graph failure.
original_files_targets = await MultiGet(
Get(
WrappedTarget,
Expand Down
Loading

0 comments on commit ff02ef0

Please sign in to comment.