Skip to content

Commit

Permalink
Warn when depending on files targets in a pex_binary or `python_a…
Browse files Browse the repository at this point in the history
…wslambda`

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
  • Loading branch information
Eric-Arellano committed Mar 9, 2021
1 parent 3e7e97a commit b8414af
Show file tree
Hide file tree
Showing 6 changed files with 276 additions and 12 deletions.
33 changes: 30 additions & 3 deletions 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
Expand Down Expand Up @@ -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):
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
71 changes: 67 additions & 4 deletions 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

Expand All @@ -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
Expand All @@ -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],
)


Expand Down Expand Up @@ -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!')
Expand All @@ -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')
Expand All @@ -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
41 changes: 36 additions & 5 deletions 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

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand Down
@@ -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"
18 changes: 18 additions & 0 deletions src/python/pants/engine/target.py
Expand Up @@ -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
# -----------------------------------------------------------------------------------------------
Expand Down

0 comments on commit b8414af

Please sign in to comment.