Skip to content

Commit

Permalink
Add output_path field to python_binary, python_awslambda, and `…
Browse files Browse the repository at this point in the history
…archive` (pantsbuild#10899)

### Problem

When building an `archive`, we believe users will want to be able to control where certain files/packages show up. This is why we added `relocated_files` in pantsbuild#10880. 

However, `relocated_files` would not work with a package. If you tried using `relocated_files` with a package, then it would not build the actual package, as it's not `FilesSources`.

Further, even if you're not using `archive`, users may want to control the output path when running `./pants package` or using `runtime_package_dependencies` in `python_tests`. For example, they may want to hardcode a certain value so that changing the target name or directory path would not change the final package name.

In v1, we had `basename` for this. But `basename` is not adequate because this solely changes the final file name, but not the full path, like `src.python.pants/pants.pex`.

### Solution

For package target types, allow users to override the default path.

We warn that this can result in ambiguous paths if the user is not careful, whereas our default is always unambiguous. While this could be surprising, the user must go out of their way to opt-in, and we will warn both in `./pants target-types` and the online docs.

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

import logging
import os
from dataclasses import dataclass

from pants.backend.awslambda.common.rules import AWSLambdaFieldSet, CreatedAWSLambda
Expand All @@ -25,23 +23,22 @@
PexFromTargetsRequest,
TwoStepPexFromTargetsRequest,
)
from pants.core.goals.package import BuiltPackage, PackageFieldSet
from pants.core.goals.package import BuiltPackage, OutputPathField, PackageFieldSet
from pants.engine.fs import Digest, MergeDigests
from pants.engine.process import ProcessResult
from pants.engine.rules import Get, collect_rules, rule
from pants.engine.unions import UnionRule
from pants.option.global_options import GlobalOptions
from pants.util.logging import LogLevel

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class PythonAwsLambdaFieldSet(PackageFieldSet, AWSLambdaFieldSet):
required_fields = (PythonAwsLambdaHandler, PythonAwsLambdaRuntime)

handler: PythonAwsLambdaHandler
runtime: PythonAwsLambdaRuntime
output_path: OutputPathField


@dataclass(frozen=True)
Expand All @@ -53,21 +50,13 @@ class LambdexSetup:
async def create_python_awslambda(
field_set: PythonAwsLambdaFieldSet, lambdex_setup: LambdexSetup, global_options: GlobalOptions
) -> CreatedAWSLambda:
# Lambdas typically use the .zip suffix, so we use that instead of .pex.
disambiguated_pex_filename = os.path.join(
field_set.address.spec_path.replace(os.sep, "."), f"{field_set.address.target_name}.zip"
output_filename = field_set.output_path.value_or_default(
field_set.address,
# Lambdas typically use the .zip suffix, so we use that instead of .pex.
file_ending="zip",
use_legacy_format=global_options.options.pants_distdir_legacy_paths,
)
if global_options.options.pants_distdir_legacy_paths:
pex_filename = f"{field_set.address.target_name}.zip"
logger.warning(
f"Writing to the legacy subpath: {pex_filename}, which may not be unique. An "
f"upcoming version of Pants will switch to writing to the fully-qualified subpath: "
f"{disambiguated_pex_filename}. You can effect that switch now (and silence this "
f"warning) by setting `pants_distdir_legacy_paths = false` in the [GLOBAL] section of "
f"pants.toml."
)
else:
pex_filename = disambiguated_pex_filename

# We hardcode the platform value to the appropriate one for each AWS Lambda runtime.
# (Running the "hello world" lambda in the example code will report the platform, and can be
# used to verify correctness of these platform strings.)
Expand All @@ -83,7 +72,7 @@ async def create_python_awslambda(
addresses=[field_set.address],
internal_only=False,
entry_point=None,
output_filename=pex_filename,
output_filename=output_filename,
platforms=PexPlatforms([platform]),
additional_args=[
# Ensure we can resolve manylinux wheels in addition to any AMI-specific wheels.
Expand All @@ -105,15 +94,15 @@ async def create_python_awslambda(
ProcessResult,
PexProcess(
lambdex_setup.requirements_pex,
argv=("build", "-e", field_set.handler.value, pex_filename),
argv=("build", "-e", field_set.handler.value, output_filename),
input_digest=input_digest,
output_files=(pex_filename,),
description=f"Setting up handler in {pex_filename}",
output_files=(output_filename,),
description=f"Setting up handler in {output_filename}",
),
)
return CreatedAWSLambda(
digest=result.output_digest,
zip_file_relpath=pex_filename,
zip_file_relpath=output_filename,
runtime=field_set.runtime.value,
# The AWS-facing handler function is always lambdex_handler.handler, which is the wrapper
# injected by lambdex that manages invocation of the actual handler.
Expand Down
23 changes: 6 additions & 17 deletions src/python/pants/backend/python/goals/package_python_binary.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import logging
import os
from dataclasses import dataclass
from typing import Tuple
Expand All @@ -24,7 +23,7 @@
TwoStepPexFromTargetsRequest,
)
from pants.core.goals.binary import BinaryFieldSet, CreatedBinary
from pants.core.goals.package import BuiltPackage, PackageFieldSet
from pants.core.goals.package import BuiltPackage, OutputPathField, PackageFieldSet
from pants.core.goals.run import RunFieldSet
from pants.engine.fs import PathGlobs, Paths
from pants.engine.rules import Get, collect_rules, rule
Expand All @@ -34,8 +33,6 @@
from pants.source.source_root import SourceRoot, SourceRootRequest
from pants.util.logging import LogLevel

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class PythonBinaryFieldSet(PackageFieldSet, BinaryFieldSet, RunFieldSet):
Expand All @@ -44,6 +41,7 @@ class PythonBinaryFieldSet(PackageFieldSet, BinaryFieldSet, RunFieldSet):
sources: PythonBinarySources
entry_point: PythonEntryPoint

output_path: OutputPathField
always_write_cache: PexAlwaysWriteCache
emit_warnings: PexEmitWarnings
ignore_errors: PexIgnoreErrors
Expand Down Expand Up @@ -98,20 +96,11 @@ async def package_python_binary(
os.path.relpath(entry_point_path, source_root.path)
)

disambiguated_output_filename = os.path.join(
field_set.address.spec_path.replace(os.sep, "."), f"{field_set.address.target_name}.pex"
output_filename = field_set.output_path.value_or_default(
field_set.address,
file_ending="pex",
use_legacy_format=global_options.options.pants_distdir_legacy_paths,
)
if global_options.options.pants_distdir_legacy_paths:
output_filename = f"{field_set.address.target_name}.pex"
logger.warning(
f"Writing to the legacy subpath: {output_filename}, which may not be unique. An "
f"upcoming version of Pants will switch to writing to the fully-qualified subpath: "
f"{disambiguated_output_filename}. You can effect that switch now (and silence this "
f"warning) by setting `pants_distdir_legacy_paths = false` in the [GLOBAL] section of "
f"pants.toml."
)
else:
output_filename = disambiguated_output_filename
two_step_pex = await Get(
TwoStepPex,
TwoStepPexFromTargetsRequest(
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pants.backend.python.macros.python_artifact import PythonArtifact
from pants.backend.python.subsystems.pytest import PyTest
from pants.base.deprecated import warn_or_error
from pants.core.goals.package import OutputPathField
from pants.engine.addresses import Address, AddressInput
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import (
Expand Down Expand Up @@ -233,6 +234,7 @@ class PythonBinary(Target):
alias = "python_binary"
core_fields = (
*COMMON_PYTHON_FIELDS,
OutputPathField,
PythonBinarySources,
PythonBinaryDependencies,
PythonEntryPoint,
Expand Down
46 changes: 45 additions & 1 deletion src/python/pants/core/goals/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,22 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import logging
import os
from abc import ABCMeta
from dataclasses import dataclass
from typing import Optional

from pants.core.util_rules.distdir import DistDir
from pants.engine.addresses import Address
from pants.engine.fs import Digest, MergeDigests, Snapshot, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import FieldSet, TargetRootsToFieldSets, TargetRootsToFieldSetsRequest
from pants.engine.target import (
FieldSet,
StringField,
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest,
)
from pants.engine.unions import union

logger = logging.getLogger(__name__)
Expand All @@ -28,6 +35,43 @@ class BuiltPackage:
extra_log_info: Optional[str] = None


class OutputPathField(StringField):
"""Where the built asset should be located.
If undefined, this will use the path to the the BUILD, followed by the target name. For
example, `src/python/project:app` would be `src.python.project/app.ext`.
When running `./pants package`, this path will be prefixed by `--distdir` (e.g. `dist/`).
Warning: setting this value risks naming collisions with other package targets you may have.
"""

alias = "output_path"

def value_or_default(
self, address: Address, *, file_ending: str, use_legacy_format: bool
) -> str:
assert not file_ending.startswith("."), "`file_ending` should not start with `.`"
if self.value is not None:
return self.value
disambiguated = os.path.join(
address.spec_path.replace(os.sep, "."), f"{address.target_name}.{file_ending}"
)
if use_legacy_format:
ambiguous_name = f"{address.target_name}.{file_ending}"
logger.warning(
f"Writing to the legacy subpath {repr(ambiguous_name)} for the target {address}. "
f"This location may not be unique. An upcoming version of Pants will switch to "
f"writing to the fully-qualified subpath: {disambiguated}.\n\nYou can make that "
"switch now (and silence this warning) by setting "
"`pants_distdir_legacy_paths = false` in the [GLOBAL] section "
"of pants.toml.\n\nAlternatively, you can set the field `output_path` on the "
f"target {address} to a hardcoded value."
)
return ambiguous_name
return disambiguated


class PackageSubsystem(GoalSubsystem):
"""Create a distributable package."""

Expand Down
26 changes: 18 additions & 8 deletions src/python/pants/core/target_types.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os
from dataclasses import dataclass
from typing import Tuple

from pants.core.goals.package import BuiltPackage, PackageFieldSet
from pants.core.goals.package import BuiltPackage, OutputPathField, PackageFieldSet
from pants.core.util_rules.archive import ArchiveFormat, CreateArchive
from pants.engine.addresses import AddressInput
from pants.engine.fs import AddPrefix, Digest, MergeDigests, RemovePrefix, Snapshot
Expand All @@ -26,6 +25,7 @@
WrappedTarget,
)
from pants.engine.unions import UnionRule
from pants.option.global_options import GlobalOptions
from pants.util.logging import LogLevel

# -----------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -267,7 +267,13 @@ class ArchiveTarget(Target):
package`."""

alias = "archive"
core_fields = (*COMMON_TARGET_FIELDS, ArchivePackages, ArchiveFiles, ArchiveFormatField)
core_fields = (
*COMMON_TARGET_FIELDS,
OutputPathField,
ArchivePackages,
ArchiveFiles,
ArchiveFormatField,
)


@dataclass(frozen=True)
Expand All @@ -277,10 +283,13 @@ class ArchiveFieldSet(PackageFieldSet):
packages: ArchivePackages
files: ArchiveFiles
format_field: ArchiveFormatField
output_path: OutputPathField


@rule(level=LogLevel.DEBUG)
async def package_archive_target(field_set: ArchiveFieldSet) -> BuiltPackage:
async def package_archive_target(
field_set: ArchiveFieldSet, global_options: GlobalOptions
) -> BuiltPackage:
package_targets = await MultiGet(
Get(
WrappedTarget,
Expand Down Expand Up @@ -330,10 +339,11 @@ async def package_archive_target(field_set: ArchiveFieldSet) -> BuiltPackage:
)
),
)
file_ending = field_set.format_field.value
output_filename = os.path.join(
field_set.address.spec_path.replace(os.sep, "."),
f"{field_set.address.target_name}.{file_ending}",

output_filename = field_set.output_path.value_or_default(
field_set.address,
file_ending=field_set.format_field.value,
use_legacy_format=global_options.options.pants_distdir_legacy_paths,
)
archive = await Get(
Digest,
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/core/target_types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ def test_archive() -> None:
packages=[":archive1"],
files=["resources:relocated_files"],
format="tar",
output_path="output/archive2.tar",
)
"""
),
Expand Down Expand Up @@ -227,6 +228,7 @@ def assert_archive1_is_valid(zip_bytes: bytes) -> None:
assert_archive1_is_valid(archive1.content)

archive2 = get_archive("archive2")
assert archive2.path == "output/archive2.tar"
io = BytesIO()
io.write(archive2.content)
io.seek(0)
Expand Down
16 changes: 5 additions & 11 deletions src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ def compute_value(cls, raw_value: Optional[bool], *, address: Address) -> Option
return value_or_default


class IntField(ScalarField, metaclass=ABCMeta):
class IntField(ScalarField[int], metaclass=ABCMeta):
expected_type = int
expected_type_description = "an integer"

Expand All @@ -1021,7 +1021,7 @@ def compute_value(cls, raw_value: Optional[int], *, address: Address) -> Optiona
return super().compute_value(raw_value, address=address)


class FloatField(ScalarField, metaclass=ABCMeta):
class FloatField(ScalarField[float], metaclass=ABCMeta):
expected_type = float
expected_type_description = "a float"

Expand All @@ -1030,7 +1030,7 @@ def compute_value(cls, raw_value: Optional[float], *, address: Address) -> Optio
return super().compute_value(raw_value, address=address)


class StringField(ScalarField, metaclass=ABCMeta):
class StringField(ScalarField[str], metaclass=ABCMeta):
"""A field whose value is a string.
If you expect the string to only be one of several values, set the class property
Expand Down Expand Up @@ -1100,10 +1100,7 @@ def compute_value(
return tuple(value_or_default)


class StringSequenceField(SequenceField, metaclass=ABCMeta):
value: Optional[Tuple[str, ...]]
default: ClassVar[Optional[Tuple[str, ...]]] = None

class StringSequenceField(SequenceField[str], metaclass=ABCMeta):
expected_element_type = str
expected_type_description = "an iterable of strings (e.g. a list of strings)"

Expand All @@ -1114,7 +1111,7 @@ def compute_value(
return super().compute_value(raw_value, address=address)


class StringOrStringSequenceField(SequenceField, metaclass=ABCMeta):
class StringOrStringSequenceField(SequenceField[str], metaclass=ABCMeta):
"""The raw_value may either be a string or be an iterable of strings.
This is syntactic sugar that we use for certain fields to make BUILD files simpler when the user
Expand All @@ -1123,9 +1120,6 @@ class StringOrStringSequenceField(SequenceField, metaclass=ABCMeta):
Generally, this should not be used by any new Fields. This mechanism is a misfeature.
"""

value: Optional[Tuple[str, ...]]
default: ClassVar[Optional[Tuple[str, ...]]] = None

expected_element_type = str
expected_type_description = (
"either a single string or an iterable of strings (e.g. a list of strings)"
Expand Down

0 comments on commit 3b6fbd7

Please sign in to comment.