diff --git a/src/python/pants/backend/awslambda/python/rules.py b/src/python/pants/backend/awslambda/python/rules.py index 21e1f8adf62..65aae6eb41e 100644 --- a/src/python/pants/backend/awslambda/python/rules.py +++ b/src/python/pants/backend/awslambda/python/rules.py @@ -1,6 +1,7 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging from dataclasses import dataclass from pants.backend.awslambda.python.lambdex import Lambdex @@ -30,11 +31,20 @@ OutputPathField, PackageFieldSet, ) +from pants.core.target_types import FilesSources from pants.engine.process import ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.unions import UnionRule +from pants.engine.target import ( + TransitiveTargets, + TransitiveTargetsRequest, + targets_with_sources_types, +) +from pants.engine.unions import UnionMembership, UnionRule +from pants.util.docutil import docs_url from pants.util.logging import LogLevel +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class PythonAwsLambdaFieldSet(PackageFieldSet): @@ -47,7 +57,7 @@ class PythonAwsLambdaFieldSet(PackageFieldSet): @rule(desc="Create Python AWS Lambda", level=LogLevel.DEBUG) async def package_python_awslambda( - field_set: PythonAwsLambdaFieldSet, lambdex: Lambdex + field_set: PythonAwsLambdaFieldSet, lambdex: Lambdex, union_membership: UnionMembership ) -> BuiltPackage: output_filename = field_set.output_path.value_or_default( field_set.address, @@ -90,12 +100,29 @@ async def package_python_awslambda( main=lambdex.main, ) - lambdex_pex, pex_result, handler = await MultiGet( + lambdex_pex, pex_result, handler, transitive_targets = await MultiGet( Get(VenvPex, PexRequest, lambdex_request), Get(TwoStepPex, TwoStepPexFromTargetsRequest, pex_request), Get(ResolvedPythonAwsHandler, ResolvePythonAwsHandlerRequest(field_set.handler)), + 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. + files_tgts = targets_with_sources_types( + [FilesSources], transitive_targets.dependencies, union_membership + ) + if files_tgts: + files_addresses = sorted(tgt.address.spec for tgt in files_tgts) + logger.warning( + f"The python_awslambda target {field_set.address} transitively depends on the below " + "files targets, but Pants will not include them in the built Lambda. 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 {docs_url('resources')}." + f"\n\nFiles targets dependencies: {files_addresses}" + ) + # NB: Lambdex modifies its input pex in-place, so the input file is also the output file. result = await Get( ProcessResult, diff --git a/src/python/pants/backend/awslambda/python/rules_test.py b/src/python/pants/backend/awslambda/python/rules_test.py index 1bf25738ca6..97a4e5a8443 100644 --- a/src/python/pants/backend/awslambda/python/rules_test.py +++ b/src/python/pants/backend/awslambda/python/rules_test.py @@ -1,8 +1,8 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import textwrap from io import BytesIO +from textwrap import dedent from typing import Tuple from zipfile import ZipFile @@ -14,6 +14,8 @@ from pants.backend.awslambda.python.target_types import rules as target_rules from pants.backend.python.target_types import PythonLibrary from pants.core.goals.package import BuiltPackage +from pants.core.target_types import Files, RelocatedFiles, Resources +from pants.core.target_types import rules as core_target_types_rules from pants.engine.addresses import Address from pants.engine.fs import DigestContents from pants.testutil.rule_runner import QueryRule, RuleRunner @@ -25,9 +27,10 @@ def rule_runner() -> RuleRunner: rules=[ *awslambda_python_rules(), *target_rules(), + *core_target_types_rules(), QueryRule(BuiltPackage, (PythonAwsLambdaFieldSet,)), ], - target_types=[PythonAWSLambda, PythonLibrary], + target_types=[PythonAWSLambda, PythonLibrary, Files, RelocatedFiles, Resources], ) @@ -55,7 +58,7 @@ def create_python_awslambda(rule_runner: RuleRunner, addr: Address) -> Tuple[str def test_create_hello_world_lambda(rule_runner: RuleRunner) -> None: rule_runner.create_file( "src/python/foo/bar/hello_world.py", - textwrap.dedent( + dedent( """ def handler(event, context): print('Hello, World!') @@ -65,7 +68,7 @@ def handler(event, context): rule_runner.add_to_build_file( "src/python/foo/bar", - textwrap.dedent( + dedent( """ python_library(name='lib') @@ -87,3 +90,63 @@ def handler(event, context): names = set(zipfile.namelist()) assert "lambdex_handler.py" in names assert "foo/bar/hello_world.py" in names + + +def test_warn_files_targets(rule_runner: RuleRunner, caplog) -> None: + rule_runner.create_file("assets/f.txt") + rule_runner.add_to_build_file( + "assets", + dedent( + """\ + files(name='files', sources=['f.txt']) + relocated_files( + name='relocated', + files_targets=[':files'], + src='assets', + dest='new_assets', + ) + + # Resources are fine. + resources(name='resources', sources=['f.txt']) + """ + ), + ) + rule_runner.create_file("src/py/project/__init__.py") + rule_runner.create_file( + "src/py/project/app.py", + """\ + def handler(event, context): + print('Hello, World!') + """, + ) + rule_runner.add_to_build_file( + "src/py/project", + dedent( + """\ + python_library( + name='lib', + dependencies=['assets:files', 'assets:relocated', 'assets:resources'], + ) + + python_awslambda( + name='lambda', + dependencies=[':lib'], + handler='foo.bar.hello_world:handler', + runtime='python3.7', + ) + """ + ), + ) + + assert not caplog.records + zip_file_relpath, _ = create_python_awslambda( + rule_runner, Address("src/py/project", target_name="lambda") + ) + 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 + ) + 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/goals/package_pex_binary.py b/src/python/pants/backend/python/goals/package_pex_binary.py index 59b4b326898..ac5a92a3b2f 100644 --- a/src/python/pants/backend/python/goals/package_pex_binary.py +++ b/src/python/pants/backend/python/goals/package_pex_binary.py @@ -1,6 +1,7 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging from dataclasses import dataclass from typing import Tuple @@ -35,11 +36,20 @@ PackageFieldSet, ) from pants.core.goals.run import RunFieldSet -from pants.engine.rules import Get, collect_rules, rule -from pants.engine.unions import UnionRule +from pants.core.target_types import FilesSources +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 docs_url from pants.util.logging import LogLevel from pants.util.memo import memoized_property +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class PexBinaryFieldSet(PackageFieldSet, RunFieldSet): @@ -101,11 +111,32 @@ def generate_additional_args(self, pex_binary_defaults: PexBinaryDefaults) -> Tu @rule(level=LogLevel.DEBUG) async def package_pex_binary( - field_set: PexBinaryFieldSet, pex_binary_defaults: PexBinaryDefaults + field_set: PexBinaryFieldSet, + pex_binary_defaults: PexBinaryDefaults, + union_membership: UnionMembership, ) -> BuiltPackage: - resolved_entry_point = await Get( - ResolvedPexEntryPoint, ResolvePexEntryPointRequest(field_set.entry_point) + resolved_entry_point, transitive_targets = await MultiGet( + Get(ResolvedPexEntryPoint, ResolvePexEntryPointRequest(field_set.entry_point)), + 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. + files_tgts = targets_with_sources_types( + [FilesSources], transitive_targets.dependencies, union_membership ) + if files_tgts: + files_addresses = sorted(tgt.address.spec for tgt in files_tgts) + logger.warning( + 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." + "\n\nInstead, use `resources` targets or wrap this `pex_binary` in an `archive`. See " + f"{docs_url('resources')}." + f"\n\nFiles targets dependencies: {files_addresses}" + ) + output_filename = field_set.output_path.value_or_default(field_set.address, file_ending="pex") two_step_pex = await Get( TwoStepPex, 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 new file mode 100644 index 00000000000..17b9b3ad026 --- /dev/null +++ b/src/python/pants/backend/python/goals/package_pex_binary_integration_test.py @@ -0,0 +1,81 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +from textwrap import dedent + +import pytest + +from pants.backend.python import target_types_rules +from pants.backend.python.goals import package_pex_binary +from pants.backend.python.goals.package_pex_binary import PexBinaryFieldSet +from pants.backend.python.target_types import PexBinary +from pants.backend.python.util_rules import pex_from_targets +from pants.build_graph.address import Address +from pants.core.goals.package import BuiltPackage +from pants.core.target_types import Files, RelocatedFiles, Resources +from pants.core.target_types import rules as core_target_types_rules +from pants.testutil.rule_runner import QueryRule, RuleRunner + + +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *package_pex_binary.rules(), + *pex_from_targets.rules(), + *target_types_rules.rules(), + *core_target_types_rules(), + QueryRule(BuiltPackage, [PexBinaryFieldSet]), + ], + target_types=[PexBinary, Files, RelocatedFiles, Resources], + ) + + +def test_warn_files_targets(rule_runner: RuleRunner, caplog) -> None: + rule_runner.set_options([], env_inherit={"PATH", "PYENV_ROOT", "HOME"}) + rule_runner.create_file("assets/f.txt") + rule_runner.add_to_build_file( + "assets", + dedent( + """\ + files(name='files', sources=['f.txt']) + relocated_files( + name='relocated', + files_targets=[':files'], + src='assets', + dest='new_assets', + ) + + # Resources are fine. + resources(name='resources', sources=['f.txt']) + """ + ), + ) + rule_runner.create_file("src/py/project/__init__.py") + rule_runner.create_file("src/py/project/app.py", "print('hello')") + rule_runner.add_to_build_file( + "src/py/project", + dedent( + """\ + pex_binary( + dependencies=['assets:files', 'assets:relocated', 'assets:resources'], + entry_point="none", + ) + """ + ), + ) + tgt = rule_runner.get_target(Address("src/py/project")) + field_set = PexBinaryFieldSet.create(tgt) + + 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 "assets/f.txt:files" in caplog.text + assert "assets:relocated" in caplog.text + assert "assets:resources" not in caplog.text + + assert len(result.artifacts) == 1 + assert result.artifacts[0].relpath == "src.py.project/project.pex" diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 43d56b38173..6e24adf1e37 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -1510,6 +1510,24 @@ def filespec(self) -> Filespec: """ +def targets_with_sources_types( + sources_types: Iterable[type[Sources]], + targets: Iterable[Target], + union_membership: UnionMembership, +) -> tuple[Target, ...]: + """Return all targets either with the specified sources subclass(es) or which can generate those + sources.""" + return tuple( + tgt + for tgt in targets + if any( + tgt.has_field(sources_type) + or tgt.get(Sources).can_generate(sources_type, union_membership) + for sources_type in sources_types + ) + ) + + # ----------------------------------------------------------------------------------------------- # `Dependencies` field # ----------------------------------------------------------------------------------------------- diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py index e09cd9372b8..1dc2825ca8e 100644 --- a/src/python/pants/engine/target_test.py +++ b/src/python/pants/engine/target_test.py @@ -16,6 +16,7 @@ DictStringToStringSequenceField, Field, FieldSet, + GenerateSourcesRequest, IntField, InvalidFieldChoiceException, InvalidFieldException, @@ -30,6 +31,7 @@ Target, generate_subtarget, generate_subtarget_address, + targets_with_sources_types, ) from pants.engine.unions import UnionMembership from pants.util.frozendict import FrozenDict @@ -701,3 +703,45 @@ class ExampleDefault(DictStringToStringSequenceField): default = FrozenDict({"default": ("val",)}) assert ExampleDefault(None, address=addr).value == FrozenDict({"default": ("val",)}) + + +# ----------------------------------------------------------------------------------------------- +# Test `Sources` helper functions +# ----------------------------------------------------------------------------------------------- + + +def test_targets_with_sources_types() -> None: + class Sources1(Sources): + pass + + class Sources2(Sources): + pass + + class CodegenSources(Sources): + pass + + class Tgt1(Target): + alias = "tgt1" + core_fields = (Sources1,) + + class Tgt2(Target): + alias = "tgt2" + core_fields = (Sources2,) + + class CodegenTgt(Target): + alias = "codegen_tgt" + core_fields = (CodegenSources,) + + class GenSources(GenerateSourcesRequest): + input = CodegenSources + output = Sources1 + + tgt1 = Tgt1({}, address=Address("tgt1")) + tgt2 = Tgt2({}, address=Address("tgt2")) + codegen_tgt = CodegenTgt({}, address=Address("codegen_tgt")) + result = targets_with_sources_types( + [Sources1], + [tgt1, tgt2, codegen_tgt], + union_membership=UnionMembership({GenerateSourcesRequest: [GenSources]}), + ) + assert set(result) == {tgt1, codegen_tgt}