Skip to content

Commit

Permalink
add [export].py-editables-in-resolves option
Browse files Browse the repository at this point in the history 
This way people have to opt into including the editables (and therefore
incurring the cost of finding/building/installing the PEP-660 wheels).
  • Loading branch information
@cognifloyd
cognifloyd committed Apr 7, 2023
1 parent 2a9876d commit a03b7e5
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 34 deletions.
50 changes: 39 additions & 11 deletions src/python/pants/backend/python/goals/export.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from pants.engine.rules import collect_rules, rule
from pants.engine.target import Target
from pants.engine.unions import UnionMembership, UnionRule, union
from pants.option.option_types import BoolOption, EnumOption
from pants.option.option_types import BoolOption, EnumOption, StrListOption
from pants.util.docutil import bin_name
from pants.util.strutil import path_safe, softwrap

Expand Down Expand Up @@ -128,6 +128,30 @@ class ExportPluginOptions:
removal_hint="Set the `[export].py_resolve_format` option to 'symlinked_immutable_virtualenv'",
)

py_editables_in_resolves = StrListOption(
# TODO: Is there a way to get [python].resolves in a memoized_property here?
# If so, then we can validate that all resolves here are defined there.
help=softwrap(
"""
When exporting a mutable virtualenv for a resolve, do PEP-660 editable installs
of all 'python_distribution' targets that own code in the exported resolve.
If a resolve name is not in this list, 'python_distribution' targets will not
be installed in the virtualenv. This defaults to an empty list for backwards
compatibility and to prevent unnecessary work to generate and install the
PEP-660 editable wheels.
This only applies when '[python].enable_resolves' is true and when exporting a
'mutable_virtualenv' ('symlinked_immutable_virtualenv' exports are not "full"
virtualenvs because they must not be edited, and do not include 'pip').
NOTE: If you are using legacy exports (not using the '--resolve' option), then
this option has no effect. Legacy exports will not include any editable installs.
"""
),
advanced=True,
)


async def _get_full_python_version(pex_or_venv_pex: Pex | VenvPex) -> str:
# Get the full python version (including patch #).
Expand Down Expand Up @@ -240,13 +264,13 @@ async def do_export(
PostProcessingCommand(
complete_pex_env.create_argv(
os.path.join(tmpdir_under_digest_root, pex_pex.exe),
*[
*(
os.path.join(tmpdir_under_digest_root, requirements_pex.name),
"venv",
"--pip",
"--collisions-ok",
output_path,
],
),
),
{
**complete_pex_env.environment_dict(python=requirements_pex.python),
Expand All @@ -257,10 +281,10 @@ async def do_export(
PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]),
]

if req.editable_local_dists_digest:
if req.editable_local_dists_digest is not None:
# insert editable wheel post processing commands
wheels_snapshot = await Get(Snapshot, Digest, req.editable_local_dists_digest)
py_minor_version = ".".join(py_version.split(".")[:2])
py_minor_version = ".".join(py_version.split(".", 2)[:2])
lib_dir = os.path.join(output_path, "lib", f"python{py_minor_version}", "site-packages")
dist_info_dirs = [
os.path.join(lib_dir, "-".join(f.split("-")[:2]) + ".dist-info")
Expand All @@ -272,7 +296,7 @@ async def do_export(
# the wheels are "sources" in the pex and get dumped in lib_dir
# so we move them to tmpdir where they will be removed at the end.
"mv",
*[os.path.join(lib_dir, f) for f in wheels_snapshot.files],
*(os.path.join(lib_dir, f) for f in wheels_snapshot.files),
tmpdir_under_digest_root,
]
),
Expand All @@ -281,7 +305,7 @@ async def do_export(
# now install the editable wheels
os.path.join(output_path, "bin", "pip"),
"install",
*[os.path.join(tmpdir_under_digest_root, f) for f in wheels_snapshot.files],
*(os.path.join(tmpdir_under_digest_root, f) for f in wheels_snapshot.files),
]
),
PostProcessingCommand(
Expand Down Expand Up @@ -326,6 +350,7 @@ class MaybeExportResult:
async def export_virtualenv_for_resolve(
request: _ExportVenvForResolveRequest,
python_setup: PythonSetup,
export_subsys: ExportSubsystem,
union_membership: UnionMembership,
) -> MaybeExportResult:
editable_local_dists_digest: Digest | None = None
Expand All @@ -345,10 +370,13 @@ async def export_virtualenv_for_resolve(
)
)

editable_local_dists = await Get(
EditableLocalDists, EditableLocalDistsRequest(resolve=resolve)
)
editable_local_dists_digest = editable_local_dists.optional_digest
if resolve in export_subsys.py_editables_in_resolve:
editable_local_dists = await Get(
EditableLocalDists, EditableLocalDistsRequest(resolve=resolve)
)
editable_local_dists_digest = editable_local_dists.optional_digest
else:
editable_local_dists_digest = None

pex_request = PexRequest(
description=f"Build pex for resolve `{resolve}`",
Expand Down
44 changes: 27 additions & 17 deletions src/python/pants/backend/python/goals/export_integration_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,12 @@ def test_export(py_resolve_format: PythonResolveExportFormat) -> None:
with setup_tmpdir(SOURCES) as tmpdir:
resolve_names = ["a", "b", *(tool.name for tool in EXPORTED_TOOLS)]
run_pants(
["generate-lockfiles", "export", *(f"--resolve={name}" for name in resolve_names)],
[
"generate-lockfiles",
"export",
*(f"--resolve={name}" for name in resolve_names),
"--export-py-editables-in-resolves=['a']",
],
config=build_config(tmpdir, py_resolve_format),
).assert_success()

Expand Down Expand Up @@ -137,23 +142,28 @@ def test_export(py_resolve_format: PythonResolveExportFormat) -> None:
expected_ansicolors_dir
), f"expected dist-info for ansicolors '{expected_ansicolors_dir}' does not exist"

if resolve == "a" and py_resolve_format == PythonResolveExportFormat.mutable_virtualenv:
# make sure the editable wheel for the python_distribution is installed
if py_resolve_format == PythonResolveExportFormat.mutable_virtualenv:
expected_foo_dir = os.path.join(lib_dir, "foo-1.2.3.dist-info")
assert os.path.isdir(
expected_foo_dir
), f"expected dist-info for foo '{expected_foo_dir}' does not exist"
# direct_url__pants__.json should be moved to direct_url.json
expected_foo_direct_url_pants = os.path.join(
expected_foo_dir, "direct_url__pants__.json"
)
assert not os.path.isfile(
expected_foo_direct_url_pants
), f"expected direct_url__pants__.json for foo '{expected_foo_direct_url_pants}' was not removed"
expected_foo_direct_url = os.path.join(expected_foo_dir, "direct_url.json")
assert os.path.isfile(
expected_foo_direct_url
), f"expected direct_url.json for foo '{expected_foo_direct_url}' does not exist"
if resolve == "b":
assert not os.path.isdir(
expected_foo_dir
), f"unexpected dist-info for foo '{expected_foo_dir}' exists"
elif resolve == "a":
# make sure the editable wheel for the python_distribution is installed
assert os.path.isdir(
expected_foo_dir
), f"expected dist-info for foo '{expected_foo_dir}' does not exist"
# direct_url__pants__.json should be moved to direct_url.json
expected_foo_direct_url_pants = os.path.join(
expected_foo_dir, "direct_url__pants__.json"
)
assert not os.path.isfile(
expected_foo_direct_url_pants
), f"expected direct_url__pants__.json for foo '{expected_foo_direct_url_pants}' was not removed"
expected_foo_direct_url = os.path.join(expected_foo_dir, "direct_url.json")
assert os.path.isfile(
expected_foo_direct_url
), f"expected direct_url.json for foo '{expected_foo_direct_url}' does not exist"

for tool_config in EXPORTED_TOOLS:
export_dir = os.path.join(export_prefix, tool_config.name, platform.python_version())
Expand Down
32 changes: 26 additions & 6 deletions src/python/pants/backend/python/goals/export_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@
from pants.backend.python.goals import export
from pants.backend.python.goals.export import ExportVenvsRequest, PythonResolveExportFormat
from pants.backend.python.lint.flake8 import subsystem as flake8_subsystem
from pants.backend.python.target_types import PythonRequirementTarget
from pants.backend.python.macros.python_artifact import PythonArtifact
from pants.backend.python.target_types import (
PythonDistribution,
PythonRequirementTarget,
PythonSourcesGeneratorTarget,
)
from pants.backend.python.util_rules import pex_from_targets
from pants.base.specs import RawSpecs, RecursiveGlobSpec
from pants.core.goals.export import ExportResults
from pants.core.util_rules import distdir
from pants.engine.internals.parametrize import Parametrize
from pants.engine.rules import QueryRule
from pants.engine.target import Targets
from pants.testutil.rule_runner import RuleRunner
Expand All @@ -34,7 +40,8 @@ def rule_runner() -> RuleRunner:
QueryRule(Targets, [RawSpecs]),
QueryRule(ExportResults, [ExportVenvsRequest]),
],
target_types=[PythonRequirementTarget],
target_types=[PythonRequirementTarget, PythonSourcesGeneratorTarget, PythonDistribution],
objects={"python_artifact": PythonArtifact, "parametrize": Parametrize},
)


Expand Down Expand Up @@ -170,8 +177,15 @@ def test_export_venv_new_codepath(
current_interpreter = f"{vinfo.major}.{vinfo.minor}.{vinfo.micro}"
rule_runner.write_files(
{
"src/foo/__init__.py": "from colors import *",
"src/foo/BUILD": dedent(
"""\
python_sources(name='foo', resolve=parametrize('a', 'b'))
python_distribution(
name='dist',
provides=python_artifact(name='foo', version='1.2.3'),
dependencies=[':foo@resolve=a'],
)
python_requirement(name='req1', requirements=['ansicolors==1.1.8'], resolve='a')
python_requirement(name='req2', requirements=['ansicolors==1.1.8'], resolve='b')
"""
Expand All @@ -184,12 +198,16 @@ def test_export_venv_new_codepath(
rule_runner.set_options(
[
f"--python-interpreter-constraints=['=={current_interpreter}']",
"--python-enable-resolves=True",
"--python-resolves={'a': 'lock.txt', 'b': 'lock.txt'}",
"--export-resolve=a",
"--export-resolve=b",
"--export-resolve=flake8",
# Turn off lockfile validation to make the test simpler.
"--python-invalid-lockfile-behavior=ignore",
# Turn off python synthetic lockfile targets to make the test simpler.
"--no-python-enable-lockfile-targets",
"--export-py-editables-in-resolves=['a', 'b']",
format_flag,
],
env_inherit={"PATH", "PYENV_ROOT"},
Expand All @@ -208,11 +226,13 @@ def test_export_venv_new_codepath(
assert ppc1.argv[3] == "{digest_root}"
assert ppc1.extra_env == FrozenDict()
else:
if resolve == "flake8":
assert len(result.post_processing_cmds) == 2
else:
# editable wheels are installed for a user resolve
if resolve == "a":
# editable wheels are installed for a user resolve that has dists
assert len(result.post_processing_cmds) == 5
else:
# tool resolves (flake8) and user resolves w/o dists (b)
# do not run the commands to do editable installs
assert len(result.post_processing_cmds) == 2

ppc0 = result.post_processing_cmds[0]
# The first arg is the full path to the python interpreter, which we
Expand Down

0 comments on commit a03b7e5

Please sign in to comment.