Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 2 additions & 0 deletions docs/notes/2.32.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ The version of [Pex](https://github.com/pex-tool/pex) used by the Python backend

When generating lockfiles, the new `python.resolves_to_uploaded_prior_to` option can be used to pass along [`--uploaded-prior-to`](https://pip.pypa.io/en/stable/user_guide/#filtering-by-upload-time). This allows you to filter packages by their upload time to an index, only considering packages that were uploaded before a specified point in time.

Including a generic `target` in the `runtime_package_dependencies` of `python_test` now correctly packages the transitive dependencies.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is a matter of correctness - it's more that we moved the goalposts (to a hopefully better place). But I wouldn't classify the previous behavior as a bug, more as an over-literal interpretation of direct dependencies...

Comment thread
benjyw marked this conversation as resolved.
Outdated

#### Shell

#### Javascript
Expand Down
39 changes: 37 additions & 2 deletions src/python/pants/core/goals/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
environment_aware_package,
)
from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
from pants.core.target_types import GenericTarget
from pants.core.util_rules.distdir import DistDir
from pants.core.util_rules.env_vars import environment_vars_subset
from pants.core.util_rules.partitions import (
Expand All @@ -47,7 +48,7 @@
from pants.engine.env_vars import EXTRA_ENV_VARS_USAGE_HELP, EnvironmentVars, EnvironmentVarsRequest
from pants.engine.fs import EMPTY_FILE_DIGEST, FileDigest, MergeDigests, Snapshot, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.internals.graph import find_valid_field_sets, resolve_targets
from pants.engine.internals.graph import find_valid_field_sets, resolve_targets, transitive_targets
from pants.engine.internals.session import RunId
from pants.engine.internals.specs_rules import find_valid_field_sets_for_target_roots
from pants.engine.intrinsics import merge_digests, run_interactive_process_in_environment
Expand All @@ -59,16 +60,21 @@
)
from pants.engine.rules import collect_rules, concurrently, goal_rule, implicitly, rule
from pants.engine.target import (
Dependencies,
DepsTraversalBehavior,
FieldSet,
FieldSetsPerTargetRequest,
IntField,
NoApplicableTargetsBehavior,
ShouldTraverseDepsPredicate,
SourcesField,
SpecialCasedDependencies,
StringField,
StringSequenceField,
Target,
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest,
TransitiveTargetsRequest,
ValidNumbers,
parse_shard_spec,
)
Expand Down Expand Up @@ -1240,6 +1246,21 @@ class RuntimePackageDependenciesField(SpecialCasedDependencies):
)


class TraverseGenericTargetDepsOnly(ShouldTraverseDepsPredicate):
"""Traverses deps of `target()` (GenericTarget) entries, stops at all other target types.

Used to unwrap a `target()` alias that groups packageable targets, so that
`runtime_package_dependencies` can reference the alias instead of each target individually.
"""

def __call__(
self, target: Target, field: Dependencies | SpecialCasedDependencies
) -> DepsTraversalBehavior:
if isinstance(target, GenericTarget) and isinstance(field, Dependencies):
return DepsTraversalBehavior.INCLUDE
return DepsTraversalBehavior.EXCLUDE


class BuiltPackageDependencies(Collection[BuiltPackage]):
pass

Expand All @@ -1257,8 +1278,22 @@ async def build_runtime_package_dependencies(
if not unparsed_addresses:
return BuiltPackageDependencies()
tgts = await resolve_targets(**implicitly(unparsed_addresses))

# Unwrap GenericTarget ("target()") entries by traversing their deps transitively,
# stopping at non-GenericTarget targets. This lets callers group packageable targets
# under a single `target()` alias and reference that alias in
# runtime_package_dependencies, rather than listing each packageable target individually.
transitive = await transitive_targets(
TransitiveTargetsRequest(
[tgt.address for tgt in tgts],
should_traverse_deps_predicate=TraverseGenericTargetDepsOnly(),
),
**implicitly(),
)
non_generic = [tgt for tgt in transitive.closure if not isinstance(tgt, GenericTarget)]

field_sets_per_tgt = await find_valid_field_sets(
FieldSetsPerTargetRequest(PackageFieldSet, tgts), **implicitly()
FieldSetsPerTargetRequest(PackageFieldSet, non_generic), **implicitly()
)
packages = await concurrently(
environment_aware_package(EnvironmentAwarePackageRequest(field_set))
Expand Down
39 changes: 39 additions & 0 deletions src/python/pants/core/goals/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
run_tests,
)
from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
from pants.core.target_types import GenericTarget
from pants.core.util_rules.distdir import DistDir
from pants.core.util_rules.partitions import Partition, Partitions
from pants.engine.addresses import Address
Expand Down Expand Up @@ -754,6 +755,44 @@ def test_runtime_package_dependencies() -> None:
assert snapshot.files == ("src.py/main.pex",)


def test_runtime_package_dependencies_via_generic_target() -> None:
"""A `target()` listed in runtime_package_dependencies should be unwrapped so that its
packageable dependencies are built, rather than being silently ignored."""
rule_runner = PythonRuleRunner(
rules=[
build_runtime_package_dependencies,
*pex_from_targets.rules(),
*package_pex_binary.rules(),
*python_target_type_rules(),
QueryRule(BuiltPackageDependencies, [BuildPackageDependenciesRequest]),
],
target_types=[PythonSourcesGeneratorTarget, PexBinary, GenericTarget],
)
rule_runner.set_options(args=[], env_inherit={"PATH", "PYENV_ROOT", "HOME"})

rule_runner.write_files(
{
"src/py/main_a.py": "",
"src/py/main_b.py": "",
"src/py/BUILD": dedent(
"""\
pex_binary(name='bin_a', entry_point='main_a.py')
pex_binary(name='bin_b', entry_point='main_b.py')
target(
name='all_bins',
dependencies=[':bin_a', ':bin_b'],
)
"""
),
}
)
input_field = RuntimePackageDependenciesField(["src/py:all_bins"], Address("fake"))
result = rule_runner.request(
BuiltPackageDependencies, [BuildPackageDependenciesRequest(input_field)]
)
assert len(result) == 2


def test_timeout_calculation() -> None:
def assert_timeout_calculated(
*,
Expand Down
Loading