diff --git a/src/python/pants/backend/awslambda/python/rules_test.py b/src/python/pants/backend/awslambda/python/rules_test.py index 26312caffb9..9f8afe48f9e 100644 --- a/src/python/pants/backend/awslambda/python/rules_test.py +++ b/src/python/pants/backend/awslambda/python/rules_test.py @@ -252,7 +252,8 @@ def handler(event, context): assert caplog.records assert "src.py.project/lambda.zip" == zip_file_relpath assert ( - "The `python_awslambda` target src/py/project:lambda transitively depends on" in caplog.text + "The target src/py/project:lambda (`python_awslambda`) transitively depends on" + in caplog.text ) assert "assets/f.txt:files" in caplog.text assert "assets:relocated" in caplog.text diff --git a/src/python/pants/backend/google_cloud_function/python/rules_test.py b/src/python/pants/backend/google_cloud_function/python/rules_test.py index e74336ce26c..7db7d99b5b0 100644 --- a/src/python/pants/backend/google_cloud_function/python/rules_test.py +++ b/src/python/pants/backend/google_cloud_function/python/rules_test.py @@ -237,7 +237,7 @@ def handler(event, context): assert caplog.records assert "src.py.project/lambda.zip" == zip_file_relpath assert ( - "The `python_google_cloud_function` target src/py/project:lambda transitively depends on" + "The target src/py/project:lambda (`python_google_cloud_function`) transitively depends on" in caplog.text ) assert "assets/f.txt:files" in caplog.text diff --git a/src/python/pants/backend/python/goals/package_pex_binary.py b/src/python/pants/backend/python/goals/package_pex_binary.py index d98788a3150..d8944bef8c0 100644 --- a/src/python/pants/backend/python/goals/package_pex_binary.py +++ b/src/python/pants/backend/python/goals/package_pex_binary.py @@ -40,19 +40,11 @@ PackageFieldSet, ) from pants.core.goals.run import RunFieldSet, RunInSandboxBehavior -from pants.core.target_types import FileSourceField from pants.core.util_rules.environments import EnvironmentField -from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import ( - TransitiveTargets, - TransitiveTargetsRequest, - targets_with_sources_types, -) -from pants.engine.unions import UnionMembership, UnionRule -from pants.util.docutil import doc_url +from pants.engine.rules import Get, collect_rules, rule +from pants.engine.unions import UnionRule from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel -from pants.util.strutil import softwrap logger = logging.getLogger(__name__) @@ -131,44 +123,11 @@ class PexFromTargetsRequestForBuiltPackage: async def package_pex_binary( field_set: PexBinaryFieldSet, pex_binary_defaults: PexBinaryDefaults, - union_membership: UnionMembership, ) -> PexFromTargetsRequestForBuiltPackage: - resolved_pex_entry_point_get = Get( + resolved_entry_point = await Get( ResolvedPexEntryPoint, ResolvePexEntryPointRequest(field_set.entry_point) ) - # If sources are included, then we need to check dependencies for `files` targets. - if not field_set.include_sources.value: - resolved_entry_point = await resolved_pex_entry_point_get - else: - resolved_entry_point, transitive_targets = await MultiGet( - resolved_pex_entry_point_get, - Get(TransitiveTargets, TransitiveTargetsRequest([field_set.address])), - ) - - # Warn if users depend on `files` targets, which won't be included in the PEX and is a common - # gotcha. - file_tgts = targets_with_sources_types( - [FileSourceField], transitive_targets.dependencies, union_membership - ) - if file_tgts: - files_addresses = sorted(tgt.address.spec for tgt in file_tgts) - logger.warning( - softwrap( - f""" - The `pex_binary` target {field_set.address} transitively depends on the below `files` - targets, but Pants will not include them in the PEX. Filesystem APIs like `open()` - are not able to load files within the binary itself; instead, they read from the - current working directory. - - Instead, use `resources` targets or wrap this `pex_binary` in an `archive`. - See {doc_url('resources')}. - - Files targets dependencies: {files_addresses} - """ - ) - ) - output_filename = field_set.output_path.value_or_default(file_ending="pex") complete_platforms = await Get( @@ -189,6 +148,7 @@ async def package_pex_binary( include_requirements=field_set.include_requirements.value, include_source_files=field_set.include_sources.value, include_local_dists=True, + warn_for_transitive_files_targets=True, ) return PexFromTargetsRequestForBuiltPackage(request) diff --git a/src/python/pants/backend/python/goals/package_pex_binary_integration_test.py b/src/python/pants/backend/python/goals/package_pex_binary_integration_test.py index 70f6918b86e..321ca3b177d 100644 --- a/src/python/pants/backend/python/goals/package_pex_binary_integration_test.py +++ b/src/python/pants/backend/python/goals/package_pex_binary_integration_test.py @@ -105,7 +105,7 @@ def test_warn_files_targets(rule_runner: PythonRuleRunner, caplog) -> None: assert not caplog.records result = rule_runner.request(BuiltPackage, [field_set]) assert caplog.records - assert f"The `pex_binary` target {tgt.address} transitively depends on" in caplog.text + assert f"The target {tgt.address} (`pex_binary`) transitively depends on" in caplog.text assert "assets/f.txt:files" in caplog.text assert "assets:relocated" in caplog.text assert "assets:resources" not in caplog.text diff --git a/src/python/pants/backend/python/util_rules/faas.py b/src/python/pants/backend/python/util_rules/faas.py index 21ff0ecd99b..d9eae29346b 100644 --- a/src/python/pants/backend/python/util_rules/faas.py +++ b/src/python/pants/backend/python/util_rules/faas.py @@ -40,7 +40,6 @@ from pants.backend.python.util_rules.pex_venv import PexVenv, PexVenvLayout, PexVenvRequest from pants.backend.python.util_rules.pex_venv import rules as pex_venv_rules from pants.core.goals.package import BuiltPackage, BuiltPackageArtifact, OutputPathField -from pants.core.target_types import FileSourceField from pants.engine.addresses import Address, UnparsedAddressInputs from pants.engine.fs import ( CreateDigest, @@ -66,12 +65,11 @@ StringField, TransitiveTargets, TransitiveTargetsRequest, - targets_with_sources_types, ) -from pants.engine.unions import UnionMembership, UnionRule +from pants.engine.unions import UnionRule from pants.source.filespec import Filespec from pants.source.source_root import SourceRoot, SourceRootRequest -from pants.util.docutil import bin_name, doc_url +from pants.util.docutil import bin_name from pants.util.strutil import help_text logger = logging.getLogger(__name__) @@ -312,7 +310,6 @@ async def build_lambdex( request: BuildLambdexRequest, lambdex: Lambdex, platform: Platform, - union_membership: UnionMembership, ) -> BuiltPackage: if platform.is_macos: logger.warning( @@ -354,6 +351,7 @@ async def build_lambdex( complete_platforms=complete_platforms, additional_args=additional_pex_args, additional_lockfile_args=additional_pex_args, + warn_for_transitive_files_targets=True, ) lambdex_request = lambdex.to_pex_request() @@ -364,22 +362,6 @@ async def build_lambdex( Get(TransitiveTargets, TransitiveTargetsRequest([request.address])), ) - # Warn if users depend on `files` targets, which won't be included in the PEX and is a common - # gotcha. - file_tgts = targets_with_sources_types( - [FileSourceField], transitive_targets.dependencies, union_membership - ) - if file_tgts: - files_addresses = sorted(tgt.address.spec for tgt in file_tgts) - logger.warning( - f"The `{request.target_name}` target {request.address} transitively depends " - "on the below `files` targets, but Pants will not include them in the built package. " - "Filesystem APIs like `open()` are not able to load files within the binary " - "itself; instead, they read from the current working directory." - f"\n\nInstead, use `resources` targets. See {doc_url('resources')}." - f"\n\nFiles targets dependencies: {files_addresses}" - ) - lambdex_args = ["build", "-e", f"{handler.module}:{handler.func}", output_filename] if request.script_handler: lambdex_args.extend(("-H", request.script_handler)) @@ -478,6 +460,7 @@ async def build_python_faas( additional_args=additional_pex_args, additional_lockfile_args=additional_pex_args, additional_sources=additional_sources, + warn_for_transitive_files_targets=True, ) pex_result = await Get(Pex, PexFromTargetsRequest, pex_request) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets.py b/src/python/pants/backend/python/util_rules/pex_from_targets.py index b9fa367653e..43bbf8761e2 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets.py @@ -46,11 +46,19 @@ ) from pants.backend.python.util_rules.python_sources import rules as python_sources_rules from pants.core.goals.generate_lockfiles import NoCompatibleResolveException +from pants.core.target_types import FileSourceField from pants.engine.addresses import Address, Addresses from pants.engine.collection import DeduplicatedCollection from pants.engine.fs import Digest, DigestContents, GlobMatchErrorBehavior, MergeDigests, PathGlobs from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import Target, TransitiveTargets, TransitiveTargetsRequest +from pants.engine.target import ( + Target, + Targets, + TransitiveTargets, + TransitiveTargetsRequest, + targets_with_sources_types, +) +from pants.engine.unions import UnionMembership from pants.util.docutil import doc_url from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel @@ -78,6 +86,7 @@ class PexFromTargetsRequest: additional_sources: Digest | None additional_inputs: Digest | None hardcoded_interpreter_constraints: InterpreterConstraints | None + warn_for_transitive_files_targets: bool # This field doesn't participate in comparison (and therefore hashing), as it doesn't affect # the result. description: str | None = dataclasses.field(compare=False) @@ -103,6 +112,7 @@ def __init__( additional_inputs: Digest | None = None, hardcoded_interpreter_constraints: InterpreterConstraints | None = None, description: str | None = None, + warn_for_transitive_files_targets: bool = False, ) -> None: """Request to create a Pex from the transitive closure of the given addresses. @@ -140,6 +150,8 @@ def __init__( constraints from the input. :param description: A human-readable description to render in the dynamic UI when building the Pex. + :param warn_for_transitive_files_targets: If True (and include_source_files is also true), + emit a warning if the pex depends on any `files` targets, since they won't be included. """ object.__setattr__(self, "addresses", Addresses(addresses)) object.__setattr__(self, "output_filename", output_filename) @@ -161,6 +173,9 @@ def __init__( self, "hardcoded_interpreter_constraints", hardcoded_interpreter_constraints ) object.__setattr__(self, "description", description) + object.__setattr__( + self, "warn_for_transitive_files_targets", warn_for_transitive_files_targets + ) self.__post_init__() @@ -473,9 +488,40 @@ async def _determine_requirements_for_pex_from_targets( return dataclasses.replace(requirements, from_superset=repository_pex.maybe_pex), () +async def _warn_about_any_files_targets( + addresses: Addresses, transitive_targets: TransitiveTargets, union_membership: UnionMembership +) -> None: + # Warn if users depend on `files` targets, which won't be included in the PEX and is a common + # gotcha. + file_tgts = targets_with_sources_types( + [FileSourceField], transitive_targets.dependencies, union_membership + ) + if file_tgts: + # make it easier for the user to find which targets are problematic by including the alias + targets = await Get(Targets, Addresses, addresses) + formatted_addresses = ", ".join( + f"{a} (`{tgt.alias}`)" for a, tgt in zip(addresses, targets) + ) + + files_addresses = sorted(tgt.address.spec for tgt in file_tgts) + targets_text, depend_text = ( + ("target", "depends") if len(addresses) == 1 else ("targets", "depend") + ) + logger.warning( + f"The {targets_text} {formatted_addresses} transitively {depend_text} " + "on the below `files` targets, but Pants will not include them in the built package. " + "Filesystem APIs like `open()` may be not able to load files within the binary " + "itself; instead, they read from the current working directory." + f"\n\nInstead, use `resources` targets. See {doc_url('resources')}." + f"\n\nFiles targets dependencies: {files_addresses}" + ) + + @rule(level=LogLevel.DEBUG) async def create_pex_from_targets( - request: PexFromTargetsRequest, python_setup: PythonSetup + request: PexFromTargetsRequest, + python_setup: PythonSetup, + union_membership: UnionMembership, ) -> PexRequest: requirements, additional_pexes = await _determine_requirements_for_pex_from_targets( request, python_setup @@ -495,6 +541,11 @@ async def create_pex_from_targets( TransitiveTargets, TransitiveTargetsRequest(request.addresses) ) sources = await Get(PythonSourceFiles, PythonSourceFilesRequest(transitive_targets.closure)) + + if request.warn_for_transitive_files_targets: + await _warn_about_any_files_targets( + request.addresses, transitive_targets, union_membership + ) else: sources = PythonSourceFiles.empty() diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 1c2af220341..6bf0738f76b 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -56,6 +56,7 @@ from pants.backend.python.util_rules.pex_test_utils import get_all_data from pants.build_graph.address import Address from pants.core.goals.generate_lockfiles import NoCompatibleResolveException +from pants.core.target_types import FileTarget, ResourceTarget from pants.engine.addresses import Addresses from pants.engine.fs import Snapshot from pants.testutil.option_util import create_subsystem @@ -85,6 +86,8 @@ def rule_runner() -> PythonRuleRunner: PythonRequirementTarget, PythonSourceTarget, PythonTestTarget, + FileTarget, + ResourceTarget, ], ) @@ -892,3 +895,39 @@ def test_lockfile_requirements_selection( assert mode == ResolveMode.pex assert isinstance(result.requirements.from_superset, Resolve) assert result.requirements.from_superset.name == "myresolve" + + +def test_warn_about_files_targets(rule_runner: PythonRuleRunner, caplog) -> None: + rule_runner.write_files( + { + "app.py": "", + "file.txt": "", + "resource.txt": "", + "BUILD": dedent( + """ + file(name="file_target", source="file.txt") + resource(name="resource_target", source="resource.txt") + python_sources(name="app", dependencies=[":file_target", ":resource_target"]) + """ + ), + } + ) + + rule_runner.request( + PexRequest, + [ + PexFromTargetsRequest( + [Address("", target_name="app")], + output_filename="app.pex", + internal_only=True, + warn_for_transitive_files_targets=True, + ) + ], + ) + + assert "The target //:app (`python_source`) transitively depends on" in caplog.text + # files are not fine: + assert "//:file_target" in caplog.text + # resources are fine: + assert "resource_target" not in caplog.text + assert "resource.txt" not in caplog.text