Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add include_sources to pex_binary target #16215

Merged
merged 1 commit into from
Jul 20, 2022
Merged

Add include_sources to pex_binary target #16215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions src/python/pants/backend/python/goals/package_pex_binary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
PexExecutionModeField,
PexIgnoreErrorsField,
PexIncludeRequirementsField,
PexIncludeSourcesField,
PexIncludeToolsField,
PexInheritPathField,
PexLayout,
Expand Down Expand Up @@ -69,6 +70,7 @@ class PexBinaryFieldSet(PackageFieldSet, RunFieldSet):
layout: PexLayoutField
execution_mode: PexExecutionModeField
include_requirements: PexIncludeRequirementsField
include_sources: PexIncludeSourcesField
include_tools: PexIncludeToolsField

@property
Expand Down Expand Up @@ -148,6 +150,7 @@ async def package_pex_binary(
layout=PexLayout(field_set.layout.value),
additional_args=field_set.generate_additional_args(pex_binary_defaults),
include_requirements=field_set.include_requirements.value,
include_source_files=field_set.include_sources.value,
include_local_dists=True,
),
)
Expand Down
11 changes: 11 additions & 0 deletions src/python/pants/backend/python/target_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,16 @@ class PexIncludeRequirementsField(BoolField):
)


class PexIncludeSourcesField(BoolField):
alias = "include_sources"
default = True
help = softwrap(
"""
Whether to include your first party sources the binary uses in the packaged PEX file.
Copy link
Member

Choose a reason for hiding this comment

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

I think expanding this doc with why this could be beneficial would make sense. It could be the difference of "what, why!?" to "oh, aha!!" for a novice user.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was planning on a blog post later, then linking it in the docker docs, and probably here too.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that explaining how this is used and actually sketching an example might be good. It might help clarify that there are other UXs available to solve the problem.

Copy link
Member Author

@thejcannon thejcannon Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I will say this value mirrors include_requirements directly, which also doesn't include potential use cases.

"""
)


class PexIncludeToolsField(BoolField):
alias = "include_tools"
default = False
Expand Down Expand Up @@ -614,6 +624,7 @@ class PexIncludeToolsField(BoolField):
PexLayoutField,
PexExecutionModeField,
PexIncludeRequirementsField,
PexIncludeSourcesField,
PexIncludeToolsField,
RestartableField,
)
Expand Down
33 changes: 33 additions & 0 deletions src/python/pants/backend/python/util_rules/pex_from_targets_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from pants.build_graph.address import Address
from pants.core.goals.generate_lockfiles import NoCompatibleResolveException
from pants.engine.addresses import Addresses
from pants.engine.fs import Snapshot
from pants.testutil.rule_runner import QueryRule, RuleRunner, engine_error
from pants.util.contextutil import pushd
from pants.util.ordered_set import OrderedSet
Expand Down Expand Up @@ -398,6 +399,38 @@ def test_exclude_requirements(
assert len(pex_request.requirements.req_strings) == (1 if include_requirements else 0)


@pytest.mark.parametrize("include_sources", [False, True])
def test_exclude_sources(include_sources: bool, rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"BUILD": dedent(
"""
python_sources(name="app", sources=["app.py"])
"""
),
"app.py": "",
}
)

rule_runner.set_options(
[
"--backend-packages=pants.backend.python",
"--python-repos-indexes=[]",
],
env_inherit={"PATH"},
)

request = PexFromTargetsRequest(
[Address("", target_name="app")],
output_filename="demo.pex",
internal_only=True,
include_source_files=include_sources,
)
pex_request = rule_runner.request(PexRequest, [request])
snapshot = rule_runner.request(Snapshot, [pex_request.sources])
assert len(snapshot.files) == (1 if include_sources else 0)


@pytest.mark.parametrize("enable_resolves", [False, True])
def test_cross_platform_pex_disables_subsetting(
rule_runner: RuleRunner, enable_resolves: bool
Expand Down