From d1cfd68db0d5d1f0d5026b347781a03880b2e8fb Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 1 Apr 2023 00:29:29 -0500 Subject: [PATCH 01/32] naive PEP660 --- .../python/util_rules/local_dists_pep660.py | 484 ++++++++++++++++++ 1 file changed, 484 insertions(+) create mode 100644 src/python/pants/backend/python/util_rules/local_dists_pep660.py diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py new file mode 100644 index 00000000000..a80c2aa731d --- /dev/null +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -0,0 +1,484 @@ +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import logging +import os +import shlex +from dataclasses import dataclass +from typing import Iterable, Mapping + +from pants.backend.python.subsystems.setup import PythonSetup +from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet +from pants.backend.python.util_rules.dists import BuildBackendError, BuildSystem, distutils_repr +from pants.backend.python.util_rules.dists import rules as dists_rules +from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints +from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess +from pants.backend.python.util_rules.pex_requirements import EntireLockfile, PexRequirements +from pants.backend.python.util_rules.python_sources import PythonSourceFiles +from pants.build_graph.address import Address +from pants.core.goals.package import BuiltPackage, PackageFieldSet +from pants.core.util_rules import system_binaries +from pants.core.util_rules.source_files import SourceFiles +from pants.core.util_rules.system_binaries import BashBinary, UnzipBinary +from pants.engine.addresses import Addresses +from pants.engine.fs import ( + CreateDigest, + Digest, + DigestSubset, + FileContent, + MergeDigests, + PathGlobs, + RemovePrefix, + Snapshot, +) +from pants.engine.process import Process, ProcessResult +from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.target import ( + TransitiveTargets, + TransitiveTargetsRequest, + WrappedTarget, + WrappedTargetRequest, +) +from pants.util.dirutil import fast_relpath_optional +from pants.util.docutil import doc_url +from pants.util.frozendict import FrozenDict +from pants.util.logging import LogLevel +from pants.util.osutil import is_macos_big_sur +from pants.util.strutil import softwrap + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class LocalDistPEP660Wheels: # Based on LocalDistWheels + """Contains the PEP 660 "editable" wheels isolated from a single local Python distribution.""" + + pep660_wheel_paths: tuple[str, ...] + pep660_wheels_digest: Digest + provided_files: frozenset[str] + + +@rule +async def isolate_local_dist_pep660_wheels( + dist_field_set: PythonDistributionFieldSet, + bash: BashBinary, + unzip_binary: UnzipBinary, +) -> LocalDistPEP660Wheels: + # TODO: BuiltPackage is not the right thing to get here... + dist = await Get(BuiltPackage, PackageFieldSet, dist_field_set) + wheels_snapshot = await Get(Snapshot, DigestSubset(dist.digest, PathGlobs(["**/*.whl"]))) + + # We ignore everything except the wheels, which should be PEP 660 Editable Wheels. + artifacts = {(a.relpath or "") for a in dist.artifacts} + wheels = [wheel for wheel in wheels_snapshot.files if wheel in artifacts] + + if not wheels: + tgt = await Get( + WrappedTarget, + WrappedTargetRequest(dist_field_set.address, description_of_origin=""), + ) + logger.warning( + softwrap( + f""" + Encountered a dependency on the {tgt.target.alias} target at {dist_field_set.address}, + but this target does not produce a Python wheel artifact. Therefore this target's + code will be used directly from sources, without a distribution being built, + and any native extensions in it will not be built. + + See {doc_url('python-distributions')} for details on how to set up a + {tgt.target.alias} target to produce a wheel. + """ + ) + ) + + wheels_listing_result = await Get( + ProcessResult, + Process( + argv=[ + bash.path, + "-c", + f""" + set -ex + for f in {' '.join(shlex.quote(f) for f in wheels)}; do + {unzip_binary.path} -Z1 "$f" + done + """, + ], + input_digest=wheels_snapshot.digest, + description=f"List contents of artifacts produced by {dist_field_set.address}", + ), + ) + provided_files = set(wheels_listing_result.stdout.decode().splitlines()) + + return LocalDistPEP660Wheels(tuple(wheels), wheels_snapshot.digest, frozenset(provided_files)) + + +@dataclass(frozen=True) +class LocalDistsPEP660PexRequest: + """Request to build a PEX populated by PEP660 wheels of local dists. + + Like LocalDistsPexRequest, the local dists come from the dependency closure of a set of + addresses. Unlike LocalDistsPexRequest, the editable wheel files must not be exported or made + available to the end-user (according to PEP 660). Instead, the PEP660Pex serves as an + intermediate, internal-only, PEX that can be used to install these wheels in a virtualenv. It + follows then, that this PEX should probably not be exported for use by end-users. + """ + + addresses: Addresses + interpreter_constraints: InterpreterConstraints + # The result will return these with the sources provided by the dists subtracted out. + # This will help the caller prevent sources from appearing twice on sys.path. + sources: PythonSourceFiles + + def __init__( + self, + addresses: Iterable[Address], + *, + interpreter_constraints: InterpreterConstraints, + sources: PythonSourceFiles = PythonSourceFiles.empty(), + ) -> None: + object.__setattr__(self, "addresses", Addresses(addresses)) + object.__setattr__(self, "interpreter_constraints", interpreter_constraints) + object.__setattr__(self, "sources", sources) + + +@dataclass(frozen=True) +class LocalDistsPEP660Pex: + """A PEX file populated by PEP660 wheels of local dists. + + Can be consumed from another PEX, e.g., by adding to PEX_PATH. + + The PEX will contain installs of PEP660 wheels generate4d from local dists. Installing PEP660 + wheels creates an "editable" install such that the sys.path gets adjusted to include source + directories that are NOT part of the PEX. As such, this PEX is decidedly not hermetic or + portable and should only be used to facilitate building virtualenvs for development. + + PEP660 wheels have .dist-info metadata and the .pth files (or similar) that adjust sys.path. + + The PEX will only contain metadata for local dists and not any dependencies. For Pants generated + `setup.py` / `pyproject.toml`, the dependencies will be included in the standard resolve process + that the locally-built dists PEX is adjoined to via PEX_PATH. For hand-made `setup.py` / + `pyproject.toml` with 3rdparty dependencies not hand-mirrored into BUILD file dependencies, this + will lead to issues. See https://github.com/pantsbuild/pants/issues/13587#issuecomment-974863636 + for one way to fix this corner which is intentionally punted on for now. + + Lists the files provided by the dists on sys.path, so they can be subtracted from + sources digests, to prevent the same file ending up on sys.path twice. + """ + + pex: Pex + # The sources from the request, but with any files provided by the local dists subtracted out. + # In general, this will have a list of all of the dists' sources. + remaining_sources: PythonSourceFiles + + +@rule(desc="Building editable local distributions (PEP 660)") +async def build_editable_local_dists( + request: LocalDistsPEP660PexRequest, +) -> LocalDistsPEP660Pex: + transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)) + applicable_targets = [ + tgt for tgt in transitive_targets.closure if PythonDistributionFieldSet.is_applicable(tgt) + ] + + local_dists_wheels = await MultiGet( + Get( + LocalDistPEP660Wheels, + PythonDistributionFieldSet, + PythonDistributionFieldSet.create(target), + ) + for target in applicable_targets + ) + + provided_files: set[str] = set() + wheels: list[str] = [] + wheels_digests = [] + for local_dist_wheels in local_dists_wheels: + wheels.extend(local_dist_wheels.pep660_wheel_paths) + wheels_digests.append(local_dist_wheels.pep660_wheels_digest) + provided_files.update(local_dist_wheels.provided_files) + + wheels_digest = await Get(Digest, MergeDigests(wheels_digests)) + + editable_dists_pex = await Get( + Pex, + PexRequest( + output_filename="editable_local_dists.pex", + requirements=PexRequirements(wheels), + interpreter_constraints=request.interpreter_constraints, + additional_inputs=wheels_digest, + internal_only=True, + additional_args=["--intransitive"], + ), + ) + + if not wheels: + # The source calculations below are not (always) cheap, so we skip them if no wheels were + # produced. See https://github.com/pantsbuild/pants/issues/14561 for one possible approach + # to sharing the cost of these calculations. + return LocalDistsPEP660Pex(editable_dists_pex, request.sources) + + # TODO: maybe DRY the logic duplicated here and in build_local_dists + # We check source roots in reverse lexicographic order, + # so we'll find the innermost root that matches. + source_roots = sorted(request.sources.source_roots, reverse=True) + remaining_sources = set(request.sources.source_files.files) + unrooted_files_set = set(request.sources.source_files.unrooted_files) + for source in request.sources.source_files.files: + if source not in unrooted_files_set: + for source_root in source_roots: + source_relpath = fast_relpath_optional(source, source_root) + if source_relpath is not None and source_relpath in provided_files: + remaining_sources.remove(source) + remaining_sources_snapshot = await Get( + Snapshot, + DigestSubset( + request.sources.source_files.snapshot.digest, PathGlobs(sorted(remaining_sources)) + ), + ) + subtracted_sources = PythonSourceFiles( + SourceFiles(remaining_sources_snapshot, request.sources.source_files.unrooted_files), + request.sources.source_roots, + ) + + return LocalDistsPEP660Pex(editable_dists_pex, subtracted_sources) + + +@dataclass(frozen=True) +class PEP660BuildRequest: # Based on DistBuildRequest + """A request to build PEP660 wheels via a PEP 517 build backend.""" + + build_system: BuildSystem + + interpreter_constraints: InterpreterConstraints + input: Digest + working_directory: str # Relpath within the input digest. + build_time_source_roots: tuple[str, ...] # Source roots for 1st party build-time deps. + output_path: str # Location of the output directory within dist dir. + + target_address_spec: str | None = None # Only needed for logging etc. + wheel_config_settings: FrozenDict[str, tuple[str, ...]] | None = None + + extra_build_time_requirements: tuple[Pex, ...] = tuple() + extra_build_time_env: Mapping[str, str] | None = None + + +@dataclass(frozen=True) +class PEP660BuildResult: # Based on DistBuildResult + output: Digest + # Relpaths in the output digest. + editable_wheel_path: str | None + + +# Sadly we have to call this shim twice, once to get any additional requirements +# and then, once those requirements are included in the build backend pex, +# to actually build the editable wheel. +_BACKEND_SHIM_BOILERPLATE = """ +# DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS + +import os +import sys +import {build_backend_module} + +backend = {build_backend_object} + +dist_dir = "{dist_dir}" +get_editable_requires = {get_requires} +build_editable = {build_editable} +wheel_config_settings = {wheel_config_settings_str} + +if get_requires: + editable_requires = backend.get_requires_for_build_editable(wheel_config_settings) + for pep508_req in editable_requires: + print("editable_requires: {{pep508_req}}".format(pep508_req=pep508_req)) + +if not build_editable: + sys.exit(0) + +# Python 2.7 doesn't have the exist_ok arg on os.makedirs(). +try: + os.makedirs(dist_dir) +except OSError as e: + if e.errno != errno.EEXIST: + raise + +editable_path = backend.build_editable(dist_dir, wheel_config_settings) +print("editable: {{editable_path}}".format(editable_path=editable_path)) +""" + + +def interpolate_backend_shim( + dist_dir: str, + request: PEP660BuildRequest, + get_editable_requires: bool = False, + build_editable: bool = False, +) -> bytes: + # See https://www.python.org/dev/peps/pep-0517/#source-trees. + module_path, _, object_path = request.build_system.build_backend.partition(":") + backend_object = f"{module_path}.{object_path}" if object_path else module_path + + def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: + # setuptools.build_meta expects list values and chokes on tuples. + # We assume/hope that other backends accept lists as well. + return distutils_repr(None if cs is None else {k: list(v) for k, v in cs.items()}) + + return _BACKEND_SHIM_BOILERPLATE.format( + build_backend_module=module_path, + build_backend_object=backend_object, + dist_dir=dist_dir, + get_editable_requires=get_editable_requires, + build_editable=build_editable, + wheel_config_settings_str=config_settings_repr(request.wheel_config_settings), + ).encode() + + +@rule +async def run_pep660_build( + request: PEP660BuildRequest, python_setup: PythonSetup +) -> PEP660BuildResult: + # Note that this pex has no entrypoint. We use it to run our generated shim, which + # in turn imports from and invokes the build backend. + build_backend_pex = await Get( + VenvPex, + PexRequest( + output_filename="build_backend.pex", + internal_only=True, + requirements=request.build_system.requires, + pex_path=request.extra_build_time_requirements, + interpreter_constraints=request.interpreter_constraints, + ), + ) + + # This is the setuptools dist directory, not Pants's, so we hardcode to dist/. + dist_dir = "dist" + dist_output_dir = os.path.join(dist_dir, request.output_path) + backend_shim_name = "backend_shim.py" + backend_shim_path = os.path.join(request.working_directory, backend_shim_name) + get_requires_backend_shim_digest = await Get( + Digest, + CreateDigest( + [ + FileContent( + backend_shim_path, + interpolate_backend_shim(dist_output_dir, request, get_editable_requires=True), + ), + ] + ), + ) + build_editable_backend_shim_digest = await Get( + Digest, + CreateDigest( + [ + FileContent( + backend_shim_path, + interpolate_backend_shim(dist_output_dir, request, build_editable=True), + ), + ] + ), + ) + + get_requires_merged_digest = await Get( + Digest, MergeDigests((request.input, get_requires_backend_shim_digest)) + ) + build_editable_merged_digest = await Get( + Digest, MergeDigests((request.input, build_editable_backend_shim_digest)) + ) + + extra_env = { + **(request.extra_build_time_env or {}), + "PEX_EXTRA_SYS_PATH": os.pathsep.join(request.build_time_source_roots), + } + if python_setup.macos_big_sur_compatibility and is_macos_big_sur(): + extra_env["MACOSX_DEPLOYMENT_TARGET"] = "10.16" + + requires_result = await Get( + ProcessResult, + VenvPexProcess( + build_backend_pex, + argv=(backend_shim_name,), + input_digest=get_requires_merged_digest, + extra_env=extra_env, + working_directory=request.working_directory, + output_directories=(dist_dir,), # Relative to the working_directory. + description=( + f"Run {request.build_system.build_backend} (get_requires_for_build_editable) for {request.target_address_spec}" + if request.target_address_spec + else f"Run {request.build_system.build_backend} (get_requires_for_build_editable)" + ), + level=LogLevel.DEBUG, + ), + ) + requires_output_lines = requires_result.stdout.decode().splitlines() + editable_requires_prefix = "editable_requires: " + editable_requires_prefix_len = len(editable_requires_prefix) + editable_requires = [] + for line in requires_output_lines: + if line.startswith(editable_requires_prefix): + editable_requires.append(line[editable_requires_prefix_len:].strip()) + + # if there are any editable_requires, then we have to build another PEX with those requirements. + if editable_requires: + if isinstance(request.build_system.requires, EntireLockfile): + req_strings = list(request.build_system.requires.complete_req_strings or ()) + else: + req_strings = list(request.build_system.requires.req_strings) + # recreate the build_backend_pex but include the additional requirements + build_backend_pex = await Get( + VenvPex, + PexRequest( + output_filename="build_backend.pex", + internal_only=True, + requirements=PexRequirements(list(req_strings) + editable_requires), + pex_path=request.extra_build_time_requirements, + interpreter_constraints=request.interpreter_constraints, + ), + ) + + result = await Get( + ProcessResult, + VenvPexProcess( + build_backend_pex, + argv=(backend_shim_name,), + input_digest=build_editable_merged_digest, + extra_env=extra_env, + working_directory=request.working_directory, + output_directories=(dist_dir,), # Relative to the working_directory. + description=( + f"Run {request.build_system.build_backend} (build_editable) for {request.target_address_spec}" + if request.target_address_spec + else f"Run {request.build_system.build_backend} (build_editable)" + ), + level=LogLevel.DEBUG, + ), + ) + output_lines = result.stdout.decode().splitlines() + dist_type = "editable" + editable_path = "" + for line in output_lines: + if line.startswith(f"{dist_type}: "): + editable_path = os.path.join(request.output_path, line[len(dist_type) + 2 :].strip()) + break + + # Note that output_digest paths are relative to the working_directory. + output_digest = await Get(Digest, RemovePrefix(result.output_digest, dist_dir)) + output_snapshot = await Get(Snapshot, Digest, output_digest) + if editable_path not in output_snapshot.files: + raise BuildBackendError( + softwrap( + f""" + PEP 660 build backend {request.build_system.build_backend} + did not create expected editable wheel file {editable_path} + """ + ) + ) + return PEP660BuildResult(output_digest, editable_wheel_path=editable_path) + + +def rules(): + return ( + *collect_rules(), + *dists_rules(), + *system_binaries.rules(), + ) From d36b839faa57799dbe42392146aaa6167592edc4 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 1 Apr 2023 18:25:20 -0500 Subject: [PATCH 02/32] refactor package_python_dist so most of the logic can be reused for editable wheels --- .../python/util_rules/local_dists_pep660.py | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index a80c2aa731d..3cd00bf9574 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -7,18 +7,23 @@ import os import shlex from dataclasses import dataclass -from typing import Iterable, Mapping +from typing import Iterable +# TODO: move this to a util_rules file +from pants.backend.python.goals.setup_py import create_dist_build_request from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet -from pants.backend.python.util_rules.dists import BuildBackendError, BuildSystem, distutils_repr +from pants.backend.python.util_rules.dists import ( + BuildBackendError, + DistBuildRequest, + distutils_repr, +) from pants.backend.python.util_rules.dists import rules as dists_rules from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.pex_requirements import EntireLockfile, PexRequirements from pants.backend.python.util_rules.python_sources import PythonSourceFiles from pants.build_graph.address import Address -from pants.core.goals.package import BuiltPackage, PackageFieldSet from pants.core.util_rules import system_binaries from pants.core.util_rules.source_files import SourceFiles from pants.core.util_rules.system_binaries import BashBinary, UnzipBinary @@ -41,6 +46,7 @@ WrappedTarget, WrappedTargetRequest, ) +from pants.engine.unions import UnionMembership from pants.util.dirutil import fast_relpath_optional from pants.util.docutil import doc_url from pants.util.frozendict import FrozenDict @@ -65,14 +71,24 @@ async def isolate_local_dist_pep660_wheels( dist_field_set: PythonDistributionFieldSet, bash: BashBinary, unzip_binary: UnzipBinary, + python_setup: PythonSetup, + union_membership: UnionMembership, ) -> LocalDistPEP660Wheels: - # TODO: BuiltPackage is not the right thing to get here... - dist = await Get(BuiltPackage, PackageFieldSet, dist_field_set) - wheels_snapshot = await Get(Snapshot, DigestSubset(dist.digest, PathGlobs(["**/*.whl"]))) + dist_build_request = await create_dist_build_request( + field_set=dist_field_set, + python_setup=python_setup, + union_membership=union_membership, + # editable wheel ignores build_wheel+build_sdist args + validate_wheel_sdist=False, + ) + pep660_result = await Get(PEP660BuildResult, DistBuildRequest, dist_build_request) - # We ignore everything except the wheels, which should be PEP 660 Editable Wheels. - artifacts = {(a.relpath or "") for a in dist.artifacts} - wheels = [wheel for wheel in wheels_snapshot.files if wheel in artifacts] + # the output digest should only contain wheels, but filter to be safe. + wheels_snapshot = await Get( + Snapshot, DigestSubset(pep660_result.output, PathGlobs(["**/*.whl"])) + ) + + wheels = tuple(wheels_snapshot.files) if not wheels: tgt = await Get( @@ -107,12 +123,12 @@ async def isolate_local_dist_pep660_wheels( """, ], input_digest=wheels_snapshot.digest, - description=f"List contents of artifacts produced by {dist_field_set.address}", + description=f"List contents of editable artifacts produced by {dist_field_set.address}", ), ) provided_files = set(wheels_listing_result.stdout.decode().splitlines()) - return LocalDistPEP660Wheels(tuple(wheels), wheels_snapshot.digest, frozenset(provided_files)) + return LocalDistPEP660Wheels(wheels, wheels_snapshot.digest, frozenset(provided_files)) @dataclass(frozen=True) @@ -246,23 +262,7 @@ async def build_editable_local_dists( return LocalDistsPEP660Pex(editable_dists_pex, subtracted_sources) -@dataclass(frozen=True) -class PEP660BuildRequest: # Based on DistBuildRequest - """A request to build PEP660 wheels via a PEP 517 build backend.""" - - build_system: BuildSystem - - interpreter_constraints: InterpreterConstraints - input: Digest - working_directory: str # Relpath within the input digest. - build_time_source_roots: tuple[str, ...] # Source roots for 1st party build-time deps. - output_path: str # Location of the output directory within dist dir. - - target_address_spec: str | None = None # Only needed for logging etc. - wheel_config_settings: FrozenDict[str, tuple[str, ...]] | None = None - - extra_build_time_requirements: tuple[Pex, ...] = tuple() - extra_build_time_env: Mapping[str, str] | None = None +# We just use DistBuildRequest directly instead of adding a PEP660BuildRequest @dataclass(frozen=True) @@ -311,7 +311,7 @@ class PEP660BuildResult: # Based on DistBuildResult def interpolate_backend_shim( dist_dir: str, - request: PEP660BuildRequest, + request: DistBuildRequest, get_editable_requires: bool = False, build_editable: bool = False, ) -> bytes: @@ -336,7 +336,7 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: @rule async def run_pep660_build( - request: PEP660BuildRequest, python_setup: PythonSetup + request: DistBuildRequest, python_setup: PythonSetup ) -> PEP660BuildResult: # Note that this pex has no entrypoint. We use it to run our generated shim, which # in turn imports from and invokes the build backend. From 49465d9231e980c3a0ab1d1071407e8bac2d1447 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sun, 2 Apr 2023 21:44:12 -0500 Subject: [PATCH 03/32] begin writing PEP 660 backend that wraps PEP 517 backend to get dist-info dir --- .../python/util_rules/local_dists_pep660.py | 137 ++++++------------ 1 file changed, 42 insertions(+), 95 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 3cd00bf9574..a4f30e79802 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -21,7 +21,7 @@ from pants.backend.python.util_rules.dists import rules as dists_rules from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess -from pants.backend.python.util_rules.pex_requirements import EntireLockfile, PexRequirements +from pants.backend.python.util_rules.pex_requirements import PexRequirements from pants.backend.python.util_rules.python_sources import PythonSourceFiles from pants.build_graph.address import Address from pants.core.util_rules import system_binaries @@ -272,10 +272,12 @@ class PEP660BuildResult: # Based on DistBuildResult editable_wheel_path: str | None -# Sadly we have to call this shim twice, once to get any additional requirements -# and then, once those requirements are included in the build backend pex, -# to actually build the editable wheel. -_BACKEND_SHIM_BOILERPLATE = """ +# PEP 660 defines `prepare_metadata_for_build_editable`. If PEP 660 is not +# supported, we fall back to PEP 517's `prepare_metadata_for_build_wheel`. +# PEP 517, however, says that method is optional. So finally we fall back +# to using `build_wheel` and then extract the dist-info directory and then +# delete the extra wheel file. Most people shouldn't hit that path (I hope). +_BACKEND_WRAPPER_BOILERPLATE = """ # DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS import os @@ -284,36 +286,42 @@ class PEP660BuildResult: # Based on DistBuildResult backend = {build_backend_object} -dist_dir = "{dist_dir}" -get_editable_requires = {get_requires} -build_editable = {build_editable} +metadata_dir = "{metadata_dir}" wheel_config_settings = {wheel_config_settings_str} -if get_requires: - editable_requires = backend.get_requires_for_build_editable(wheel_config_settings) - for pep508_req in editable_requires: - print("editable_requires: {{pep508_req}}".format(pep508_req=pep508_req)) - -if not build_editable: - sys.exit(0) - # Python 2.7 doesn't have the exist_ok arg on os.makedirs(). try: - os.makedirs(dist_dir) + os.makedirs(metadata_dir) except OSError as e: if e.errno != errno.EEXIST: raise -editable_path = backend.build_editable(dist_dir, wheel_config_settings) -print("editable: {{editable_path}}".format(editable_path=editable_path)) +prepare_metadata = getattr( + backend, + "prepare_metadata_for_build_editable", # PEP 660 + getattr(backend, "prepare_metadata_for_build_wheel", None) # PEP 517 +) +if prepare_metadata is not None: + metadata_path = prepare_metadata(metadata_dir, wheel_config_settings) +else: + # Optional PEP 517 method not defined. Use build_wheel instead. + wheel_path = backend.build_wheel(metadata_dir, wheel_config_settings) + full_wheel_path = os.path.join(metadata_dir, wheel_path) + + import zipfile + with zipfile.ZipFile(full_wheel_path, 'r') as zip_ref: + file_names = zip_ref.namelist() + dist_info_files = [n for n in file_names if ".dist-info" in n] + zip_ref.extractall(metadata_dir, dist_info_files) + os.unlink(full_wheel_path) + +print("metadata_path: {{metadata_path}}".format(metadata_path=metadata_path)) """ -def interpolate_backend_shim( - dist_dir: str, +def interpolate_backend_wrapper( + metadata_dir: str, request: DistBuildRequest, - get_editable_requires: bool = False, - build_editable: bool = False, ) -> bytes: # See https://www.python.org/dev/peps/pep-0517/#source-trees. module_path, _, object_path = request.build_system.build_backend.partition(":") @@ -324,12 +332,10 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: # We assume/hope that other backends accept lists as well. return distutils_repr(None if cs is None else {k: list(v) for k, v in cs.items()}) - return _BACKEND_SHIM_BOILERPLATE.format( + return _BACKEND_WRAPPER_BOILERPLATE.format( build_backend_module=module_path, build_backend_object=backend_object, - dist_dir=dist_dir, - get_editable_requires=get_editable_requires, - build_editable=build_editable, + metadata_dir=metadata_dir, wheel_config_settings_str=config_settings_repr(request.wheel_config_settings), ).encode() @@ -338,7 +344,7 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: async def run_pep660_build( request: DistBuildRequest, python_setup: PythonSetup ) -> PEP660BuildResult: - # Note that this pex has no entrypoint. We use it to run our generated shim, which + # Note that this pex has no entrypoint. We use it to run our generated wrapper, which # in turn imports from and invokes the build backend. build_backend_pex = await Get( VenvPex, @@ -354,37 +360,21 @@ async def run_pep660_build( # This is the setuptools dist directory, not Pants's, so we hardcode to dist/. dist_dir = "dist" dist_output_dir = os.path.join(dist_dir, request.output_path) - backend_shim_name = "backend_shim.py" - backend_shim_path = os.path.join(request.working_directory, backend_shim_name) - get_requires_backend_shim_digest = await Get( - Digest, - CreateDigest( - [ - FileContent( - backend_shim_path, - interpolate_backend_shim(dist_output_dir, request, get_editable_requires=True), - ), - ] - ), - ) - build_editable_backend_shim_digest = await Get( + backend_wrapper_name = "backend_wrapper.py" + backend_wrapper_path = os.path.join(request.working_directory, backend_wrapper_name) + backend_wrapper_digest = await Get( Digest, CreateDigest( [ FileContent( - backend_shim_path, - interpolate_backend_shim(dist_output_dir, request, build_editable=True), + backend_wrapper_path, + interpolate_backend_wrapper(dist_output_dir, request), ), ] ), ) - get_requires_merged_digest = await Get( - Digest, MergeDigests((request.input, get_requires_backend_shim_digest)) - ) - build_editable_merged_digest = await Get( - Digest, MergeDigests((request.input, build_editable_backend_shim_digest)) - ) + merged_digest = await Get(Digest, MergeDigests((request.input, backend_wrapper_digest))) extra_env = { **(request.extra_build_time_env or {}), @@ -393,55 +383,12 @@ async def run_pep660_build( if python_setup.macos_big_sur_compatibility and is_macos_big_sur(): extra_env["MACOSX_DEPLOYMENT_TARGET"] = "10.16" - requires_result = await Get( - ProcessResult, - VenvPexProcess( - build_backend_pex, - argv=(backend_shim_name,), - input_digest=get_requires_merged_digest, - extra_env=extra_env, - working_directory=request.working_directory, - output_directories=(dist_dir,), # Relative to the working_directory. - description=( - f"Run {request.build_system.build_backend} (get_requires_for_build_editable) for {request.target_address_spec}" - if request.target_address_spec - else f"Run {request.build_system.build_backend} (get_requires_for_build_editable)" - ), - level=LogLevel.DEBUG, - ), - ) - requires_output_lines = requires_result.stdout.decode().splitlines() - editable_requires_prefix = "editable_requires: " - editable_requires_prefix_len = len(editable_requires_prefix) - editable_requires = [] - for line in requires_output_lines: - if line.startswith(editable_requires_prefix): - editable_requires.append(line[editable_requires_prefix_len:].strip()) - - # if there are any editable_requires, then we have to build another PEX with those requirements. - if editable_requires: - if isinstance(request.build_system.requires, EntireLockfile): - req_strings = list(request.build_system.requires.complete_req_strings or ()) - else: - req_strings = list(request.build_system.requires.req_strings) - # recreate the build_backend_pex but include the additional requirements - build_backend_pex = await Get( - VenvPex, - PexRequest( - output_filename="build_backend.pex", - internal_only=True, - requirements=PexRequirements(list(req_strings) + editable_requires), - pex_path=request.extra_build_time_requirements, - interpreter_constraints=request.interpreter_constraints, - ), - ) - result = await Get( ProcessResult, VenvPexProcess( build_backend_pex, - argv=(backend_shim_name,), - input_digest=build_editable_merged_digest, + argv=(backend_wrapper_name,), + input_digest=merged_digest, extra_env=extra_env, working_directory=request.working_directory, output_directories=(dist_dir,), # Relative to the working_directory. From 692afac4afa39779c9659e4503ea07f257e33719 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 3 Apr 2023 17:58:03 -0500 Subject: [PATCH 04/32] finish pep660 backend/wrapper --- .../python/util_rules/local_dists_pep660.py | 116 +++++++++++++----- 1 file changed, 88 insertions(+), 28 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index a4f30e79802..0febb2d46ef 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -23,6 +23,7 @@ from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.pex_requirements import PexRequirements from pants.backend.python.util_rules.python_sources import PythonSourceFiles +from pants.base.build_root import BuildRoot from pants.build_graph.address import Address from pants.core.util_rules import system_binaries from pants.core.util_rules.source_files import SourceFiles @@ -277,24 +278,30 @@ class PEP660BuildResult: # Based on DistBuildResult # PEP 517, however, says that method is optional. So finally we fall back # to using `build_wheel` and then extract the dist-info directory and then # delete the extra wheel file. Most people shouldn't hit that path (I hope). +# NOTE: PEP 660 does not address the `.data` directory, so we ignore it. _BACKEND_WRAPPER_BOILERPLATE = """ # DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS import os +import shutil import sys +import zipfile import {build_backend_module} backend = {build_backend_object} -metadata_dir = "{metadata_dir}" +dist_dir = "{dist_dir}" +build_dir = "build" +pth_file_path = "{pth_file_path}" wheel_config_settings = {wheel_config_settings_str} # Python 2.7 doesn't have the exist_ok arg on os.makedirs(). -try: - os.makedirs(metadata_dir) -except OSError as e: - if e.errno != errno.EEXIST: - raise +for d in (dist_dir, build_dir): + try: + os.makedirs(d) + except OSError as e: + if e.errno != errno.EEXIST: + raise prepare_metadata = getattr( backend, @@ -302,25 +309,51 @@ class PEP660BuildResult: # Based on DistBuildResult getattr(backend, "prepare_metadata_for_build_wheel", None) # PEP 517 ) if prepare_metadata is not None: - metadata_path = prepare_metadata(metadata_dir, wheel_config_settings) + metadata_path = prepare_metadata(build_dir, wheel_config_settings) else: # Optional PEP 517 method not defined. Use build_wheel instead. - wheel_path = backend.build_wheel(metadata_dir, wheel_config_settings) - full_wheel_path = os.path.join(metadata_dir, wheel_path) - - import zipfile - with zipfile.ZipFile(full_wheel_path, 'r') as zip_ref: - file_names = zip_ref.namelist() - dist_info_files = [n for n in file_names if ".dist-info" in n] - zip_ref.extractall(metadata_dir, dist_info_files) + wheel_path = backend.build_wheel(build_dir, wheel_config_settings) + full_wheel_path = os.path.join(build_dir, wheel_path) + + with zipfile.ZipFile(full_wheel_path, "r") as whl: + dist_info_files = [n for n in whl.namelist() if ".dist-info/" in n] + whl.extractall(build_dir, dist_info_files) + metadata_path = os.path.dirname(dist_info_files[0]) os.unlink(full_wheel_path) -print("metadata_path: {{metadata_path}}".format(metadata_path=metadata_path)) +for file in os.listdir(os.path.join(build_dir, metadata_path)): + # PEP 660 excludes RECORD and RECORD.* (signatures) + if file == "RECORD" or file.startswith("RECORD."): + os.unlink(os.path.join(build_dir, metadata_path, file)) + +pkg, version = metadata_path.replace(".dist_info", "").split("-") +pth_file_dest = pkg + "__pants__.pth" + +build_tag = "0.editable" # copy of what setuptools uses +lang_tag = "py3" +abi_tag = "none" +platform_tag = "any" +wheel_path = "{}-{}-{}-{}-{}-{}.whl".format( + pkg, version, build_tag, lang_tag, abi_tag, platform_tag +) +with zipfile.ZipFile(os.path.join(dist_dir, wheel_name), "w") as whl: + whl.write(pth_file_path, pth_file_dest) + # based on wheel.wheelfile.WheelFile.write_files (MIT license) + for root, dirnames, filenames in os.walk(metadata_path): + dirnames.sort() + for name in sorted(filenames): + path = os.path.normpath(os.path.join(root, name)) + if os.path.isfile(path): + arcname = os.path.join(metadata_path, path) + self.write(path, arcname) + +print("editable_path: {{editable_path}}".format(editable_path=wheel_path)) """ def interpolate_backend_wrapper( - metadata_dir: str, + dist_dir: str, + pth_file_path: str, request: DistBuildRequest, ) -> bytes: # See https://www.python.org/dev/peps/pep-0517/#source-trees. @@ -335,15 +368,38 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: return _BACKEND_WRAPPER_BOILERPLATE.format( build_backend_module=module_path, build_backend_object=backend_object, - metadata_dir=metadata_dir, + dist_dir=dist_dir, + pth_file_path=pth_file_path, wheel_config_settings_str=config_settings_repr(request.wheel_config_settings), ).encode() @rule async def run_pep660_build( - request: DistBuildRequest, python_setup: PythonSetup + request: DistBuildRequest, python_setup: PythonSetup, build_root: BuildRoot ) -> PEP660BuildResult: + # Create the .pth files to add the relevant source root to PYTHONPATH + # We cannot use the build backend to do this because we do not want to tell + # it where the workspace is and risk it adding anything there. + # NOTE: We use .pth files to support ICs less than python3.7. + # A future enhancement might be to provide more precise editable + # wheels based on https://pypi.org/project/editables/, but that only + # supports python3.7+ (what pip supports as of April 2023). + # Or maybe do something like setuptools strict editable wheel. + + pth_file_contents = "" + for source_root in request.build_time_source_roots: + # can we use just the first one to only have the dist's source root? + abs_path = ( + build_root.path if source_root == "." else str(build_root.pathlib_path / source_root) + ) + pth_file_contents += f"{abs_path}\n" + pth_file_path = os.path.join(request.working_directory, "__pants__.pth") + pth_file_digest = await Get( + Digest, + CreateDigest([FileContent(pth_file_path, pth_file_contents.encode())]), + ) + # Note that this pex has no entrypoint. We use it to run our generated wrapper, which # in turn imports from and invokes the build backend. build_backend_pex = await Get( @@ -360,6 +416,7 @@ async def run_pep660_build( # This is the setuptools dist directory, not Pants's, so we hardcode to dist/. dist_dir = "dist" dist_output_dir = os.path.join(dist_dir, request.output_path) + backend_wrapper_name = "backend_wrapper.py" backend_wrapper_path = os.path.join(request.working_directory, backend_wrapper_name) backend_wrapper_digest = await Get( @@ -368,13 +425,15 @@ async def run_pep660_build( [ FileContent( backend_wrapper_path, - interpolate_backend_wrapper(dist_output_dir, request), + interpolate_backend_wrapper(dist_output_dir, pth_file_path, request), ), ] ), ) - merged_digest = await Get(Digest, MergeDigests((request.input, backend_wrapper_digest))) + merged_digest = await Get( + Digest, MergeDigests((request.input, pth_file_digest, backend_wrapper_digest)) + ) extra_env = { **(request.extra_build_time_env or {}), @@ -393,19 +452,19 @@ async def run_pep660_build( working_directory=request.working_directory, output_directories=(dist_dir,), # Relative to the working_directory. description=( - f"Run {request.build_system.build_backend} (build_editable) for {request.target_address_spec}" + f"Run {request.build_system.build_backend} to gather .dist-info for {request.target_address_spec}" if request.target_address_spec - else f"Run {request.build_system.build_backend} (build_editable)" + else f"Run {request.build_system.build_backend} to gather .dist-info" ), level=LogLevel.DEBUG, ), ) output_lines = result.stdout.decode().splitlines() - dist_type = "editable" + line_prefix = "editable_path: " editable_path = "" for line in output_lines: - if line.startswith(f"{dist_type}: "): - editable_path = os.path.join(request.output_path, line[len(dist_type) + 2 :].strip()) + if line.startswith(line_prefix): + editable_path = os.path.join(request.output_path, line[len(line_prefix) :].strip()) break # Note that output_digest paths are relative to the working_directory. @@ -415,8 +474,9 @@ async def run_pep660_build( raise BuildBackendError( softwrap( f""" - PEP 660 build backend {request.build_system.build_backend} - did not create expected editable wheel file {editable_path} + Failed to build PEP 660 editable wheel {editable_path} + (extracted dist-info from PEP 517 build backend + {request.build_system.build_backend}). """ ) ) From eb941f443a6d486045f422295220535714a87764 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 4 Apr 2023 11:00:30 -0500 Subject: [PATCH 05/32] move tags calculation out of wrapper script --- .../python/util_rules/local_dists_pep660.py | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 0febb2d46ef..c62047acd80 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -294,6 +294,7 @@ class PEP660BuildResult: # Based on DistBuildResult build_dir = "build" pth_file_path = "{pth_file_path}" wheel_config_settings = {wheel_config_settings_str} +tags = "{tags}" # Python 2.7 doesn't have the exist_ok arg on os.makedirs(). for d in (dist_dir, build_dir): @@ -306,7 +307,7 @@ class PEP660BuildResult: # Based on DistBuildResult prepare_metadata = getattr( backend, "prepare_metadata_for_build_editable", # PEP 660 - getattr(backend, "prepare_metadata_for_build_wheel", None) # PEP 517 + getattr(backend, "prepare_metadata_for_build_wheel", None), # PEP 517 ) if prepare_metadata is not None: metadata_path = prepare_metadata(build_dir, wheel_config_settings) @@ -319,7 +320,6 @@ class PEP660BuildResult: # Based on DistBuildResult dist_info_files = [n for n in whl.namelist() if ".dist-info/" in n] whl.extractall(build_dir, dist_info_files) metadata_path = os.path.dirname(dist_info_files[0]) - os.unlink(full_wheel_path) for file in os.listdir(os.path.join(build_dir, metadata_path)): # PEP 660 excludes RECORD and RECORD.* (signatures) @@ -329,23 +329,17 @@ class PEP660BuildResult: # Based on DistBuildResult pkg, version = metadata_path.replace(".dist_info", "").split("-") pth_file_dest = pkg + "__pants__.pth" -build_tag = "0.editable" # copy of what setuptools uses -lang_tag = "py3" -abi_tag = "none" -platform_tag = "any" -wheel_path = "{}-{}-{}-{}-{}-{}.whl".format( - pkg, version, build_tag, lang_tag, abi_tag, platform_tag -) -with zipfile.ZipFile(os.path.join(dist_dir, wheel_name), "w") as whl: +wheel_path = "{}-{}-{}.whl".format(pkg, version, tags) +with zipfile.ZipFile(os.path.join(dist_dir, wheel_path), "w") as whl: whl.write(pth_file_path, pth_file_dest) - # based on wheel.wheelfile.WheelFile.write_files (MIT license) + # based on wheel.wheelfile.WheelFile.write_files (by @argonholm MIT license) for root, dirnames, filenames in os.walk(metadata_path): dirnames.sort() for name in sorted(filenames): path = os.path.normpath(os.path.join(root, name)) if os.path.isfile(path): - arcname = os.path.join(metadata_path, path) - self.write(path, arcname) + arcname = os.path.join(metadata_path, path).replace(os.path.sep, "/") + whl.write(path, arcname) print("editable_path: {{editable_path}}".format(editable_path=wheel_path)) """ @@ -365,11 +359,19 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: # We assume/hope that other backends accept lists as well. return distutils_repr(None if cs is None else {k: list(v) for k, v in cs.items()}) + build_tag = "0.editable" # copy of what setuptools uses + # tag the editable wheel as widely compatible + lang_tag, abi_tag, platform_tag = "py3", "none", "any" + if request.interpreter_constraints.includes_python2(): + # assume everything has py3 support. + lang_tag = "py2.py3" + return _BACKEND_WRAPPER_BOILERPLATE.format( build_backend_module=module_path, build_backend_object=backend_object, dist_dir=dist_dir, pth_file_path=pth_file_path, + tags="-".join([build_tag, lang_tag, abi_tag, platform_tag]), wheel_config_settings_str=config_settings_repr(request.wheel_config_settings), ).encode() From d15ecb4e255affe7aae2bc2cb04f464e72942b0f Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 4 Apr 2023 11:53:15 -0500 Subject: [PATCH 06/32] reorder rules so that they get registered in an order pants can handle --- .../python/util_rules/local_dists_pep660.py | 416 +++++++++--------- 1 file changed, 208 insertions(+), 208 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index c62047acd80..70a686757b2 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -58,216 +58,11 @@ logger = logging.getLogger(__name__) -@dataclass(frozen=True) -class LocalDistPEP660Wheels: # Based on LocalDistWheels - """Contains the PEP 660 "editable" wheels isolated from a single local Python distribution.""" - - pep660_wheel_paths: tuple[str, ...] - pep660_wheels_digest: Digest - provided_files: frozenset[str] - - -@rule -async def isolate_local_dist_pep660_wheels( - dist_field_set: PythonDistributionFieldSet, - bash: BashBinary, - unzip_binary: UnzipBinary, - python_setup: PythonSetup, - union_membership: UnionMembership, -) -> LocalDistPEP660Wheels: - dist_build_request = await create_dist_build_request( - field_set=dist_field_set, - python_setup=python_setup, - union_membership=union_membership, - # editable wheel ignores build_wheel+build_sdist args - validate_wheel_sdist=False, - ) - pep660_result = await Get(PEP660BuildResult, DistBuildRequest, dist_build_request) - - # the output digest should only contain wheels, but filter to be safe. - wheels_snapshot = await Get( - Snapshot, DigestSubset(pep660_result.output, PathGlobs(["**/*.whl"])) - ) - - wheels = tuple(wheels_snapshot.files) - - if not wheels: - tgt = await Get( - WrappedTarget, - WrappedTargetRequest(dist_field_set.address, description_of_origin=""), - ) - logger.warning( - softwrap( - f""" - Encountered a dependency on the {tgt.target.alias} target at {dist_field_set.address}, - but this target does not produce a Python wheel artifact. Therefore this target's - code will be used directly from sources, without a distribution being built, - and any native extensions in it will not be built. - - See {doc_url('python-distributions')} for details on how to set up a - {tgt.target.alias} target to produce a wheel. - """ - ) - ) - - wheels_listing_result = await Get( - ProcessResult, - Process( - argv=[ - bash.path, - "-c", - f""" - set -ex - for f in {' '.join(shlex.quote(f) for f in wheels)}; do - {unzip_binary.path} -Z1 "$f" - done - """, - ], - input_digest=wheels_snapshot.digest, - description=f"List contents of editable artifacts produced by {dist_field_set.address}", - ), - ) - provided_files = set(wheels_listing_result.stdout.decode().splitlines()) - - return LocalDistPEP660Wheels(wheels, wheels_snapshot.digest, frozenset(provided_files)) - - -@dataclass(frozen=True) -class LocalDistsPEP660PexRequest: - """Request to build a PEX populated by PEP660 wheels of local dists. - - Like LocalDistsPexRequest, the local dists come from the dependency closure of a set of - addresses. Unlike LocalDistsPexRequest, the editable wheel files must not be exported or made - available to the end-user (according to PEP 660). Instead, the PEP660Pex serves as an - intermediate, internal-only, PEX that can be used to install these wheels in a virtualenv. It - follows then, that this PEX should probably not be exported for use by end-users. - """ - - addresses: Addresses - interpreter_constraints: InterpreterConstraints - # The result will return these with the sources provided by the dists subtracted out. - # This will help the caller prevent sources from appearing twice on sys.path. - sources: PythonSourceFiles - - def __init__( - self, - addresses: Iterable[Address], - *, - interpreter_constraints: InterpreterConstraints, - sources: PythonSourceFiles = PythonSourceFiles.empty(), - ) -> None: - object.__setattr__(self, "addresses", Addresses(addresses)) - object.__setattr__(self, "interpreter_constraints", interpreter_constraints) - object.__setattr__(self, "sources", sources) - - -@dataclass(frozen=True) -class LocalDistsPEP660Pex: - """A PEX file populated by PEP660 wheels of local dists. - - Can be consumed from another PEX, e.g., by adding to PEX_PATH. - - The PEX will contain installs of PEP660 wheels generate4d from local dists. Installing PEP660 - wheels creates an "editable" install such that the sys.path gets adjusted to include source - directories that are NOT part of the PEX. As such, this PEX is decidedly not hermetic or - portable and should only be used to facilitate building virtualenvs for development. - - PEP660 wheels have .dist-info metadata and the .pth files (or similar) that adjust sys.path. - - The PEX will only contain metadata for local dists and not any dependencies. For Pants generated - `setup.py` / `pyproject.toml`, the dependencies will be included in the standard resolve process - that the locally-built dists PEX is adjoined to via PEX_PATH. For hand-made `setup.py` / - `pyproject.toml` with 3rdparty dependencies not hand-mirrored into BUILD file dependencies, this - will lead to issues. See https://github.com/pantsbuild/pants/issues/13587#issuecomment-974863636 - for one way to fix this corner which is intentionally punted on for now. - - Lists the files provided by the dists on sys.path, so they can be subtracted from - sources digests, to prevent the same file ending up on sys.path twice. - """ - - pex: Pex - # The sources from the request, but with any files provided by the local dists subtracted out. - # In general, this will have a list of all of the dists' sources. - remaining_sources: PythonSourceFiles - - -@rule(desc="Building editable local distributions (PEP 660)") -async def build_editable_local_dists( - request: LocalDistsPEP660PexRequest, -) -> LocalDistsPEP660Pex: - transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)) - applicable_targets = [ - tgt for tgt in transitive_targets.closure if PythonDistributionFieldSet.is_applicable(tgt) - ] - - local_dists_wheels = await MultiGet( - Get( - LocalDistPEP660Wheels, - PythonDistributionFieldSet, - PythonDistributionFieldSet.create(target), - ) - for target in applicable_targets - ) - - provided_files: set[str] = set() - wheels: list[str] = [] - wheels_digests = [] - for local_dist_wheels in local_dists_wheels: - wheels.extend(local_dist_wheels.pep660_wheel_paths) - wheels_digests.append(local_dist_wheels.pep660_wheels_digest) - provided_files.update(local_dist_wheels.provided_files) - - wheels_digest = await Get(Digest, MergeDigests(wheels_digests)) - - editable_dists_pex = await Get( - Pex, - PexRequest( - output_filename="editable_local_dists.pex", - requirements=PexRequirements(wheels), - interpreter_constraints=request.interpreter_constraints, - additional_inputs=wheels_digest, - internal_only=True, - additional_args=["--intransitive"], - ), - ) - - if not wheels: - # The source calculations below are not (always) cheap, so we skip them if no wheels were - # produced. See https://github.com/pantsbuild/pants/issues/14561 for one possible approach - # to sharing the cost of these calculations. - return LocalDistsPEP660Pex(editable_dists_pex, request.sources) - - # TODO: maybe DRY the logic duplicated here and in build_local_dists - # We check source roots in reverse lexicographic order, - # so we'll find the innermost root that matches. - source_roots = sorted(request.sources.source_roots, reverse=True) - remaining_sources = set(request.sources.source_files.files) - unrooted_files_set = set(request.sources.source_files.unrooted_files) - for source in request.sources.source_files.files: - if source not in unrooted_files_set: - for source_root in source_roots: - source_relpath = fast_relpath_optional(source, source_root) - if source_relpath is not None and source_relpath in provided_files: - remaining_sources.remove(source) - remaining_sources_snapshot = await Get( - Snapshot, - DigestSubset( - request.sources.source_files.snapshot.digest, PathGlobs(sorted(remaining_sources)) - ), - ) - subtracted_sources = PythonSourceFiles( - SourceFiles(remaining_sources_snapshot, request.sources.source_files.unrooted_files), - request.sources.source_roots, - ) - - return LocalDistsPEP660Pex(editable_dists_pex, subtracted_sources) - - # We just use DistBuildRequest directly instead of adding a PEP660BuildRequest @dataclass(frozen=True) -class PEP660BuildResult: # Based on DistBuildResult +class PEP660BuildResult: # based on DistBuildResult output: Digest # Relpaths in the output digest. editable_wheel_path: str | None @@ -345,7 +140,7 @@ class PEP660BuildResult: # Based on DistBuildResult """ -def interpolate_backend_wrapper( +def interpolate_backend_wrapper( # based on interpolate_backend_shim dist_dir: str, pth_file_path: str, request: DistBuildRequest, @@ -377,7 +172,7 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: @rule -async def run_pep660_build( +async def run_pep660_build( # based on run_pep517_build request: DistBuildRequest, python_setup: PythonSetup, build_root: BuildRoot ) -> PEP660BuildResult: # Create the .pth files to add the relevant source root to PYTHONPATH @@ -485,6 +280,211 @@ async def run_pep660_build( return PEP660BuildResult(output_digest, editable_wheel_path=editable_path) +@dataclass(frozen=True) +class LocalDistPEP660Wheels: # based on LocalDistWheels + """Contains the PEP 660 "editable" wheels isolated from a single local Python distribution.""" + + pep660_wheel_paths: tuple[str, ...] + pep660_wheels_digest: Digest + provided_files: frozenset[str] + + +@rule +async def isolate_local_dist_pep660_wheels( # based on isolate_local_dist_wheels + dist_field_set: PythonDistributionFieldSet, + bash: BashBinary, + unzip_binary: UnzipBinary, + python_setup: PythonSetup, + union_membership: UnionMembership, +) -> LocalDistPEP660Wheels: + dist_build_request = await create_dist_build_request( + field_set=dist_field_set, + python_setup=python_setup, + union_membership=union_membership, + # editable wheel ignores build_wheel+build_sdist args + validate_wheel_sdist=False, + ) + pep660_result = await Get(PEP660BuildResult, DistBuildRequest, dist_build_request) + + # the output digest should only contain wheels, but filter to be safe. + wheels_snapshot = await Get( + Snapshot, DigestSubset(pep660_result.output, PathGlobs(["**/*.whl"])) + ) + + wheels = tuple(wheels_snapshot.files) + + if not wheels: + tgt = await Get( + WrappedTarget, + WrappedTargetRequest(dist_field_set.address, description_of_origin=""), + ) + logger.warning( + softwrap( + f""" + Encountered a dependency on the {tgt.target.alias} target at {dist_field_set.address}, + but this target does not produce a Python wheel artifact. Therefore this target's + code will be used directly from sources, without a distribution being built, + and any native extensions in it will not be built. + + See {doc_url('python-distributions')} for details on how to set up a + {tgt.target.alias} target to produce a wheel. + """ + ) + ) + + wheels_listing_result = await Get( + ProcessResult, + Process( + argv=[ + bash.path, + "-c", + f""" + set -ex + for f in {' '.join(shlex.quote(f) for f in wheels)}; do + {unzip_binary.path} -Z1 "$f" + done + """, + ], + input_digest=wheels_snapshot.digest, + description=f"List contents of editable artifacts produced by {dist_field_set.address}", + ), + ) + provided_files = set(wheels_listing_result.stdout.decode().splitlines()) + + return LocalDistPEP660Wheels(wheels, wheels_snapshot.digest, frozenset(provided_files)) + + +@dataclass(frozen=True) +class LocalDistsPEP660PexRequest: # based on LocalDistsPexRequest + """Request to build a PEX populated by PEP660 wheels of local dists. + + Like LocalDistsPexRequest, the local dists come from the dependency closure of a set of + addresses. Unlike LocalDistsPexRequest, the editable wheel files must not be exported or made + available to the end-user (according to PEP 660). Instead, the PEP660Pex serves as an + intermediate, internal-only, PEX that can be used to install these wheels in a virtualenv. It + follows then, that this PEX should probably not be exported for use by end-users. + """ + + addresses: Addresses + interpreter_constraints: InterpreterConstraints + # The result will return these with the sources provided by the dists subtracted out. + # This will help the caller prevent sources from appearing twice on sys.path. + sources: PythonSourceFiles + + def __init__( + self, + addresses: Iterable[Address], + *, + interpreter_constraints: InterpreterConstraints, + sources: PythonSourceFiles = PythonSourceFiles.empty(), + ) -> None: + object.__setattr__(self, "addresses", Addresses(addresses)) + object.__setattr__(self, "interpreter_constraints", interpreter_constraints) + object.__setattr__(self, "sources", sources) + + +@dataclass(frozen=True) +class LocalDistsPEP660Pex: # based on LocalDistsPex + """A PEX file populated by PEP660 wheels of local dists. + + Can be consumed from another PEX, e.g., by adding to PEX_PATH. + + The PEX will contain installs of PEP660 wheels generate4d from local dists. Installing PEP660 + wheels creates an "editable" install such that the sys.path gets adjusted to include source + directories that are NOT part of the PEX. As such, this PEX is decidedly not hermetic or + portable and should only be used to facilitate building virtualenvs for development. + + PEP660 wheels have .dist-info metadata and the .pth files (or similar) that adjust sys.path. + + The PEX will only contain metadata for local dists and not any dependencies. For Pants generated + `setup.py` / `pyproject.toml`, the dependencies will be included in the standard resolve process + that the locally-built dists PEX is adjoined to via PEX_PATH. For hand-made `setup.py` / + `pyproject.toml` with 3rdparty dependencies not hand-mirrored into BUILD file dependencies, this + will lead to issues. See https://github.com/pantsbuild/pants/issues/13587#issuecomment-974863636 + for one way to fix this corner which is intentionally punted on for now. + + Lists the files provided by the dists on sys.path, so they can be subtracted from + sources digests, to prevent the same file ending up on sys.path twice. + """ + + pex: Pex + # The sources from the request, but with any files provided by the local dists subtracted out. + # In general, this will have a list of all of the dists' sources. + remaining_sources: PythonSourceFiles + + +@rule(desc="Building editable local distributions (PEP 660)") +async def build_editable_local_dists( # based on build_local_dists + request: LocalDistsPEP660PexRequest, +) -> LocalDistsPEP660Pex: + transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)) + applicable_targets = [ + tgt for tgt in transitive_targets.closure if PythonDistributionFieldSet.is_applicable(tgt) + ] + + local_dists_wheels = await MultiGet( + Get( + LocalDistPEP660Wheels, + PythonDistributionFieldSet, + PythonDistributionFieldSet.create(target), + ) + for target in applicable_targets + ) + + provided_files: set[str] = set() + wheels: list[str] = [] + wheels_digests = [] + for local_dist_wheels in local_dists_wheels: + wheels.extend(local_dist_wheels.pep660_wheel_paths) + wheels_digests.append(local_dist_wheels.pep660_wheels_digest) + provided_files.update(local_dist_wheels.provided_files) + + wheels_digest = await Get(Digest, MergeDigests(wheels_digests)) + + editable_dists_pex = await Get( + Pex, + PexRequest( + output_filename="editable_local_dists.pex", + requirements=PexRequirements(wheels), + interpreter_constraints=request.interpreter_constraints, + additional_inputs=wheels_digest, + internal_only=True, + additional_args=["--intransitive"], + ), + ) + + if not wheels: + # The source calculations below are not (always) cheap, so we skip them if no wheels were + # produced. See https://github.com/pantsbuild/pants/issues/14561 for one possible approach + # to sharing the cost of these calculations. + return LocalDistsPEP660Pex(editable_dists_pex, request.sources) + + # TODO: maybe DRY the logic duplicated here and in build_local_dists + # We check source roots in reverse lexicographic order, + # so we'll find the innermost root that matches. + source_roots = sorted(request.sources.source_roots, reverse=True) + remaining_sources = set(request.sources.source_files.files) + unrooted_files_set = set(request.sources.source_files.unrooted_files) + for source in request.sources.source_files.files: + if source not in unrooted_files_set: + for source_root in source_roots: + source_relpath = fast_relpath_optional(source, source_root) + if source_relpath is not None and source_relpath in provided_files: + remaining_sources.remove(source) + remaining_sources_snapshot = await Get( + Snapshot, + DigestSubset( + request.sources.source_files.snapshot.digest, PathGlobs(sorted(remaining_sources)) + ), + ) + subtracted_sources = PythonSourceFiles( + SourceFiles(remaining_sources_snapshot, request.sources.source_files.unrooted_files), + request.sources.source_roots, + ) + + return LocalDistsPEP660Pex(editable_dists_pex, subtracted_sources) + + def rules(): return ( *collect_rules(), From b18f0002e950a8e682be34e2e9a6a0895248defb Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 4 Apr 2023 17:01:27 -0500 Subject: [PATCH 07/32] generate metadata required to build a valid wheel --- .../python/util_rules/local_dists_pep660.py | 85 ++++++++++++++++--- 1 file changed, 72 insertions(+), 13 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 70a686757b2..a069e1ec0fc 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -77,9 +77,9 @@ class PEP660BuildResult: # based on DistBuildResult _BACKEND_WRAPPER_BOILERPLATE = """ # DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS +import base64 +import hashlib import os -import shutil -import sys import zipfile import {build_backend_module} @@ -90,6 +90,7 @@ class PEP660BuildResult: # based on DistBuildResult pth_file_path = "{pth_file_path}" wheel_config_settings = {wheel_config_settings_str} tags = "{tags}" +direct_url = "{direct_url}" # Python 2.7 doesn't have the exist_ok arg on os.makedirs(). for d in (dist_dir, build_dir): @@ -105,6 +106,7 @@ class PEP660BuildResult: # based on DistBuildResult getattr(backend, "prepare_metadata_for_build_wheel", None), # PEP 517 ) if prepare_metadata is not None: + print("prepare_metadata: " + str(prepare_metadata)) metadata_path = prepare_metadata(build_dir, wheel_config_settings) else: # Optional PEP 517 method not defined. Use build_wheel instead. @@ -116,25 +118,75 @@ class PEP660BuildResult: # based on DistBuildResult whl.extractall(build_dir, dist_info_files) metadata_path = os.path.dirname(dist_info_files[0]) +# Any RECORD* file will be incorrect since we are creating the wheel. for file in os.listdir(os.path.join(build_dir, metadata_path)): - # PEP 660 excludes RECORD and RECORD.* (signatures) if file == "RECORD" or file.startswith("RECORD."): os.unlink(os.path.join(build_dir, metadata_path, file)) +metadata_wheel_file = os.path.join(build_dir, metadata_path, "WHEEL") +if not os.path.exists(metadata_wheel_file): + with open(metadata_wheel_file, "w") as f: + f.write('''\ +Wheel-Version: 1.0 +Generator: pantsbuild +Root-Is-Purelib: true +Tag: {{}} +Build: 0.editable +'''.format(tags)) + +if direct_url: + # We abuse pex to get PEP660 editable wheels installed in a virtualenv. + # Pex and pip do not know that this is an editable install. + # We can't add direct_url.json to the wheel, because that has to be added + # by the wheel installer. So, we will rename this file once installed. + direct_url_file = os.path.join(build_dir, metadata_path, "direct_url__pants__.json") + with open(direct_url_file , "w") as f: + f.write('''\ +{{{{ + "url": "{{}}", + "dir_info": {{{{ + "editable": true + }}}} +}}}} +'''.format(direct_url)) + +pkg_version = metadata_path.replace(".dist-info", "") +if "-" in pkg_version: + pkg, version = pkg_version.split("-") +else: + pkg = pkg_version + version = "" + with open(os.path.join(build_dir, metadata_path, "METADATA"), "r") as f: + lines = f.readlines() + for line in lines: + if line.startswith("Version: "): + version = line[len("Version: "):].strip() + break +pth_file_arcname = pkg + "__pants__.pth" -pkg, version = metadata_path.replace(".dist_info", "").split("-") -pth_file_dest = pkg + "__pants__.pth" +_record = [] +def record(file_path, file_arcname): + with open(file_path, "rb") as f: + file_digest = hashlib.sha256(f.read()).digest() + file_hash = "sha256=" + base64.urlsafe_b64encode(file_digest).decode().rstrip("=") + file_size = str(os.stat(file_path).st_size) + _record.append(",".join([file_arcname, file_hash, file_size])) -wheel_path = "{}-{}-{}.whl".format(pkg, version, tags) +wheel_path = "{{}}-{{}}-0.editable-{{}}.whl".format(pkg, version, tags) with zipfile.ZipFile(os.path.join(dist_dir, wheel_path), "w") as whl: - whl.write(pth_file_path, pth_file_dest) + record(pth_file_path, pth_file_arcname) + whl.write(pth_file_path, pth_file_arcname) # based on wheel.wheelfile.WheelFile.write_files (by @argonholm MIT license) - for root, dirnames, filenames in os.walk(metadata_path): + for root, dirnames, filenames in os.walk(os.path.join(build_dir, metadata_path)): dirnames.sort() for name in sorted(filenames): path = os.path.normpath(os.path.join(root, name)) if os.path.isfile(path): - arcname = os.path.join(metadata_path, path).replace(os.path.sep, "/") + arcname = os.path.relpath(path, build_dir).replace(os.path.sep, "/") + record(path, arcname) whl.write(path, arcname) + record_path = os.path.join(metadata_path, "RECORD") + _record.extend([record_path + ",,", ""]) # "" to add newline at eof + whl.writestr(record_path, os.linesep.join(_record)) print("editable_path: {{editable_path}}".format(editable_path=wheel_path)) """ @@ -143,6 +195,7 @@ class PEP660BuildResult: # based on DistBuildResult def interpolate_backend_wrapper( # based on interpolate_backend_shim dist_dir: str, pth_file_path: str, + direct_url: str, request: DistBuildRequest, ) -> bytes: # See https://www.python.org/dev/peps/pep-0517/#source-trees. @@ -154,7 +207,6 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: # We assume/hope that other backends accept lists as well. return distutils_repr(None if cs is None else {k: list(v) for k, v in cs.items()}) - build_tag = "0.editable" # copy of what setuptools uses # tag the editable wheel as widely compatible lang_tag, abi_tag, platform_tag = "py3", "none", "any" if request.interpreter_constraints.includes_python2(): @@ -166,8 +218,9 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: build_backend_object=backend_object, dist_dir=dist_dir, pth_file_path=pth_file_path, - tags="-".join([build_tag, lang_tag, abi_tag, platform_tag]), wheel_config_settings_str=config_settings_repr(request.wheel_config_settings), + tags="-".join([lang_tag, abi_tag, platform_tag]), + direct_url=direct_url, ).encode() @@ -185,13 +238,17 @@ async def run_pep660_build( # based on run_pep517_build # Or maybe do something like setuptools strict editable wheel. pth_file_contents = "" + direct_url = "" for source_root in request.build_time_source_roots: # can we use just the first one to only have the dist's source root? abs_path = ( build_root.path if source_root == "." else str(build_root.pathlib_path / source_root) ) pth_file_contents += f"{abs_path}\n" - pth_file_path = os.path.join(request.working_directory, "__pants__.pth") + if not direct_url: # use just the first source_root + direct_url = "file://" + abs_path.replace(os.path.sep, "/") + pth_file_name = "__pants__.pth" + pth_file_path = os.path.join(request.working_directory, pth_file_name) pth_file_digest = await Get( Digest, CreateDigest([FileContent(pth_file_path, pth_file_contents.encode())]), @@ -222,7 +279,9 @@ async def run_pep660_build( # based on run_pep517_build [ FileContent( backend_wrapper_path, - interpolate_backend_wrapper(dist_output_dir, pth_file_path, request), + interpolate_backend_wrapper( + dist_output_dir, pth_file_name, direct_url, request + ), ), ] ), From cfd72131f04f4312c9011c8ca3e874f24e259ba4 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 4 Apr 2023 17:01:44 -0500 Subject: [PATCH 08/32] add tests for local_dists_pep660 --- .../util_rules/local_dists_pep660_test.py | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 src/python/pants/backend/python/util_rules/local_dists_pep660_test.py diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py new file mode 100644 index 00000000000..65523e251be --- /dev/null +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -0,0 +1,187 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import io +import zipfile +from pathlib import PurePath +from textwrap import dedent + +import pytest + +from pants.backend.python import target_types_rules +from pants.backend.python.goals.setup_py import rules as setup_py_rules +from pants.backend.python.macros.python_artifact import PythonArtifact +from pants.backend.python.subsystems.setuptools import rules as setuptools_rules +from pants.backend.python.target_types import PythonDistribution, PythonSourcesGeneratorTarget +from pants.backend.python.util_rules import local_dists_pep660, pex_from_targets +from pants.backend.python.util_rules.dists import BuildSystem, DistBuildRequest +from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints +from pants.backend.python.util_rules.local_dists_pep660 import ( + LocalDistsPEP660Pex, + LocalDistsPEP660PexRequest, + PEP660BuildResult, +) +from pants.backend.python.util_rules.pex_from_targets import InterpreterConstraintsRequest +from pants.backend.python.util_rules.pex_requirements import PexRequirements +from pants.backend.python.util_rules.python_sources import PythonSourceFiles +from pants.build_graph.address import Address +from pants.core.util_rules.source_files import SourceFiles +from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent, Snapshot +from pants.testutil.python_interpreter_selection import ( + skip_unless_python27_present, + skip_unless_python39_present, +) +from pants.testutil.rule_runner import QueryRule, RuleRunner +from pants.util.frozendict import FrozenDict + + +@pytest.fixture +def rule_runner() -> RuleRunner: + ret = RuleRunner( + rules=[ + *local_dists_pep660.rules(), + *setup_py_rules(), + *setuptools_rules(), + *target_types_rules.rules(), + *pex_from_targets.rules(), + QueryRule(InterpreterConstraints, (InterpreterConstraintsRequest,)), + QueryRule(LocalDistsPEP660Pex, (LocalDistsPEP660PexRequest,)), + QueryRule(PEP660BuildResult, (DistBuildRequest,)), + ], + target_types=[PythonSourcesGeneratorTarget, PythonDistribution], + objects={"python_artifact": PythonArtifact}, + ) + ret.set_options( + [], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) + return ret + + +def do_test_backend_wrapper(rule_runner: RuleRunner, constraints: str) -> None: + setup_py = "from setuptools import setup; setup(name='foobar', version='1.2.3')" + input_digest = rule_runner.request( + Digest, [CreateDigest([FileContent("setup.py", setup_py.encode())])] + ) + req = DistBuildRequest( + build_system=BuildSystem( + PexRequirements( + # NB: These are the last versions compatible with Python 2.7. + ["setuptools==44.1.1", "wheel==0.37.1"] + ), + "setuptools.build_meta", + ), + interpreter_constraints=InterpreterConstraints([constraints]), + build_wheel=True, + build_sdist=True, + input=input_digest, + working_directory="", + build_time_source_roots=tuple(), + output_path="dist", + wheel_config_settings=FrozenDict({"setting1": ("value1",), "setting2": ("value2",)}), + ) + res = rule_runner.request(PEP660BuildResult, [req]) + + is_py2 = "2.7" in constraints + assert ( + res.editable_wheel_path + == f"dist/foobar-1.2.3-0.editable-{'py2.' if is_py2 else ''}py3-none-any.whl" + ) + + +@skip_unless_python27_present +def test_works_with_python2(rule_runner: RuleRunner) -> None: + do_test_backend_wrapper(rule_runner, constraints="CPython==2.7.*") + + +@skip_unless_python39_present +def test_works_with_python39(rule_runner: RuleRunner) -> None: + do_test_backend_wrapper(rule_runner, constraints="CPython==3.9.*") + + +def test_build_editable_local_dists(rule_runner: RuleRunner) -> None: + foo = PurePath("foo") + rule_runner.write_files( + { + foo + / "BUILD": dedent( + """ + python_sources() + + python_distribution( + name="dist", + dependencies=[":foo"], + provides=python_artifact(name="foo", version="9.8.7"), + sdist=False, + generate_setup=False, + ) + """ + ), + foo / "bar.py": "BAR = 42", + foo + / "setup.py": dedent( + """ + from setuptools import setup + + setup( + name="foo", + version="9.8.7", + packages=["foo"], + package_dir={"foo": "."}, + entry_points={"foo.plugins": ["bar = foo.bar.BAR"]}, + ) + """ + ), + } + ) + rule_runner.set_options([], env_inherit={"PATH"}) + sources_digest = rule_runner.request( + Digest, + [ + CreateDigest( + [FileContent("srcroot/foo/bar.py", b""), FileContent("srcroot/foo/qux.py", b"")] + ) + ], + ) + sources_snapshot = rule_runner.request(Snapshot, [sources_digest]) + sources = PythonSourceFiles(SourceFiles(sources_snapshot, tuple()), ("srcroot",)) + addresses = [Address("foo", target_name="dist")] + interpreter_constraints = rule_runner.request( + InterpreterConstraints, [InterpreterConstraintsRequest(addresses)] + ) + request = LocalDistsPEP660PexRequest( + addresses, + sources=sources, + interpreter_constraints=interpreter_constraints, + ) + result = rule_runner.request(LocalDistsPEP660Pex, [request]) + + assert result.pex is not None + contents = rule_runner.request(DigestContents, [result.pex.digest]) + whl_content = None + for content in contents: + if content.path == "editable_local_dists.pex/.deps/foo-9.8.7-0.editable-py3-none-any.whl": + whl_content = content + assert whl_content + with io.BytesIO(whl_content.content) as fp: + with zipfile.ZipFile(fp, "r") as whl: + whl_files = whl.namelist() + # Check that sources are not present in editable wheel + assert "foo/bar.py" not in whl_files + assert "foo/qux.py" not in whl_files + # Once installed, wheel does not have RECORD + assert "foo-9.8.7.dist-info/RECORD" not in whl_files + # Check that pth and metadata files are present in editable wheel + assert "foo__pants__.pth" in whl_files + assert "foo-9.8.7.dist-info/METADATA" in whl_files + assert "foo-9.8.7.dist-info/WHEEL" in whl_files + assert "foo-9.8.7.dist-info/direct_url__pants__.json" in whl_files + assert "foo-9.8.7.dist-info/entry_points.txt" in whl_files + + # Check that all sources are not in the editable wheel + assert result.remaining_sources.source_files.files == ( + "srcroot/foo/bar.py", + "srcroot/foo/qux.py", + ) From 7a52ce08257586d6e0590969a05f79d69e7c0881 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 4 Apr 2023 17:47:40 -0500 Subject: [PATCH 09/32] simplify LocalDistsPEP660Pex by dropping pointless remaining_sources --- .../python/util_rules/local_dists_pep660.py | 38 +------------------ .../util_rules/local_dists_pep660_test.py | 6 --- 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index a069e1ec0fc..82f8c09149a 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -26,7 +26,6 @@ from pants.base.build_root import BuildRoot from pants.build_graph.address import Address from pants.core.util_rules import system_binaries -from pants.core.util_rules.source_files import SourceFiles from pants.core.util_rules.system_binaries import BashBinary, UnzipBinary from pants.engine.addresses import Addresses from pants.engine.fs import ( @@ -48,7 +47,6 @@ WrappedTargetRequest, ) from pants.engine.unions import UnionMembership -from pants.util.dirutil import fast_relpath_optional from pants.util.docutil import doc_url from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel @@ -467,9 +465,6 @@ class LocalDistsPEP660Pex: # based on LocalDistsPex """ pex: Pex - # The sources from the request, but with any files provided by the local dists subtracted out. - # In general, this will have a list of all of the dists' sources. - remaining_sources: PythonSourceFiles @rule(desc="Building editable local distributions (PEP 660)") @@ -490,13 +485,11 @@ async def build_editable_local_dists( # based on build_local_dists for target in applicable_targets ) - provided_files: set[str] = set() wheels: list[str] = [] wheels_digests = [] for local_dist_wheels in local_dists_wheels: wheels.extend(local_dist_wheels.pep660_wheel_paths) wheels_digests.append(local_dist_wheels.pep660_wheels_digest) - provided_files.update(local_dist_wheels.provided_files) wheels_digest = await Get(Digest, MergeDigests(wheels_digests)) @@ -512,36 +505,7 @@ async def build_editable_local_dists( # based on build_local_dists ), ) - if not wheels: - # The source calculations below are not (always) cheap, so we skip them if no wheels were - # produced. See https://github.com/pantsbuild/pants/issues/14561 for one possible approach - # to sharing the cost of these calculations. - return LocalDistsPEP660Pex(editable_dists_pex, request.sources) - - # TODO: maybe DRY the logic duplicated here and in build_local_dists - # We check source roots in reverse lexicographic order, - # so we'll find the innermost root that matches. - source_roots = sorted(request.sources.source_roots, reverse=True) - remaining_sources = set(request.sources.source_files.files) - unrooted_files_set = set(request.sources.source_files.unrooted_files) - for source in request.sources.source_files.files: - if source not in unrooted_files_set: - for source_root in source_roots: - source_relpath = fast_relpath_optional(source, source_root) - if source_relpath is not None and source_relpath in provided_files: - remaining_sources.remove(source) - remaining_sources_snapshot = await Get( - Snapshot, - DigestSubset( - request.sources.source_files.snapshot.digest, PathGlobs(sorted(remaining_sources)) - ), - ) - subtracted_sources = PythonSourceFiles( - SourceFiles(remaining_sources_snapshot, request.sources.source_files.unrooted_files), - request.sources.source_roots, - ) - - return LocalDistsPEP660Pex(editable_dists_pex, subtracted_sources) + return LocalDistsPEP660Pex(editable_dists_pex) def rules(): diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 65523e251be..8bf43e3a385 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -179,9 +179,3 @@ def test_build_editable_local_dists(rule_runner: RuleRunner) -> None: assert "foo-9.8.7.dist-info/WHEEL" in whl_files assert "foo-9.8.7.dist-info/direct_url__pants__.json" in whl_files assert "foo-9.8.7.dist-info/entry_points.txt" in whl_files - - # Check that all sources are not in the editable wheel - assert result.remaining_sources.source_files.files == ( - "srcroot/foo/bar.py", - "srcroot/foo/qux.py", - ) From 690955025b1fe22101d5c279ecae4e540dc5d3cf Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 4 Apr 2023 17:54:14 -0500 Subject: [PATCH 10/32] simplify LocalDistsPEP660PexRequest by dropping pointless sources --- .../python/util_rules/local_dists_pep660.py | 6 ------ .../python/util_rules/local_dists_pep660_test.py | 16 +--------------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 82f8c09149a..865b40e60e6 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -22,7 +22,6 @@ from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.pex_requirements import PexRequirements -from pants.backend.python.util_rules.python_sources import PythonSourceFiles from pants.base.build_root import BuildRoot from pants.build_graph.address import Address from pants.core.util_rules import system_binaries @@ -424,20 +423,15 @@ class LocalDistsPEP660PexRequest: # based on LocalDistsPexRequest addresses: Addresses interpreter_constraints: InterpreterConstraints - # The result will return these with the sources provided by the dists subtracted out. - # This will help the caller prevent sources from appearing twice on sys.path. - sources: PythonSourceFiles def __init__( self, addresses: Iterable[Address], *, interpreter_constraints: InterpreterConstraints, - sources: PythonSourceFiles = PythonSourceFiles.empty(), ) -> None: object.__setattr__(self, "addresses", Addresses(addresses)) object.__setattr__(self, "interpreter_constraints", interpreter_constraints) - object.__setattr__(self, "sources", sources) @dataclass(frozen=True) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 8bf43e3a385..3a2956d5b2c 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -25,10 +25,8 @@ ) from pants.backend.python.util_rules.pex_from_targets import InterpreterConstraintsRequest from pants.backend.python.util_rules.pex_requirements import PexRequirements -from pants.backend.python.util_rules.python_sources import PythonSourceFiles from pants.build_graph.address import Address -from pants.core.util_rules.source_files import SourceFiles -from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent, Snapshot +from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent from pants.testutil.python_interpreter_selection import ( skip_unless_python27_present, skip_unless_python39_present, @@ -136,24 +134,12 @@ def test_build_editable_local_dists(rule_runner: RuleRunner) -> None: ), } ) - rule_runner.set_options([], env_inherit={"PATH"}) - sources_digest = rule_runner.request( - Digest, - [ - CreateDigest( - [FileContent("srcroot/foo/bar.py", b""), FileContent("srcroot/foo/qux.py", b"")] - ) - ], - ) - sources_snapshot = rule_runner.request(Snapshot, [sources_digest]) - sources = PythonSourceFiles(SourceFiles(sources_snapshot, tuple()), ("srcroot",)) addresses = [Address("foo", target_name="dist")] interpreter_constraints = rule_runner.request( InterpreterConstraints, [InterpreterConstraintsRequest(addresses)] ) request = LocalDistsPEP660PexRequest( addresses, - sources=sources, interpreter_constraints=interpreter_constraints, ) result = rule_runner.request(LocalDistsPEP660Pex, [request]) From d86a9a7f85210888e3c56a03ca382f67b464ac74 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 5 Apr 2023 20:08:59 -0500 Subject: [PATCH 11/32] wire up export to install editable wheels in mutable resolves for user resolves --- .../pants/backend/python/goals/export.py | 70 +++++++++++++++---- .../pants/backend/python/goals/export_test.py | 58 +++++++++------ .../python/util_rules/local_dists_pep660.py | 58 ++++++++------- .../util_rules/local_dists_pep660_test.py | 6 +- 4 files changed, 132 insertions(+), 60 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 1953847c8f6..d5752843e3a 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -12,9 +12,15 @@ from enum import Enum from typing import Any, DefaultDict, Iterable, cast +from pants.backend.python.goals.setup_py import rules as setup_py_rules from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.target_types import PexLayout, PythonResolveField from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints +from pants.backend.python.util_rules.local_dists_pep660 import ( + LocalDistsPEP660Pex, + LocalDistsPEP660PexRequest, +) +from pants.backend.python.util_rules.local_dists_pep660 import rules as local_dists_pep660_rules from pants.backend.python.util_rules.pex import Pex, PexProcess, PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.pex_cli import PexPEX from pants.backend.python.util_rules.pex_environment import PexEnvironment @@ -150,6 +156,8 @@ class VenvExportRequest: dest_prefix: str resolve_name: str qualify_path_with_python_version: bool + # this can only be used for mutable venvs in the new code path + editable_dists_pex_request: LocalDistsPEP660PexRequest | None = None @rule @@ -225,35 +233,64 @@ async def do_export( f"(using Python {py_version})" ) - merged_digest = await Get(Digest, MergeDigests([pex_pex.digest, requirements_pex.digest])) + digests = [pex_pex.digest, requirements_pex.digest] + if req.editable_dists_pex_request is not None: + editable_local_dists = await Get( + LocalDistsPEP660Pex, LocalDistsPEP660PexRequest, req.editable_dists_pex_request + ) + digests.append(editable_local_dists.pex.digest) + + merged_digest = await Get(Digest, MergeDigests(digests)) tmpdir_prefix = f".{uuid.uuid4().hex}.tmp" tmpdir_under_digest_root = os.path.join("{digest_root}", tmpdir_prefix) merged_digest_under_tmpdir = await Get(Digest, AddPrefix(merged_digest, tmpdir_prefix)) - return ExportResult( - description, - dest, - digest=merged_digest_under_tmpdir, - post_processing_cmds=[ + post_processing_cmds: list[PostProcessingCommand] = [ + 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), + "PEX_MODULE": "pex.tools", + }, + ), + # Remove the requirements and pex pexes, to avoid confusion. + PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), + ] + + if req.editable_dists_pex_request is not None: + post_processing_cmds.insert( + 1, 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), + os.path.join(tmpdir_under_digest_root, editable_local_dists.pex.name), "venv", - "--pip", "--collisions-ok", output_path, ], ), { - **complete_pex_env.environment_dict(python=requirements_pex.python), + **complete_pex_env.environment_dict(python=editable_local_dists.pex.python), "PEX_MODULE": "pex.tools", }, ), - # Remove the requirements and pex pexes, to avoid confusion. - PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), - ], + ) + + return ExportResult( + description, + dest, + digest=merged_digest_under_tmpdir, + post_processing_cmds=post_processing_cmds, resolve=req.resolve_name or None, ) else: @@ -271,6 +308,7 @@ async def export_virtualenv_for_resolve( python_setup: PythonSetup, union_membership: UnionMembership, ) -> MaybeExportResult: + editable_dists_pex_request: LocalDistsPEP660PexRequest | None = None resolve = request.resolve lockfile_path = python_setup.resolves.get(resolve) if lockfile_path: @@ -296,6 +334,11 @@ async def export_virtualenv_for_resolve( # Packed layout should lead to the best performance in this use case. layout=PexLayout.PACKED, ) + + editable_dists_pex_request = LocalDistsPEP660PexRequest( + interpreter_constraints=interpreter_constraints, + resolve=resolve, + ) else: # It's a tool resolve. # TODO: Can we simplify tool lockfiles to be more uniform with user lockfiles? @@ -337,6 +380,7 @@ async def export_virtualenv_for_resolve( dest_prefix, resolve, qualify_path_with_python_version=True, + editable_dists_pex_request=editable_dists_pex_request, ), ) return MaybeExportResult(export_result) @@ -488,6 +532,8 @@ async def export_virtualenvs( def rules(): return [ *collect_rules(), + *local_dists_pep660_rules(), + *setup_py_rules(), Export.subsystem_cls.register_plugin_options(ExportPluginOptions), UnionRule(ExportRequest, ExportVenvsRequest), ] diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index 331ae86c2bd..367656743d5 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -208,34 +208,46 @@ def test_export_venv_new_codepath( assert ppc1.argv[3] == "{digest_root}" assert ppc1.extra_env == FrozenDict() else: - assert len(result.post_processing_cmds) == 2 + assert len(result.post_processing_cmds) in [2, 3] - ppc0 = result.post_processing_cmds[0] - # The first arg is the full path to the python interpreter, which we - # don't easily know here, so we ignore it in this comparison. + pex_ppcs = result.post_processing_cmds[:1] + if len(result.post_processing_cmds) == 3: + pex_ppcs = result.post_processing_cmds[:2] + for index, ppc in enumerate(pex_ppcs): + # The first arg is the full path to the python interpreter, which we + # don't easily know here, so we ignore it in this comparison. - # The second arg is expected to be tmpdir/./pex. - tmpdir, pex_pex_name = os.path.split(os.path.normpath(ppc0.argv[1])) - assert pex_pex_name == "pex" - assert re.match(r"\{digest_root\}/\.[0-9a-f]{32}\.tmp", tmpdir) + # The second arg is expected to be tmpdir/./pex. + tmpdir, pex_pex_name = os.path.split(os.path.normpath(ppc.argv[1])) + assert pex_pex_name == "pex" + assert re.match(r"\{digest_root\}/\.[0-9a-f]{32}\.tmp", tmpdir) - # The third arg is expected to be tmpdir/{resolve}.pex. - req_pex_dir, req_pex_name = os.path.split(ppc0.argv[2]) - assert req_pex_dir == tmpdir - assert req_pex_name == f"{resolve}.pex" + # The third arg is expected to be tmpdir/{resolve}.pex. + req_pex_dir, req_pex_name = os.path.split(ppc.argv[2]) + assert req_pex_dir == tmpdir - assert ppc0.argv[3:] == ( - "venv", - "--pip", - "--collisions-ok", - "{digest_root}", - ) - assert ppc0.extra_env["PEX_MODULE"] == "pex.tools" - assert ppc0.extra_env.get("PEX_ROOT") is not None + if index == 0: + assert req_pex_name == f"{resolve}.pex" + assert ppc.argv[3:] == ( + "venv", + "--pip", + "--collisions-ok", + "{digest_root}", + ) + elif index == 1: + assert req_pex_name == "editable_local_dists.pex" + assert ppc.argv[3:] == ( + "venv", + "--collisions-ok", + "{digest_root}", + ) - ppc1 = result.post_processing_cmds[1] - assert ppc1.argv == ("rm", "-rf", tmpdir) - assert ppc1.extra_env == FrozenDict() + assert ppc.extra_env["PEX_MODULE"] == "pex.tools" + assert ppc.extra_env.get("PEX_ROOT") is not None + + ppc_last = result.post_processing_cmds[-1] + assert ppc_last.argv == ("rm", "-rf", tmpdir) + assert ppc_last.extra_env == FrozenDict() reldirs = [result.reldir for result in all_results] assert reldirs == [ diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 865b40e60e6..2bd50e09d8a 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -6,13 +6,14 @@ import logging import os import shlex +from collections import defaultdict from dataclasses import dataclass -from typing import Iterable # TODO: move this to a util_rules file from pants.backend.python.goals.setup_py import create_dist_build_request from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet +from pants.backend.python.target_types import PythonProvidesField, PythonResolveField from pants.backend.python.util_rules.dists import ( BuildBackendError, DistBuildRequest, @@ -23,10 +24,8 @@ from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.pex_requirements import PexRequirements from pants.base.build_root import BuildRoot -from pants.build_graph.address import Address from pants.core.util_rules import system_binaries from pants.core.util_rules.system_binaries import BashBinary, UnzipBinary -from pants.engine.addresses import Addresses from pants.engine.fs import ( CreateDigest, Digest, @@ -39,12 +38,7 @@ ) from pants.engine.process import Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import ( - TransitiveTargets, - TransitiveTargetsRequest, - WrappedTarget, - WrappedTargetRequest, -) +from pants.engine.target import AllTargets, Target, WrappedTarget, WrappedTargetRequest from pants.engine.unions import UnionMembership from pants.util.docutil import doc_url from pants.util.frozendict import FrozenDict @@ -410,6 +404,31 @@ async def isolate_local_dist_pep660_wheels( # based on isolate_local_dist_wheel return LocalDistPEP660Wheels(wheels, wheels_snapshot.digest, frozenset(provided_files)) +@dataclass(frozen=True) +class AllPythonDistributionTargets: + targets: FrozenDict[str | None, tuple[Target, ...]] + + +@rule(desc="Find all Python Distribution targets in project", level=LogLevel.DEBUG) +def find_all_python_distributions( + all_targets: AllTargets, + python_setup: PythonSetup, +) -> AllPythonDistributionTargets: + dists = defaultdict(list) + for tgt in all_targets: + # this is the field used in PythonDistributionFieldSet + if tgt.has_field(PythonProvidesField): + resolve = ( + tgt.get(PythonResolveField).normalized_value(python_setup) + if python_setup.enable_resolves + else None + ) + dists[resolve].append(tgt) + return AllPythonDistributionTargets( + FrozenDict({resolve: tuple(targets) for resolve, targets in dists.items()}) + ) + + @dataclass(frozen=True) class LocalDistsPEP660PexRequest: # based on LocalDistsPexRequest """Request to build a PEX populated by PEP660 wheels of local dists. @@ -421,17 +440,8 @@ class LocalDistsPEP660PexRequest: # based on LocalDistsPexRequest follows then, that this PEX should probably not be exported for use by end-users. """ - addresses: Addresses interpreter_constraints: InterpreterConstraints - - def __init__( - self, - addresses: Iterable[Address], - *, - interpreter_constraints: InterpreterConstraints, - ) -> None: - object.__setattr__(self, "addresses", Addresses(addresses)) - object.__setattr__(self, "interpreter_constraints", interpreter_constraints) + resolve: str | None # None if resolves is not enabled @dataclass(frozen=True) @@ -464,11 +474,11 @@ class LocalDistsPEP660Pex: # based on LocalDistsPex @rule(desc="Building editable local distributions (PEP 660)") async def build_editable_local_dists( # based on build_local_dists request: LocalDistsPEP660PexRequest, + all_dists: AllPythonDistributionTargets, + python_setup: PythonSetup, ) -> LocalDistsPEP660Pex: - transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)) - applicable_targets = [ - tgt for tgt in transitive_targets.closure if PythonDistributionFieldSet.is_applicable(tgt) - ] + resolve = request.resolve if python_setup.enable_resolves else None + resolve_dists = all_dists.targets.get(resolve, ()) local_dists_wheels = await MultiGet( Get( @@ -476,7 +486,7 @@ async def build_editable_local_dists( # based on build_local_dists PythonDistributionFieldSet, PythonDistributionFieldSet.create(target), ) - for target in applicable_targets + for target in resolve_dists ) wheels: list[str] = [] diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 3a2956d5b2c..282bf8135b0 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -19,6 +19,7 @@ from pants.backend.python.util_rules.dists import BuildSystem, DistBuildRequest from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.local_dists_pep660 import ( + AllPythonDistributionTargets, LocalDistsPEP660Pex, LocalDistsPEP660PexRequest, PEP660BuildResult, @@ -45,6 +46,7 @@ def rule_runner() -> RuleRunner: *target_types_rules.rules(), *pex_from_targets.rules(), QueryRule(InterpreterConstraints, (InterpreterConstraintsRequest,)), + QueryRule(AllPythonDistributionTargets, ()), QueryRule(LocalDistsPEP660Pex, (LocalDistsPEP660PexRequest,)), QueryRule(PEP660BuildResult, (DistBuildRequest,)), ], @@ -134,13 +136,15 @@ def test_build_editable_local_dists(rule_runner: RuleRunner) -> None: ), } ) + # all_dists = rule_runner.request(AllPythonDistributionTargets, ()) + # assert not all_dists addresses = [Address("foo", target_name="dist")] interpreter_constraints = rule_runner.request( InterpreterConstraints, [InterpreterConstraintsRequest(addresses)] ) request = LocalDistsPEP660PexRequest( - addresses, interpreter_constraints=interpreter_constraints, + resolve=None, # resolves is disabled ) result = rule_runner.request(LocalDistsPEP660Pex, [request]) From cec0583c7a4e1624a7865a01e2cc5277779f74b4 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 5 Apr 2023 20:57:25 -0500 Subject: [PATCH 12/32] get the resolve for python_distributions which do not have a resolve field --- .../pants/backend/python/goals/export.py | 2 - .../python/util_rules/local_dists_pep660.py | 64 ++++++++++++++----- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index d5752843e3a..de9e45994f5 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -12,7 +12,6 @@ from enum import Enum from typing import Any, DefaultDict, Iterable, cast -from pants.backend.python.goals.setup_py import rules as setup_py_rules from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.target_types import PexLayout, PythonResolveField from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints @@ -533,7 +532,6 @@ def rules(): return [ *collect_rules(), *local_dists_pep660_rules(), - *setup_py_rules(), Export.subsystem_cls.register_plugin_options(ExportPluginOptions), UnionRule(ExportRequest, ExportVenvsRequest), ] diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 2bd50e09d8a..eb7362982f3 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -10,7 +10,13 @@ from dataclasses import dataclass # TODO: move this to a util_rules file -from pants.backend.python.goals.setup_py import create_dist_build_request +from pants.backend.python.goals import setup_py +from pants.backend.python.goals.setup_py import ( + DependencyOwner, + ExportedTarget, + OwnedDependencies, + create_dist_build_request, +) from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet from pants.backend.python.target_types import PythonProvidesField, PythonResolveField @@ -38,7 +44,7 @@ ) from pants.engine.process import Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import AllTargets, Target, WrappedTarget, WrappedTargetRequest +from pants.engine.target import AllTargets, Target, Targets, WrappedTarget, WrappedTargetRequest from pants.engine.unions import UnionMembership from pants.util.docutil import doc_url from pants.util.frozendict import FrozenDict @@ -406,25 +412,52 @@ async def isolate_local_dist_pep660_wheels( # based on isolate_local_dist_wheel @dataclass(frozen=True) class AllPythonDistributionTargets: - targets: FrozenDict[str | None, tuple[Target, ...]] + targets: Targets @rule(desc="Find all Python Distribution targets in project", level=LogLevel.DEBUG) def find_all_python_distributions( all_targets: AllTargets, - python_setup: PythonSetup, ) -> AllPythonDistributionTargets: - dists = defaultdict(list) - for tgt in all_targets: - # this is the field used in PythonDistributionFieldSet - if tgt.has_field(PythonProvidesField): - resolve = ( - tgt.get(PythonResolveField).normalized_value(python_setup) - if python_setup.enable_resolves - else None - ) - dists[resolve].append(tgt) return AllPythonDistributionTargets( + # 'provides' is the field used in PythonDistributionFieldSet + Targets(tgt for tgt in all_targets if tgt.has_field(PythonProvidesField)) + ) + + +@dataclass(frozen=True) +class ResolveSortedPythonDistributionTargets: + targets: FrozenDict[str | None, tuple[Target, ...]] + + +@rule( + desc="Associate resolves with all Python Distribution targets in project", level=LogLevel.DEBUG +) +async def sort_all_python_distributions_by_resolve( + all_dists: AllPythonDistributionTargets, + python_setup: PythonSetup, +) -> ResolveSortedPythonDistributionTargets: + dists = defaultdict(list) + + if not python_setup.enable_resolves: + resolve = None + return ResolveSortedPythonDistributionTargets( + FrozenDict({resolve: tuple(all_dists.targets)}) + ) + + dist_owned_deps = await MultiGet( + Get(OwnedDependencies, DependencyOwner(ExportedTarget(tgt))) for tgt in all_dists.targets + ) + + for dist, owned_deps in zip(all_dists.targets, dist_owned_deps): + resolve = None + # assumption: all owned deps are in the same resolve + for dep in owned_deps: + if dep.target.has_field(PythonResolveField): + resolve = dep.target[PythonResolveField].normalized_value(python_setup) + break + dists[resolve].append(dist) + return ResolveSortedPythonDistributionTargets( FrozenDict({resolve: tuple(targets) for resolve, targets in dists.items()}) ) @@ -474,7 +507,7 @@ class LocalDistsPEP660Pex: # based on LocalDistsPex @rule(desc="Building editable local distributions (PEP 660)") async def build_editable_local_dists( # based on build_local_dists request: LocalDistsPEP660PexRequest, - all_dists: AllPythonDistributionTargets, + all_dists: ResolveSortedPythonDistributionTargets, python_setup: PythonSetup, ) -> LocalDistsPEP660Pex: resolve = request.resolve if python_setup.enable_resolves else None @@ -516,5 +549,6 @@ def rules(): return ( *collect_rules(), *dists_rules(), + *setup_py.rules(), *system_binaries.rules(), ) From 750c56272ccc7eee9e848729fbba911c4f90455a Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 5 Apr 2023 22:42:22 -0500 Subject: [PATCH 13/32] revert some export changes to simplify editable dists implementation --- .../pants/backend/python/goals/export.py | 75 +++++++------------ .../pants/backend/python/goals/export_test.py | 58 ++++++-------- .../python/util_rules/local_dists_pep660.py | 5 +- 3 files changed, 52 insertions(+), 86 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index de9e45994f5..443f6249717 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -155,8 +155,6 @@ class VenvExportRequest: dest_prefix: str resolve_name: str qualify_path_with_python_version: bool - # this can only be used for mutable venvs in the new code path - editable_dists_pex_request: LocalDistsPEP660PexRequest | None = None @rule @@ -232,64 +230,35 @@ async def do_export( f"(using Python {py_version})" ) - digests = [pex_pex.digest, requirements_pex.digest] - if req.editable_dists_pex_request is not None: - editable_local_dists = await Get( - LocalDistsPEP660Pex, LocalDistsPEP660PexRequest, req.editable_dists_pex_request - ) - digests.append(editable_local_dists.pex.digest) - - merged_digest = await Get(Digest, MergeDigests(digests)) + merged_digest = await Get(Digest, MergeDigests([pex_pex.digest, requirements_pex.digest])) tmpdir_prefix = f".{uuid.uuid4().hex}.tmp" tmpdir_under_digest_root = os.path.join("{digest_root}", tmpdir_prefix) merged_digest_under_tmpdir = await Get(Digest, AddPrefix(merged_digest, tmpdir_prefix)) - post_processing_cmds: list[PostProcessingCommand] = [ - 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), - "PEX_MODULE": "pex.tools", - }, - ), - # Remove the requirements and pex pexes, to avoid confusion. - PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), - ] - - if req.editable_dists_pex_request is not None: - post_processing_cmds.insert( - 1, + return ExportResult( + description, + dest, + digest=merged_digest_under_tmpdir, + post_processing_cmds=[ PostProcessingCommand( complete_pex_env.create_argv( os.path.join(tmpdir_under_digest_root, pex_pex.exe), *[ - os.path.join(tmpdir_under_digest_root, editable_local_dists.pex.name), + os.path.join(tmpdir_under_digest_root, requirements_pex.name), "venv", + "--pip", "--collisions-ok", output_path, ], ), { - **complete_pex_env.environment_dict(python=editable_local_dists.pex.python), + **complete_pex_env.environment_dict(python=requirements_pex.python), "PEX_MODULE": "pex.tools", }, ), - ) - - return ExportResult( - description, - dest, - digest=merged_digest_under_tmpdir, - post_processing_cmds=post_processing_cmds, + # Remove the requirements and pex pexes, to avoid confusion. + PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), + ], resolve=req.resolve_name or None, ) else: @@ -307,7 +276,6 @@ async def export_virtualenv_for_resolve( python_setup: PythonSetup, union_membership: UnionMembership, ) -> MaybeExportResult: - editable_dists_pex_request: LocalDistsPEP660PexRequest | None = None resolve = request.resolve lockfile_path = python_setup.resolves.get(resolve) if lockfile_path: @@ -324,6 +292,18 @@ async def export_virtualenv_for_resolve( ) ) + pex_path = [] + + editable_local_dists = await Get( + LocalDistsPEP660Pex, + LocalDistsPEP660PexRequest( + interpreter_constraints=interpreter_constraints, + resolve=resolve, + ), + ) + if editable_local_dists.pex: + pex_path.append(editable_local_dists.pex) + pex_request = PexRequest( description=f"Build pex for resolve `{resolve}`", output_filename=f"{path_safe(resolve)}.pex", @@ -332,11 +312,7 @@ async def export_virtualenv_for_resolve( interpreter_constraints=interpreter_constraints, # Packed layout should lead to the best performance in this use case. layout=PexLayout.PACKED, - ) - - editable_dists_pex_request = LocalDistsPEP660PexRequest( - interpreter_constraints=interpreter_constraints, - resolve=resolve, + pex_path=pex_path, ) else: # It's a tool resolve. @@ -379,7 +355,6 @@ async def export_virtualenv_for_resolve( dest_prefix, resolve, qualify_path_with_python_version=True, - editable_dists_pex_request=editable_dists_pex_request, ), ) return MaybeExportResult(export_result) diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index 367656743d5..331ae86c2bd 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -208,46 +208,34 @@ def test_export_venv_new_codepath( assert ppc1.argv[3] == "{digest_root}" assert ppc1.extra_env == FrozenDict() else: - assert len(result.post_processing_cmds) in [2, 3] - - pex_ppcs = result.post_processing_cmds[:1] - if len(result.post_processing_cmds) == 3: - pex_ppcs = result.post_processing_cmds[:2] - for index, ppc in enumerate(pex_ppcs): - # The first arg is the full path to the python interpreter, which we - # don't easily know here, so we ignore it in this comparison. + assert len(result.post_processing_cmds) == 2 - # The second arg is expected to be tmpdir/./pex. - tmpdir, pex_pex_name = os.path.split(os.path.normpath(ppc.argv[1])) - assert pex_pex_name == "pex" - assert re.match(r"\{digest_root\}/\.[0-9a-f]{32}\.tmp", tmpdir) + ppc0 = result.post_processing_cmds[0] + # The first arg is the full path to the python interpreter, which we + # don't easily know here, so we ignore it in this comparison. - # The third arg is expected to be tmpdir/{resolve}.pex. - req_pex_dir, req_pex_name = os.path.split(ppc.argv[2]) - assert req_pex_dir == tmpdir + # The second arg is expected to be tmpdir/./pex. + tmpdir, pex_pex_name = os.path.split(os.path.normpath(ppc0.argv[1])) + assert pex_pex_name == "pex" + assert re.match(r"\{digest_root\}/\.[0-9a-f]{32}\.tmp", tmpdir) - if index == 0: - assert req_pex_name == f"{resolve}.pex" - assert ppc.argv[3:] == ( - "venv", - "--pip", - "--collisions-ok", - "{digest_root}", - ) - elif index == 1: - assert req_pex_name == "editable_local_dists.pex" - assert ppc.argv[3:] == ( - "venv", - "--collisions-ok", - "{digest_root}", - ) + # The third arg is expected to be tmpdir/{resolve}.pex. + req_pex_dir, req_pex_name = os.path.split(ppc0.argv[2]) + assert req_pex_dir == tmpdir + assert req_pex_name == f"{resolve}.pex" - assert ppc.extra_env["PEX_MODULE"] == "pex.tools" - assert ppc.extra_env.get("PEX_ROOT") is not None + assert ppc0.argv[3:] == ( + "venv", + "--pip", + "--collisions-ok", + "{digest_root}", + ) + assert ppc0.extra_env["PEX_MODULE"] == "pex.tools" + assert ppc0.extra_env.get("PEX_ROOT") is not None - ppc_last = result.post_processing_cmds[-1] - assert ppc_last.argv == ("rm", "-rf", tmpdir) - assert ppc_last.extra_env == FrozenDict() + ppc1 = result.post_processing_cmds[1] + assert ppc1.argv == ("rm", "-rf", tmpdir) + assert ppc1.extra_env == FrozenDict() reldirs = [result.reldir for result in all_results] assert reldirs == [ diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index eb7362982f3..7ebac9e6769 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -501,7 +501,7 @@ class LocalDistsPEP660Pex: # based on LocalDistsPex sources digests, to prevent the same file ending up on sys.path twice. """ - pex: Pex + pex: Pex | None @rule(desc="Building editable local distributions (PEP 660)") @@ -513,6 +513,9 @@ async def build_editable_local_dists( # based on build_local_dists resolve = request.resolve if python_setup.enable_resolves else None resolve_dists = all_dists.targets.get(resolve, ()) + if not resolve_dists: + return LocalDistsPEP660Pex(None) + local_dists_wheels = await MultiGet( Get( LocalDistPEP660Wheels, From f2b8b58b22e6c560a3c3a6416b2b38e6d6617bdc Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 6 Apr 2023 14:19:18 -0500 Subject: [PATCH 14/32] refactor editable wheel passing and installation Installing the editable wheels via a pex was not effective. Using pex-tools to install two different pexes in the same venv caused conflicts with the __main__.py file. Using pex_path did not work because the editable wheel pex only had the editable wheels, not any of the dependencies. So, installing them in a pex was not effective. However, it does work to tell pex that the wheels are "sources". That way, pex just dumps them in the lib directory. Then, it was just a matter of adding the post-processing commands to move, install, and wrap up installation of the editable wheels. Even though we use pip to install the editable wheels, pants is both backend and frontend for PEP 660, because we generate the wheel (delegating dist-info generation to the external backend) and then install the wheel (delegating unpacking the wheel to pip). The final piece of complying with PEP-660 is to mark the install as an editable install via direct_url.json. We generate that when building the wheel, and then move it into place after pip has installed the editable wheels for us. One other method I considered was installing the editable wheels in the requirements pex. But I could not figure out how to combine EntireLockfile with additional wheel requirements. So, I punted and went merely passing the wheels as is via pex instead of attempting to install them in any pex. Finally, all of the export and local_dists_pep660 test pass! --- .../pants/backend/python/goals/export.py | 113 +++++++++++++----- .../python/goals/export_integration_test.py | 29 ++++- .../pants/backend/python/goals/export_test.py | 8 +- .../python/util_rules/local_dists_pep660.py | 67 ++++------- .../util_rules/local_dists_pep660_test.py | 34 ++---- 5 files changed, 147 insertions(+), 104 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 443f6249717..af62ec4a2d9 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -16,8 +16,8 @@ from pants.backend.python.target_types import PexLayout, PythonResolveField from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.local_dists_pep660 import ( - LocalDistsPEP660Pex, - LocalDistsPEP660PexRequest, + EditableLocalDists, + EditableLocalDistsRequest, ) from pants.backend.python.util_rules.local_dists_pep660 import rules as local_dists_pep660_rules from pants.backend.python.util_rules.pex import Pex, PexProcess, PexRequest, VenvPex, VenvPexProcess @@ -38,7 +38,7 @@ from pants.core.util_rules.distdir import DistDir from pants.engine.engine_aware import EngineAwareParameter from pants.engine.environment import EnvironmentName -from pants.engine.internals.native_engine import AddPrefix, Digest, MergeDigests +from pants.engine.internals.native_engine import AddPrefix, Digest, MergeDigests, Snapshot from pants.engine.internals.selectors import Get, MultiGet from pants.engine.process import ProcessCacheScope, ProcessResult from pants.engine.rules import collect_rules, rule @@ -155,6 +155,7 @@ class VenvExportRequest: dest_prefix: str resolve_name: str qualify_path_with_python_version: bool + editable_local_dists_digest: Digest | None = None @rule @@ -235,30 +236,81 @@ async def do_export( tmpdir_under_digest_root = os.path.join("{digest_root}", tmpdir_prefix) merged_digest_under_tmpdir = await Get(Digest, AddPrefix(merged_digest, tmpdir_prefix)) + post_processing_cmds = [ + 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), + "PEX_MODULE": "pex.tools", + }, + ), + # Remove the requirements and pex pexes, to avoid confusion. + PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), + ] + + if req.editable_local_dists_digest: + # 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]) + 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") + for f in wheels_snapshot.files + ] + post_processing_cmds[1:1] = [ + PostProcessingCommand( + [ + # 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], + tmpdir_under_digest_root, + ] + ), + PostProcessingCommand( + [ + # 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], + ] + ), + PostProcessingCommand( + [ + # replace pip's direct_url.json (which points to the wheel) + # with ours (which points to build_dir sources and is marked "editable"). + "sh", + "-c", + " ".join( + [ + f"mv -f {src} {dst};" + for src, dst in zip( + [ + os.path.join(d, "direct_url__pants__.json") + for d in dist_info_dirs + ], + [os.path.join(d, "direct_url.json") for d in dist_info_dirs], + ) + ] + ), + ] + ), + ] + return ExportResult( description, dest, digest=merged_digest_under_tmpdir, - post_processing_cmds=[ - 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), - "PEX_MODULE": "pex.tools", - }, - ), - # Remove the requirements and pex pexes, to avoid confusion. - PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), - ], + post_processing_cmds=post_processing_cmds, resolve=req.resolve_name or None, ) else: @@ -276,6 +328,7 @@ async def export_virtualenv_for_resolve( python_setup: PythonSetup, union_membership: UnionMembership, ) -> MaybeExportResult: + editable_local_dists_digest: Digest | None = None resolve = request.resolve lockfile_path = python_setup.resolves.get(resolve) if lockfile_path: @@ -292,27 +345,20 @@ async def export_virtualenv_for_resolve( ) ) - pex_path = [] - editable_local_dists = await Get( - LocalDistsPEP660Pex, - LocalDistsPEP660PexRequest( - interpreter_constraints=interpreter_constraints, - resolve=resolve, - ), + EditableLocalDists, EditableLocalDistsRequest(resolve=resolve) ) - if editable_local_dists.pex: - pex_path.append(editable_local_dists.pex) + editable_local_dists_digest = editable_local_dists.optional_digest pex_request = PexRequest( description=f"Build pex for resolve `{resolve}`", output_filename=f"{path_safe(resolve)}.pex", internal_only=True, requirements=EntireLockfile(lockfile), + sources=editable_local_dists_digest, interpreter_constraints=interpreter_constraints, # Packed layout should lead to the best performance in this use case. layout=PexLayout.PACKED, - pex_path=pex_path, ) else: # It's a tool resolve. @@ -355,6 +401,7 @@ async def export_virtualenv_for_resolve( dest_prefix, resolve, qualify_path_with_python_version=True, + editable_local_dists_digest=editable_local_dists_digest, ), ) return MaybeExportResult(export_result) diff --git a/src/python/pants/backend/python/goals/export_integration_test.py b/src/python/pants/backend/python/goals/export_integration_test.py index c91c35094b7..f5c7fdb3693 100644 --- a/src/python/pants/backend/python/goals/export_integration_test.py +++ b/src/python/pants/backend/python/goals/export_integration_test.py @@ -24,7 +24,16 @@ """ ), "src/python/foo.py": "from colors import *", - "src/python/BUILD": "python_source(name='foo', source='foo.py', resolve=parametrize('a', 'b'))", + "src/python/BUILD": dedent( + """\ + python_source(name='foo', source='foo.py', resolve=parametrize('a', 'b')) + python_distribution( + name='dist', + provides=python_artifact(name='foo', version='1.2.3'), + dependencies=[':foo@resolve=a'], + ) + """ + ), } @@ -128,6 +137,24 @@ 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 + 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" + for tool_config in EXPORTED_TOOLS: export_dir = os.path.join(export_prefix, tool_config.name, platform.python_version()) assert os.path.isdir(export_dir), f"expected export dir '{export_dir}' does not exist" diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index 331ae86c2bd..e293f782fe2 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -208,7 +208,11 @@ def test_export_venv_new_codepath( assert ppc1.argv[3] == "{digest_root}" assert ppc1.extra_env == FrozenDict() else: - assert len(result.post_processing_cmds) == 2 + if resolve == "flake8": + assert len(result.post_processing_cmds) == 2 + else: + # editable wheels are installed for a user resolve + assert len(result.post_processing_cmds) == 5 ppc0 = result.post_processing_cmds[0] # The first arg is the full path to the python interpreter, which we @@ -233,7 +237,7 @@ def test_export_venv_new_codepath( assert ppc0.extra_env["PEX_MODULE"] == "pex.tools" assert ppc0.extra_env.get("PEX_ROOT") is not None - ppc1 = result.post_processing_cmds[1] + ppc1 = result.post_processing_cmds[-1] assert ppc1.argv == ("rm", "-rf", tmpdir) assert ppc1.extra_env == FrozenDict() diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 7ebac9e6769..1392a6c5218 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -26,9 +26,7 @@ distutils_repr, ) from pants.backend.python.util_rules.dists import rules as dists_rules -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess -from pants.backend.python.util_rules.pex_requirements import PexRequirements +from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.base.build_root import BuildRoot from pants.core.util_rules import system_binaries from pants.core.util_rules.system_binaries import BashBinary, UnzipBinary @@ -463,58 +461,47 @@ async def sort_all_python_distributions_by_resolve( @dataclass(frozen=True) -class LocalDistsPEP660PexRequest: # based on LocalDistsPexRequest - """Request to build a PEX populated by PEP660 wheels of local dists. - - Like LocalDistsPexRequest, the local dists come from the dependency closure of a set of - addresses. Unlike LocalDistsPexRequest, the editable wheel files must not be exported or made - available to the end-user (according to PEP 660). Instead, the PEP660Pex serves as an - intermediate, internal-only, PEX that can be used to install these wheels in a virtualenv. It - follows then, that this PEX should probably not be exported for use by end-users. +class EditableLocalDistsRequest: + """Request toild generate PEP660 wheels of local dists in the given resolve. + + The editable wheel files must not be exported or made available to the end-user (according to + PEP 660). Instead, the PEP660 editable wheels serve as intermediate, internal-only, + representation of what should be installed in the exported virtualenv to create the editable + installs of local python_distributions. """ - interpreter_constraints: InterpreterConstraints resolve: str | None # None if resolves is not enabled @dataclass(frozen=True) -class LocalDistsPEP660Pex: # based on LocalDistsPex - """A PEX file populated by PEP660 wheels of local dists. +class EditableLocalDists: + """A Digest populated by editable (PEP660) wheels of local dists. - Can be consumed from another PEX, e.g., by adding to PEX_PATH. + According to PEP660, these wheels should not be exported to users and must be discarded + after install. Anything that uses this should ensure that these wheels get installed and + then deleted. - The PEX will contain installs of PEP660 wheels generate4d from local dists. Installing PEP660 - wheels creates an "editable" install such that the sys.path gets adjusted to include source - directories that are NOT part of the PEX. As such, this PEX is decidedly not hermetic or - portable and should only be used to facilitate building virtualenvs for development. + Installing PEP660 wheels creates an "editable" install such that the sys.path gets + adjusted to include source directories from the build root (not from the sandbox). + This is decidedly not hermetic or portable and should only be used locally. PEP660 wheels have .dist-info metadata and the .pth files (or similar) that adjust sys.path. - - The PEX will only contain metadata for local dists and not any dependencies. For Pants generated - `setup.py` / `pyproject.toml`, the dependencies will be included in the standard resolve process - that the locally-built dists PEX is adjoined to via PEX_PATH. For hand-made `setup.py` / - `pyproject.toml` with 3rdparty dependencies not hand-mirrored into BUILD file dependencies, this - will lead to issues. See https://github.com/pantsbuild/pants/issues/13587#issuecomment-974863636 - for one way to fix this corner which is intentionally punted on for now. - - Lists the files provided by the dists on sys.path, so they can be subtracted from - sources digests, to prevent the same file ending up on sys.path twice. """ - pex: Pex | None + optional_digest: Digest | None @rule(desc="Building editable local distributions (PEP 660)") async def build_editable_local_dists( # based on build_local_dists - request: LocalDistsPEP660PexRequest, + request: EditableLocalDistsRequest, all_dists: ResolveSortedPythonDistributionTargets, python_setup: PythonSetup, -) -> LocalDistsPEP660Pex: +) -> EditableLocalDists: resolve = request.resolve if python_setup.enable_resolves else None resolve_dists = all_dists.targets.get(resolve, ()) if not resolve_dists: - return LocalDistsPEP660Pex(None) + return EditableLocalDists(None) local_dists_wheels = await MultiGet( Get( @@ -533,19 +520,7 @@ async def build_editable_local_dists( # based on build_local_dists wheels_digest = await Get(Digest, MergeDigests(wheels_digests)) - editable_dists_pex = await Get( - Pex, - PexRequest( - output_filename="editable_local_dists.pex", - requirements=PexRequirements(wheels), - interpreter_constraints=request.interpreter_constraints, - additional_inputs=wheels_digest, - internal_only=True, - additional_args=["--intransitive"], - ), - ) - - return LocalDistsPEP660Pex(editable_dists_pex) + return EditableLocalDists(wheels_digest) def rules(): diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 282bf8135b0..1cc8751e514 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -20,13 +20,12 @@ from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.local_dists_pep660 import ( AllPythonDistributionTargets, - LocalDistsPEP660Pex, - LocalDistsPEP660PexRequest, + EditableLocalDists, + EditableLocalDistsRequest, PEP660BuildResult, ) from pants.backend.python.util_rules.pex_from_targets import InterpreterConstraintsRequest from pants.backend.python.util_rules.pex_requirements import PexRequirements -from pants.build_graph.address import Address from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent from pants.testutil.python_interpreter_selection import ( skip_unless_python27_present, @@ -47,7 +46,7 @@ def rule_runner() -> RuleRunner: *pex_from_targets.rules(), QueryRule(InterpreterConstraints, (InterpreterConstraintsRequest,)), QueryRule(AllPythonDistributionTargets, ()), - QueryRule(LocalDistsPEP660Pex, (LocalDistsPEP660PexRequest,)), + QueryRule(EditableLocalDists, (EditableLocalDistsRequest,)), QueryRule(PEP660BuildResult, (DistBuildRequest,)), ], target_types=[PythonSourcesGeneratorTarget, PythonDistribution], @@ -136,36 +135,27 @@ def test_build_editable_local_dists(rule_runner: RuleRunner) -> None: ), } ) - # all_dists = rule_runner.request(AllPythonDistributionTargets, ()) - # assert not all_dists - addresses = [Address("foo", target_name="dist")] - interpreter_constraints = rule_runner.request( - InterpreterConstraints, [InterpreterConstraintsRequest(addresses)] - ) - request = LocalDistsPEP660PexRequest( - interpreter_constraints=interpreter_constraints, + request = EditableLocalDistsRequest( resolve=None, # resolves is disabled ) - result = rule_runner.request(LocalDistsPEP660Pex, [request]) - - assert result.pex is not None - contents = rule_runner.request(DigestContents, [result.pex.digest]) - whl_content = None - for content in contents: - if content.path == "editable_local_dists.pex/.deps/foo-9.8.7-0.editable-py3-none-any.whl": - whl_content = content + result = rule_runner.request(EditableLocalDists, [request]) + + assert result.optional_digest is not None + contents = rule_runner.request(DigestContents, [result.optional_digest]) + assert len(contents) == 1 + whl_content = contents[0] assert whl_content + assert whl_content.path == "foo-9.8.7-0.editable-py3-none-any.whl" with io.BytesIO(whl_content.content) as fp: with zipfile.ZipFile(fp, "r") as whl: whl_files = whl.namelist() # Check that sources are not present in editable wheel assert "foo/bar.py" not in whl_files assert "foo/qux.py" not in whl_files - # Once installed, wheel does not have RECORD - assert "foo-9.8.7.dist-info/RECORD" not in whl_files # Check that pth and metadata files are present in editable wheel assert "foo__pants__.pth" in whl_files assert "foo-9.8.7.dist-info/METADATA" in whl_files + assert "foo-9.8.7.dist-info/RECORD" in whl_files assert "foo-9.8.7.dist-info/WHEEL" in whl_files assert "foo-9.8.7.dist-info/direct_url__pants__.json" in whl_files assert "foo-9.8.7.dist-info/entry_points.txt" in whl_files From 3557fac8b66a9c27927103a83af41080b174a294 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 6 Apr 2023 21:30:25 -0500 Subject: [PATCH 15/32] add test_sort_all_python_distributions_by_resolve --- .../util_rules/local_dists_pep660_test.py | 64 +++++++++++++++++-- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 1cc8751e514..822cd6b8aba 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -11,7 +11,6 @@ import pytest from pants.backend.python import target_types_rules -from pants.backend.python.goals.setup_py import rules as setup_py_rules from pants.backend.python.macros.python_artifact import PythonArtifact from pants.backend.python.subsystems.setuptools import rules as setuptools_rules from pants.backend.python.target_types import PythonDistribution, PythonSourcesGeneratorTarget @@ -19,14 +18,16 @@ from pants.backend.python.util_rules.dists import BuildSystem, DistBuildRequest from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.local_dists_pep660 import ( - AllPythonDistributionTargets, EditableLocalDists, EditableLocalDistsRequest, PEP660BuildResult, + ResolveSortedPythonDistributionTargets, ) from pants.backend.python.util_rules.pex_from_targets import InterpreterConstraintsRequest from pants.backend.python.util_rules.pex_requirements import PexRequirements +from pants.build_graph.address import Address from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent +from pants.engine.internals.parametrize import Parametrize from pants.testutil.python_interpreter_selection import ( skip_unless_python27_present, skip_unless_python39_present, @@ -40,17 +41,16 @@ def rule_runner() -> RuleRunner: ret = RuleRunner( rules=[ *local_dists_pep660.rules(), - *setup_py_rules(), *setuptools_rules(), *target_types_rules.rules(), *pex_from_targets.rules(), QueryRule(InterpreterConstraints, (InterpreterConstraintsRequest,)), - QueryRule(AllPythonDistributionTargets, ()), + QueryRule(ResolveSortedPythonDistributionTargets, ()), QueryRule(EditableLocalDists, (EditableLocalDistsRequest,)), QueryRule(PEP660BuildResult, (DistBuildRequest,)), ], target_types=[PythonSourcesGeneratorTarget, PythonDistribution], - objects={"python_artifact": PythonArtifact}, + objects={"python_artifact": PythonArtifact, "parametrize": Parametrize}, ) ret.set_options( [], @@ -100,6 +100,60 @@ def test_works_with_python39(rule_runner: RuleRunner) -> None: do_test_backend_wrapper(rule_runner, constraints="CPython==3.9.*") +def test_sort_all_python_distributions_by_resolve(rule_runner: RuleRunner) -> None: + rule_runner.set_options( + [ + "--python-enable-resolves=True", + "--python-resolves={'a': 'lock.txt', 'b': 'lock.txt'}", + # 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']", + ], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) + rule_runner.write_files( + { + "foo/BUILD": dedent( + """ + python_sources(resolve=parametrize("a", "b")) + + python_distribution( + name="dist", + dependencies=[":foo@resolve=a"], + provides=python_artifact(name="foo", version="9.8.7"), + sdist=False, + generate_setup=False, + ) + """ + ), + "foo/bar.py": "BAR = 42", + "foo/setup.py": dedent( + """ + from setuptools import setup + + setup( + name="foo", + version="9.8.7", + packages=["foo"], + package_dir={"foo": "."}, + entry_points={"foo.plugins": ["bar = foo.bar.BAR"]}, + ) + """ + ), + "lock.txt": "", + } + ) + dist = rule_runner.get_target(Address("foo", target_name="dist")) + result = rule_runner.request(ResolveSortedPythonDistributionTargets, ()) + assert len(result.targets) == 1 + assert "b" not in result.targets + assert "a" in result.targets + assert len(result.targets["a"]) == 1 + assert dist == result.targets["a"][0] + + def test_build_editable_local_dists(rule_runner: RuleRunner) -> None: foo = PurePath("foo") rule_runner.write_files( From 6357b85bc871524fa45f31e2eafd4d057796097d Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 6 Apr 2023 21:32:54 -0500 Subject: [PATCH 16/32] add [export].py-editables-in-resolves option This way people have to opt into including the editables (and therefore incurring the cost of finding/building/installing the PEP-660 wheels). --- .../pants/backend/python/goals/export.py | 50 +++++++++++++++---- .../python/goals/export_integration_test.py | 44 +++++++++------- .../pants/backend/python/goals/export_test.py | 32 +++++++++--- 3 files changed, 92 insertions(+), 34 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index af62ec4a2d9..7183320bce8 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -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 @@ -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 #). @@ -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), @@ -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") @@ -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, ] ), @@ -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( @@ -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 @@ -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}`", diff --git a/src/python/pants/backend/python/goals/export_integration_test.py b/src/python/pants/backend/python/goals/export_integration_test.py index f5c7fdb3693..15185b88fbf 100644 --- a/src/python/pants/backend/python/goals/export_integration_test.py +++ b/src/python/pants/backend/python/goals/export_integration_test.py @@ -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() @@ -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()) diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index e293f782fe2..bdc7540159e 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -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 @@ -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}, ) @@ -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') """ @@ -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"}, @@ -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 From b65a07986b3873d166c8634de8813b47a1714390 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 6 Apr 2023 21:51:43 -0500 Subject: [PATCH 17/32] typos --- .../pants/backend/python/util_rules/local_dists_pep660.py | 2 +- .../pants/backend/python/util_rules/local_dists_pep660_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 1392a6c5218..850ab0e6d55 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -462,7 +462,7 @@ async def sort_all_python_distributions_by_resolve( @dataclass(frozen=True) class EditableLocalDistsRequest: - """Request toild generate PEP660 wheels of local dists in the given resolve. + """Request to generate PEP660 wheels of local dists in the given resolve. The editable wheel files must not be exported or made available to the end-user (according to PEP 660). Instead, the PEP660 editable wheels serve as intermediate, internal-only, diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 822cd6b8aba..cea7037f3cd 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -1,4 +1,4 @@ -# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). from __future__ import annotations From 0aa91728bf296fc19ad940b39fb8ee0fe57e240d Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 6 Apr 2023 22:08:18 -0500 Subject: [PATCH 18/32] fix option access --- src/python/pants/backend/python/goals/export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 7183320bce8..b18d022dac2 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -370,7 +370,7 @@ async def export_virtualenv_for_resolve( ) ) - if resolve in export_subsys.py_editables_in_resolve: + if resolve in export_subsys.options.py_editables_in_resolves: editable_local_dists = await Get( EditableLocalDists, EditableLocalDistsRequest(resolve=resolve) ) From d5b35621e322bf78353ab1056470e5f5ef66a6f9 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 6 Apr 2023 23:33:01 -0500 Subject: [PATCH 19/32] drop unnecessary option in test --- .../pants/backend/python/util_rules/local_dists_pep660_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index cea7037f3cd..29667ece0b1 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -109,7 +109,6 @@ def test_sort_all_python_distributions_by_resolve(rule_runner: RuleRunner) -> No "--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']", ], env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) From 2001e4f116470d9014a355aa856048a65cdcbe94 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 7 Apr 2023 20:30:01 -0500 Subject: [PATCH 20/32] PEP660: Standardize wheel contents for older PEP517 backends --- .../python/util_rules/local_dists_pep660.py | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 850ab0e6d55..67a453ae359 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -75,6 +75,7 @@ class PEP660BuildResult: # based on DistBuildResult import base64 import hashlib import os +import shutil import zipfile import {build_backend_module} @@ -113,6 +114,24 @@ class PEP660BuildResult: # based on DistBuildResult whl.extractall(build_dir, dist_info_files) metadata_path = os.path.dirname(dist_info_files[0]) +pkg_version = metadata_path.replace(".dist-info", "") +if "-" in pkg_version: + pkg, version = pkg_version.split("-") +else: + # an old backend that doesn't conform to the latest specs + pkg = pkg_version + version = "" + with open(os.path.join(build_dir, metadata_path, "METADATA"), "r") as f: + lines = f.readlines() + for line in lines: + if line.startswith("Version: "): + version = line[len("Version: "):].strip() + break + # standardize the name of the dist-info directory + old_metadata_path = metadata_path + metadata_path = pkg + "-" + version + ".dist-info" + shutil.move(os.path.join(build_dir, old_metadata_path), os.path.join(build_dir, metadata_path)) + # Any RECORD* file will be incorrect since we are creating the wheel. for file in os.listdir(os.path.join(build_dir, metadata_path)): if file == "RECORD" or file.startswith("RECORD."): @@ -144,18 +163,6 @@ class PEP660BuildResult: # based on DistBuildResult }}}} '''.format(direct_url)) -pkg_version = metadata_path.replace(".dist-info", "") -if "-" in pkg_version: - pkg, version = pkg_version.split("-") -else: - pkg = pkg_version - version = "" - with open(os.path.join(build_dir, metadata_path, "METADATA"), "r") as f: - lines = f.readlines() - for line in lines: - if line.startswith("Version: "): - version = line[len("Version: "):].strip() - break pth_file_arcname = pkg + "__pants__.pth" _record = [] From b660c571673baefa65094ad05ea265f1d7e3f024 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 7 Apr 2023 20:34:22 -0500 Subject: [PATCH 21/32] clean up editable wheel install to not re-install deps --- src/python/pants/backend/python/goals/export.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index b18d022dac2..6933ab1856c 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -305,6 +305,8 @@ async def do_export( # now install the editable wheels os.path.join(output_path, "bin", "pip"), "install", + "--no-deps", # deps already installed via requirements.pex + "--no-build-isolation", # avoid vcs dep downloads (as they are installed) *(os.path.join(tmpdir_under_digest_root, f) for f in wheels_snapshot.files), ] ), From 9c1df00301e36bb4c40ab7a4afbddb91ad35c195 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 12:17:00 -0500 Subject: [PATCH 22/32] adjust for moved rules --- .../python/util_rules/local_dists_pep660.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 67a453ae359..ae87aa56ffd 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -9,23 +9,22 @@ from collections import defaultdict from dataclasses import dataclass -# TODO: move this to a util_rules file -from pants.backend.python.goals import setup_py -from pants.backend.python.goals.setup_py import ( - DependencyOwner, - ExportedTarget, - OwnedDependencies, - create_dist_build_request, -) from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet from pants.backend.python.target_types import PythonProvidesField, PythonResolveField +from pants.backend.python.util_rules import package_dists from pants.backend.python.util_rules.dists import ( BuildBackendError, DistBuildRequest, distutils_repr, ) from pants.backend.python.util_rules.dists import rules as dists_rules +from pants.backend.python.util_rules.package_dists import ( + DependencyOwner, + ExportedTarget, + OwnedDependencies, + create_dist_build_request, +) from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.base.build_root import BuildRoot from pants.core.util_rules import system_binaries @@ -534,6 +533,6 @@ def rules(): return ( *collect_rules(), *dists_rules(), - *setup_py.rules(), + *package_dists.rules(), *system_binaries.rules(), ) From 017dc7b6775311a53f9ebd00b326f01099348c5b Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 14:18:08 -0500 Subject: [PATCH 23/32] address review feedback --- .../pants/backend/python/goals/export.py | 23 +++++----- .../python/goals/export_integration_test.py | 12 ++--- .../pants/backend/python/goals/export_test.py | 3 +- src/python/pants/backend/python/register.py | 2 + .../python/util_rules/local_dists_pep660.py | 44 ++++++++++++------- 5 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 6933ab1856c..0d515d71fd0 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -19,7 +19,6 @@ EditableLocalDists, EditableLocalDistsRequest, ) -from pants.backend.python.util_rules.local_dists_pep660 import rules as local_dists_pep660_rules from pants.backend.python.util_rules.pex import Pex, PexProcess, PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.pex_cli import PexPEX from pants.backend.python.util_rules.pex_environment import PexEnvironment @@ -281,19 +280,24 @@ async def do_export( PostProcessingCommand(["rm", "-rf", tmpdir_under_digest_root]), ] + # Insert editable wheel post processing commands if needed. if req.editable_local_dists_digest is not None: - # insert editable wheel post processing commands + # We need the snapshot to get the wheel file names which are something like: + # - pkg_name-1.2.3-0.editable-py3-none-any.whl wheels_snapshot = await Get(Snapshot, Digest, req.editable_local_dists_digest) - py_minor_version = ".".join(py_version.split(".", 2)[:2]) - lib_dir = os.path.join(output_path, "lib", f"python{py_minor_version}", "site-packages") + # We need the paths to the installed .dist-info directories to finish installation. + py_major_minor_version = ".".join(py_version.split(".", 2)[:2]) + lib_dir = os.path.join(output_path, "lib", f"python{py_major_minor_version}", "site-packages") dist_info_dirs = [ + # This builds: dist/.../resolve/3.8.9/lib/python3.8/site-packages/pkg_name-1.2.3.dist-info os.path.join(lib_dir, "-".join(f.split("-")[:2]) + ".dist-info") for f in wheels_snapshot.files ] + # We use slice assignment to insert multiple elements at index 1. post_processing_cmds[1:1] = [ PostProcessingCommand( [ - # the wheels are "sources" in the pex and get dumped in lib_dir + # 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), @@ -302,17 +306,17 @@ async def do_export( ), PostProcessingCommand( [ - # now install the editable wheels + # Now install the editable wheels. os.path.join(output_path, "bin", "pip"), "install", - "--no-deps", # deps already installed via requirements.pex - "--no-build-isolation", # avoid vcs dep downloads (as they are installed) + "--no-deps", # The deps were already installed via requirements.pex. + "--no-build-isolation", # Avoid VCS dep downloads (as they are installed). *(os.path.join(tmpdir_under_digest_root, f) for f in wheels_snapshot.files), ] ), PostProcessingCommand( [ - # replace pip's direct_url.json (which points to the wheel) + # Replace pip's direct_url.json (which points to the temp editable wheel) # with ours (which points to build_dir sources and is marked "editable"). "sh", "-c", @@ -583,7 +587,6 @@ async def export_virtualenvs( def rules(): return [ *collect_rules(), - *local_dists_pep660_rules(), Export.subsystem_cls.register_plugin_options(ExportPluginOptions), UnionRule(ExportRequest, ExportVenvsRequest), ] diff --git a/src/python/pants/backend/python/goals/export_integration_test.py b/src/python/pants/backend/python/goals/export_integration_test.py index 15185b88fbf..75b47a74dcd 100644 --- a/src/python/pants/backend/python/goals/export_integration_test.py +++ b/src/python/pants/backend/python/goals/export_integration_test.py @@ -29,7 +29,7 @@ python_source(name='foo', source='foo.py', resolve=parametrize('a', 'b')) python_distribution( name='dist', - provides=python_artifact(name='foo', version='1.2.3'), + provides=python_artifact(name='foo-dist', version='1.2.3'), dependencies=[':foo@resolve=a'], ) """ @@ -143,27 +143,27 @@ def test_export(py_resolve_format: PythonResolveExportFormat) -> None: ), f"expected dist-info for ansicolors '{expected_ansicolors_dir}' does not exist" if py_resolve_format == PythonResolveExportFormat.mutable_virtualenv: - expected_foo_dir = os.path.join(lib_dir, "foo-1.2.3.dist-info") + expected_foo_dir = os.path.join(lib_dir, "foo_dist-1.2.3.dist-info") if resolve == "b": assert not os.path.isdir( expected_foo_dir - ), f"unexpected dist-info for foo '{expected_foo_dir}' exists" + ), f"unexpected dist-info for foo-dist '{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" + ), f"expected dist-info for foo-dist '{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" + ), f"expected direct_url__pants__.json for foo-dist '{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" + ), f"expected direct_url.json for foo-dist '{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()) diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index bdc7540159e..5202438a96f 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -17,7 +17,7 @@ PythonRequirementTarget, PythonSourcesGeneratorTarget, ) -from pants.backend.python.util_rules import pex_from_targets +from pants.backend.python.util_rules import pex_from_targets, local_dists_pep660 from pants.base.specs import RawSpecs, RecursiveGlobSpec from pants.core.goals.export import ExportResults from pants.core.util_rules import distdir @@ -36,6 +36,7 @@ def rule_runner() -> RuleRunner: *pex_from_targets.rules(), *target_types_rules.rules(), *distdir.rules(), + *local_dists_pep660.rules(), *flake8_subsystem.rules(), QueryRule(Targets, [RawSpecs]), QueryRule(ExportResults, [ExportVenvsRequest]), diff --git a/src/python/pants/backend/python/register.py b/src/python/pants/backend/python/register.py index 51dc645df53..38ee925abec 100644 --- a/src/python/pants/backend/python/register.py +++ b/src/python/pants/backend/python/register.py @@ -46,6 +46,7 @@ from pants.backend.python.util_rules import ( ancestor_files, local_dists, + local_dists_pep660, pex, pex_from_targets, python_sources, @@ -70,6 +71,7 @@ def rules(): # Util rules *ancestor_files.rules(), *dependency_inference_rules.rules(), + *local_dists_pep660.rules(), *pex.rules(), *pex_from_targets.rules(), *python_sources.rules(), diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index ae87aa56ffd..727d6023c83 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -52,11 +52,8 @@ logger = logging.getLogger(__name__) -# We just use DistBuildRequest directly instead of adding a PEP660BuildRequest - - @dataclass(frozen=True) -class PEP660BuildResult: # based on DistBuildResult +class PEP660BuildResult: output: Digest # Relpaths in the output digest. editable_wheel_path: str | None @@ -67,6 +64,7 @@ class PEP660BuildResult: # based on DistBuildResult # PEP 517, however, says that method is optional. So finally we fall back # to using `build_wheel` and then extract the dist-info directory and then # delete the extra wheel file. Most people shouldn't hit that path (I hope). +# See also: the docstring on the 'interpolate_backend_wrapper' function. # NOTE: PEP 660 does not address the `.data` directory, so we ignore it. _BACKEND_WRAPPER_BOILERPLATE = """ # DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS @@ -106,9 +104,7 @@ class PEP660BuildResult: # based on DistBuildResult else: # Optional PEP 517 method not defined. Use build_wheel instead. wheel_path = backend.build_wheel(build_dir, wheel_config_settings) - full_wheel_path = os.path.join(build_dir, wheel_path) - - with zipfile.ZipFile(full_wheel_path, "r") as whl: + with zipfile.ZipFile(os.path.join(build_dir, wheel_path), "r") as whl: dist_info_files = [n for n in whl.namelist() if ".dist-info/" in n] whl.extractall(build_dir, dist_info_files) metadata_path = os.path.dirname(dist_info_files[0]) @@ -117,7 +113,7 @@ class PEP660BuildResult: # based on DistBuildResult if "-" in pkg_version: pkg, version = pkg_version.split("-") else: - # an old backend that doesn't conform to the latest specs + # The wrapped backend does not conform to the latest specs. pkg = pkg_version version = "" with open(os.path.join(build_dir, metadata_path, "METADATA"), "r") as f: @@ -126,7 +122,7 @@ class PEP660BuildResult: # based on DistBuildResult if line.startswith("Version: "): version = line[len("Version: "):].strip() break - # standardize the name of the dist-info directory + # Standardize the name of the dist-info directory per Binary distribution format spec. old_metadata_path = metadata_path metadata_path = pkg + "-" + version + ".dist-info" shutil.move(os.path.join(build_dir, old_metadata_path), os.path.join(build_dir, metadata_path)) @@ -193,13 +189,27 @@ def record(file_path, file_arcname): """ -def interpolate_backend_wrapper( # based on interpolate_backend_shim +def interpolate_backend_wrapper( dist_dir: str, pth_file_path: str, direct_url: str, request: DistBuildRequest, ) -> bytes: - # See https://www.python.org/dev/peps/pep-0517/#source-trees. + """ + This builds a PEP 517 / PEP 660 wrapper script that builds the editable wheel. + + This backend wrapper script, along with the commands that install the editable wheel, + need to conform to the following specs so that Pants is a PEP 660 compliant frontend, + a PEP 660 compliant backend, and that it builds a compliant wheel and install. + + Relevant Specs: + https://peps.python.org/pep-0517/ + https://peps.python.org/pep-0660/ + https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/ + https://packaging.python.org/en/latest/specifications/recording-installed-packages/ + https://packaging.python.org/en/latest/specifications/direct-url-data-structure/ + https://packaging.python.org/en/latest/specifications/binary-distribution-format/ + """ module_path, _, object_path = request.build_system.build_backend.partition(":") backend_object = f"{module_path}.{object_path}" if object_path else module_path @@ -211,7 +221,7 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: # tag the editable wheel as widely compatible lang_tag, abi_tag, platform_tag = "py3", "none", "any" if request.interpreter_constraints.includes_python2(): - # assume everything has py3 support. + # Assume everything has py3 support. If not, we'll need a new includes_python3 method. lang_tag = "py2.py3" return _BACKEND_WRAPPER_BOILERPLATE.format( @@ -226,10 +236,10 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: @rule -async def run_pep660_build( # based on run_pep517_build +async def run_pep660_build( request: DistBuildRequest, python_setup: PythonSetup, build_root: BuildRoot ) -> PEP660BuildResult: - # Create the .pth files to add the relevant source root to PYTHONPATH + # Create the .pth files to add the relevant source root to sys.path. # We cannot use the build backend to do this because we do not want to tell # it where the workspace is and risk it adding anything there. # NOTE: We use .pth files to support ICs less than python3.7. @@ -341,7 +351,7 @@ async def run_pep660_build( # based on run_pep517_build @dataclass(frozen=True) -class LocalDistPEP660Wheels: # based on LocalDistWheels +class LocalDistPEP660Wheels: """Contains the PEP 660 "editable" wheels isolated from a single local Python distribution.""" pep660_wheel_paths: tuple[str, ...] @@ -350,7 +360,7 @@ class LocalDistPEP660Wheels: # based on LocalDistWheels @rule -async def isolate_local_dist_pep660_wheels( # based on isolate_local_dist_wheels +async def isolate_local_dist_pep660_wheels( dist_field_set: PythonDistributionFieldSet, bash: BashBinary, unzip_binary: UnzipBinary, @@ -498,7 +508,7 @@ class EditableLocalDists: @rule(desc="Building editable local distributions (PEP 660)") -async def build_editable_local_dists( # based on build_local_dists +async def build_editable_local_dists( request: EditableLocalDistsRequest, all_dists: ResolveSortedPythonDistributionTargets, python_setup: PythonSetup, From d230ea577e21bf76d46acce0e5c07748498914a1 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 14:29:57 -0500 Subject: [PATCH 24/32] reformat --- src/python/pants/backend/python/goals/export.py | 4 +++- src/python/pants/backend/python/goals/export_test.py | 2 +- .../pants/backend/python/util_rules/local_dists_pep660.py | 3 +-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 0d515d71fd0..b733aa812e4 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -287,7 +287,9 @@ async def do_export( wheels_snapshot = await Get(Snapshot, Digest, req.editable_local_dists_digest) # We need the paths to the installed .dist-info directories to finish installation. py_major_minor_version = ".".join(py_version.split(".", 2)[:2]) - lib_dir = os.path.join(output_path, "lib", f"python{py_major_minor_version}", "site-packages") + lib_dir = os.path.join( + output_path, "lib", f"python{py_major_minor_version}", "site-packages" + ) dist_info_dirs = [ # This builds: dist/.../resolve/3.8.9/lib/python3.8/site-packages/pkg_name-1.2.3.dist-info os.path.join(lib_dir, "-".join(f.split("-")[:2]) + ".dist-info") diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index 5202438a96f..faf931f87e3 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -17,7 +17,7 @@ PythonRequirementTarget, PythonSourcesGeneratorTarget, ) -from pants.backend.python.util_rules import pex_from_targets, local_dists_pep660 +from pants.backend.python.util_rules import local_dists_pep660, pex_from_targets from pants.base.specs import RawSpecs, RecursiveGlobSpec from pants.core.goals.export import ExportResults from pants.core.util_rules import distdir diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 727d6023c83..091bfd258e7 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -195,8 +195,7 @@ def interpolate_backend_wrapper( direct_url: str, request: DistBuildRequest, ) -> bytes: - """ - This builds a PEP 517 / PEP 660 wrapper script that builds the editable wheel. + """This builds a PEP 517 / PEP 660 wrapper script that builds the editable wheel. This backend wrapper script, along with the commands that install the editable wheel, need to conform to the following specs so that Pants is a PEP 660 compliant frontend, From 0d95146699175bc2491b61572542fb54470bff3a Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 16:56:45 -0500 Subject: [PATCH 25/32] refactor PEP 660 wrapper into a resource script --- .../python/util_rules/local_dists_pep660.py | 262 +++++------------- .../backend/python/util_rules/scripts/BUILD | 1 + .../python/util_rules/scripts/__init__.py | 0 .../scripts/pep660_backend_wrapper.py | 172 ++++++++++++ 4 files changed, 248 insertions(+), 187 deletions(-) create mode 100644 src/python/pants/backend/python/util_rules/scripts/BUILD create mode 100644 src/python/pants/backend/python/util_rules/scripts/__init__.py create mode 100644 src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 091bfd258e7..59220f1daa1 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -3,12 +3,14 @@ from __future__ import annotations +import json import logging import os import shlex from collections import defaultdict from dataclasses import dataclass +from pants.backend.python.dependency_inference import get_scripts_digest from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet from pants.backend.python.target_types import PythonProvidesField, PythonResolveField @@ -52,6 +54,9 @@ logger = logging.getLogger(__name__) +_scripts_package = "pants.backend.python.util_rules.scripts" + + @dataclass(frozen=True) class PEP660BuildResult: output: Digest @@ -59,163 +64,18 @@ class PEP660BuildResult: editable_wheel_path: str | None -# PEP 660 defines `prepare_metadata_for_build_editable`. If PEP 660 is not -# supported, we fall back to PEP 517's `prepare_metadata_for_build_wheel`. -# PEP 517, however, says that method is optional. So finally we fall back -# to using `build_wheel` and then extract the dist-info directory and then -# delete the extra wheel file. Most people shouldn't hit that path (I hope). -# See also: the docstring on the 'interpolate_backend_wrapper' function. -# NOTE: PEP 660 does not address the `.data` directory, so we ignore it. -_BACKEND_WRAPPER_BOILERPLATE = """ -# DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS - -import base64 -import hashlib -import os -import shutil -import zipfile -import {build_backend_module} - -backend = {build_backend_object} - -dist_dir = "{dist_dir}" -build_dir = "build" -pth_file_path = "{pth_file_path}" -wheel_config_settings = {wheel_config_settings_str} -tags = "{tags}" -direct_url = "{direct_url}" - -# Python 2.7 doesn't have the exist_ok arg on os.makedirs(). -for d in (dist_dir, build_dir): - try: - os.makedirs(d) - except OSError as e: - if e.errno != errno.EEXIST: - raise - -prepare_metadata = getattr( - backend, - "prepare_metadata_for_build_editable", # PEP 660 - getattr(backend, "prepare_metadata_for_build_wheel", None), # PEP 517 -) -if prepare_metadata is not None: - print("prepare_metadata: " + str(prepare_metadata)) - metadata_path = prepare_metadata(build_dir, wheel_config_settings) -else: - # Optional PEP 517 method not defined. Use build_wheel instead. - wheel_path = backend.build_wheel(build_dir, wheel_config_settings) - with zipfile.ZipFile(os.path.join(build_dir, wheel_path), "r") as whl: - dist_info_files = [n for n in whl.namelist() if ".dist-info/" in n] - whl.extractall(build_dir, dist_info_files) - metadata_path = os.path.dirname(dist_info_files[0]) - -pkg_version = metadata_path.replace(".dist-info", "") -if "-" in pkg_version: - pkg, version = pkg_version.split("-") -else: - # The wrapped backend does not conform to the latest specs. - pkg = pkg_version - version = "" - with open(os.path.join(build_dir, metadata_path, "METADATA"), "r") as f: - lines = f.readlines() - for line in lines: - if line.startswith("Version: "): - version = line[len("Version: "):].strip() - break - # Standardize the name of the dist-info directory per Binary distribution format spec. - old_metadata_path = metadata_path - metadata_path = pkg + "-" + version + ".dist-info" - shutil.move(os.path.join(build_dir, old_metadata_path), os.path.join(build_dir, metadata_path)) - -# Any RECORD* file will be incorrect since we are creating the wheel. -for file in os.listdir(os.path.join(build_dir, metadata_path)): - if file == "RECORD" or file.startswith("RECORD."): - os.unlink(os.path.join(build_dir, metadata_path, file)) -metadata_wheel_file = os.path.join(build_dir, metadata_path, "WHEEL") -if not os.path.exists(metadata_wheel_file): - with open(metadata_wheel_file, "w") as f: - f.write('''\ -Wheel-Version: 1.0 -Generator: pantsbuild -Root-Is-Purelib: true -Tag: {{}} -Build: 0.editable -'''.format(tags)) - -if direct_url: - # We abuse pex to get PEP660 editable wheels installed in a virtualenv. - # Pex and pip do not know that this is an editable install. - # We can't add direct_url.json to the wheel, because that has to be added - # by the wheel installer. So, we will rename this file once installed. - direct_url_file = os.path.join(build_dir, metadata_path, "direct_url__pants__.json") - with open(direct_url_file , "w") as f: - f.write('''\ -{{{{ - "url": "{{}}", - "dir_info": {{{{ - "editable": true - }}}} -}}}} -'''.format(direct_url)) - -pth_file_arcname = pkg + "__pants__.pth" - -_record = [] -def record(file_path, file_arcname): - with open(file_path, "rb") as f: - file_digest = hashlib.sha256(f.read()).digest() - file_hash = "sha256=" + base64.urlsafe_b64encode(file_digest).decode().rstrip("=") - file_size = str(os.stat(file_path).st_size) - _record.append(",".join([file_arcname, file_hash, file_size])) - -wheel_path = "{{}}-{{}}-0.editable-{{}}.whl".format(pkg, version, tags) -with zipfile.ZipFile(os.path.join(dist_dir, wheel_path), "w") as whl: - record(pth_file_path, pth_file_arcname) - whl.write(pth_file_path, pth_file_arcname) - # based on wheel.wheelfile.WheelFile.write_files (by @argonholm MIT license) - for root, dirnames, filenames in os.walk(os.path.join(build_dir, metadata_path)): - dirnames.sort() - for name in sorted(filenames): - path = os.path.normpath(os.path.join(root, name)) - if os.path.isfile(path): - arcname = os.path.relpath(path, build_dir).replace(os.path.sep, "/") - record(path, arcname) - whl.write(path, arcname) - record_path = os.path.join(metadata_path, "RECORD") - _record.extend([record_path + ",,", ""]) # "" to add newline at eof - whl.writestr(record_path, os.linesep.join(_record)) - -print("editable_path: {{editable_path}}".format(editable_path=wheel_path)) -""" - - -def interpolate_backend_wrapper( +def dump_backend_wrapper_json( dist_dir: str, pth_file_path: str, direct_url: str, request: DistBuildRequest, ) -> bytes: - """This builds a PEP 517 / PEP 660 wrapper script that builds the editable wheel. + """Build the settings json for our PEP 517 / PEP 660 wrapper script.""" - This backend wrapper script, along with the commands that install the editable wheel, - need to conform to the following specs so that Pants is a PEP 660 compliant frontend, - a PEP 660 compliant backend, and that it builds a compliant wheel and install. - - Relevant Specs: - https://peps.python.org/pep-0517/ - https://peps.python.org/pep-0660/ - https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/ - https://packaging.python.org/en/latest/specifications/recording-installed-packages/ - https://packaging.python.org/en/latest/specifications/direct-url-data-structure/ - https://packaging.python.org/en/latest/specifications/binary-distribution-format/ - """ - module_path, _, object_path = request.build_system.build_backend.partition(":") - backend_object = f"{module_path}.{object_path}" if object_path else module_path - - def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: + def clean_config_settings(cs: FrozenDict[str, tuple[str, ...]] | None) -> dict[str, list[str]] | None: # setuptools.build_meta expects list values and chokes on tuples. # We assume/hope that other backends accept lists as well. - return distutils_repr(None if cs is None else {k: list(v) for k, v in cs.items()}) + return None if cs is None else {k: list(v) for k, v in cs.items()} # tag the editable wheel as widely compatible lang_tag, abi_tag, platform_tag = "py3", "none", "any" @@ -223,21 +83,39 @@ def config_settings_repr(cs: FrozenDict[str, tuple[str, ...]] | None) -> str: # Assume everything has py3 support. If not, we'll need a new includes_python3 method. lang_tag = "py2.py3" - return _BACKEND_WRAPPER_BOILERPLATE.format( - build_backend_module=module_path, - build_backend_object=backend_object, - dist_dir=dist_dir, - pth_file_path=pth_file_path, - wheel_config_settings_str=config_settings_repr(request.wheel_config_settings), - tags="-".join([lang_tag, abi_tag, platform_tag]), - direct_url=direct_url, - ).encode() + settings = { + "build_backend": request.build_system.build_backend, + "dist_dir": dist_dir, + "pth_file_path": pth_file_path, + "wheel_config_settings": clean_config_settings(request.wheel_config_settings), + "platform_compatibility_tags": "-".join([lang_tag, abi_tag, platform_tag]), + "direct_url": direct_url, + } + return json.dumps(settings) @rule async def run_pep660_build( request: DistBuildRequest, python_setup: PythonSetup, build_root: BuildRoot ) -> PEP660BuildResult: + """Run our PEP 517 / PEP 660 wrapper script to generate an editable wheel. + + The PEP 517 / PEP 660 wraper script is responsible for building the editable wheel. + The backend wrapper script, along with the commands that install the editable wheel, + need to conform to the following specs so that Pants is a PEP 660 compliant frontend, + a PEP 660 compliant backend, and that it builds a compliant wheel and install. + + NOTE: PEP 660 does not address the `.data` directory, so the wrapper ignores it. + + Relevant Specs: + https://peps.python.org/pep-0517/ + https://peps.python.org/pep-0660/ + https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/ + https://packaging.python.org/en/latest/specifications/recording-installed-packages/ + https://packaging.python.org/en/latest/specifications/direct-url-data-structure/ + https://packaging.python.org/en/latest/specifications/binary-distribution-format/ + """ + # Create the .pth files to add the relevant source root to sys.path. # We cannot use the build backend to do this because we do not want to tell # it where the workspace is and risk it adding anything there. @@ -246,7 +124,6 @@ async def run_pep660_build( # wheels based on https://pypi.org/project/editables/, but that only # supports python3.7+ (what pip supports as of April 2023). # Or maybe do something like setuptools strict editable wheel. - pth_file_contents = "" direct_url = "" for source_root in request.build_time_source_roots: @@ -259,46 +136,57 @@ async def run_pep660_build( direct_url = "file://" + abs_path.replace(os.path.sep, "/") pth_file_name = "__pants__.pth" pth_file_path = os.path.join(request.working_directory, pth_file_name) - pth_file_digest = await Get( - Digest, - CreateDigest([FileContent(pth_file_path, pth_file_contents.encode())]), - ) - # Note that this pex has no entrypoint. We use it to run our generated wrapper, which - # in turn imports from and invokes the build backend. - build_backend_pex = await Get( - VenvPex, - PexRequest( - output_filename="build_backend.pex", - internal_only=True, - requirements=request.build_system.requires, - pex_path=request.extra_build_time_requirements, - interpreter_constraints=request.interpreter_constraints, - ), - ) # This is the setuptools dist directory, not Pants's, so we hardcode to dist/. dist_dir = "dist" dist_output_dir = os.path.join(dist_dir, request.output_path) + backend_wrapper_json = "backend_wrapper.json" + backend_wrapper_json_path = os.path.join(request.working_directory, backend_wrapper_json) backend_wrapper_name = "backend_wrapper.py" backend_wrapper_path = os.path.join(request.working_directory, backend_wrapper_name) - backend_wrapper_digest = await Get( - Digest, - CreateDigest( - [ - FileContent( - backend_wrapper_path, - interpolate_backend_wrapper( - dist_output_dir, pth_file_name, direct_url, request + backend_wrapper_content = read_resource(_scripts_package, "pep660_backend_wrapper.py") + assert backend_wrapper_content is not None + + conf_digest, backend_wrapper_digest, build_backend_pex = await MultiGet( + Get( + Digest, + CreateDigest( + [ + FileContent(pth_file_path, pth_file_contents.encode()), + FileContent( + backend_wrapper_json, + dump_backend_wrapper_json( + dist_output_dir, pth_file_name, direct_url, request + ), ), - ), - ] + ] + ), ), + # The backend_wrapper has its own digest for cache reuse. + Get( + Digest, + CreateDigest( + [FileContent(backend_wrapper_name, backend_wrapper_content)] + ), + ), + # Note that this pex has no entrypoint. We use it to run our wrapper, which + # in turn imports from and invokes the build backend. + Get( + VenvPex, + PexRequest( + output_filename="build_backend.pex", + internal_only=True, + requirements=request.build_system.requires, + pex_path=request.extra_build_time_requirements, + interpreter_constraints=request.interpreter_constraints, + ), + ) ) merged_digest = await Get( - Digest, MergeDigests((request.input, pth_file_digest, backend_wrapper_digest)) + Digest, MergeDigests((request.input, conf_digest, backend_wrapper_digest)) ) extra_env = { @@ -312,7 +200,7 @@ async def run_pep660_build( ProcessResult, VenvPexProcess( build_backend_pex, - argv=(backend_wrapper_name,), + argv=(backend_wrapper_name, backend_wrapper_json), input_digest=merged_digest, extra_env=extra_env, working_directory=request.working_directory, diff --git a/src/python/pants/backend/python/util_rules/scripts/BUILD b/src/python/pants/backend/python/util_rules/scripts/BUILD new file mode 100644 index 00000000000..db46e8d6c97 --- /dev/null +++ b/src/python/pants/backend/python/util_rules/scripts/BUILD @@ -0,0 +1 @@ +python_sources() diff --git a/src/python/pants/backend/python/util_rules/scripts/__init__.py b/src/python/pants/backend/python/util_rules/scripts/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py b/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py new file mode 100644 index 00000000000..e41f964b6c9 --- /dev/null +++ b/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py @@ -0,0 +1,172 @@ +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import base64 +import hashlib +import importlib +import os +import shutil +import zipfile + + +_WHEEL_TEMPLATE = """\ +Wheel-Version: 1.0 +Generator: pantsbuild.pants +Root-Is-Purelib: true +Tag: {} +Build: 0.editable +""" + + +def import_build_backend(build_backend): + module_path, _, object_path = build_backend.partition(":") + backend_module = importlib.import_module(module_path) + return getattr(backend_module, object_path) if object_path else backend_module + + +def mkdir(directory): + """Python 2.7 doesn't have the exist_ok arg on os.makedirs().""" + try: + os.makedirs(d) + except OSError as e: + if e.errno != errno.EEXIST: + raise + + +def prepare_dist_info(backend, build_dir, wheel_config_settings): + """Use PEP 660 or PEP 517 backend methods to create .dist-info directory. + + PEP 660 defines `prepare_metadata_for_build_editable`. If PEP 660 is not + supported, we fall back to PEP 517's `prepare_metadata_for_build_wheel`. + PEP 517, however, says that method is optional. So finally we fall back + to using `build_wheel` and then extract the dist-info directory and then + delete the extra wheel file (like one of PEP 517's examples). + """ + prepare_metadata = getattr( + backend, + "prepare_metadata_for_build_editable", # PEP 660 + getattr(backend, "prepare_metadata_for_build_wheel", None), # PEP 517 + ) + if prepare_metadata is not None: + print("prepare_metadata: " + str(prepare_metadata)) + metadata_path = _prepare_metadata(build_dir, wheel_config_settings) + else: + # Optional PEP 517 method not defined. Use build_wheel instead. + wheel_path = backend.build_wheel(build_dir, wheel_config_settings) + with zipfile.ZipFile(os.path.join(build_dir, wheel_path), "r") as whl: + dist_info_files = [n for n in whl.namelist() if ".dist-info/" in n] + whl.extractall(build_dir, dist_info_files) + metadata_path = os.path.dirname(dist_info_files[0]) + return standardize_dist_info_path(metadata_path) + + +def standardize_dist_info_path(metadata_path): + """Make sure dist-info dir is named pkg-version.dist-info. + + Returns the package name, version, and update metadata_path + """ + pkg_version = metadata_path.replace(".dist-info", "") + if "-" in pkg_version: + pkg, version = pkg_version.split("-") + else: + # The wrapped backend does not conform to the latest specs. + pkg = pkg_version + version = "" + with open(os.path.join(build_dir, metadata_path, "METADATA"), "r") as f: + lines = f.readlines() + for line in lines: + if line.startswith("Version: "): + version = line[len("Version: "):].strip() + break + # Standardize the name of the dist-info directory per Binary distribution format spec. + old_metadata_path = metadata_path + metadata_path = pkg + "-" + version + ".dist-info" + shutil.move(os.path.join(build_dir, old_metadata_path), os.path.join(build_dir, metadata_path)) + return pkg, version, metadata_path + + +def remove_record_files(build_dir, metadata_path): + """Any RECORD* file will be incorrect since we are creating the wheel.""" + for file in os.listdir(os.path.join(build_dir, metadata_path)): + if file == "RECORD" or file.startswith("RECORD."): + os.unlink(os.path.join(build_dir, metadata_path, file)) + + +def write_wheel_file(build_dir, metadata_path): + metadata_wheel_file = os.path.join(build_dir, metadata_path, "WHEEL") + if not os.path.exists(metadata_wheel_file): + with open(metadata_wheel_file, "w") as f: + f.write(_WHEEL_TEMPLATE.format(tags)) + + +def write_direct_url_file(direct_url, build_dir, metadata_path): + """Create a direct_url.json file for later use during wheel install. + + We abuse PEX to get the PEP 660 editable wheels into the virtualenv, and then use + pip to actually install the wheel. But PEX and pip do not know that this is an + editable install. We cannot add direct_url.json directly to the wheel because + that must be added by the wheel installer. So we will rename this file to + 'direct_url.json' after pip has installed everything else. + """ + direct_url_contents = {"url": direct_url, "dir_info": {"editable": True}} + direct_url_file = os.path.join(build_dir, metadata_path, "direct_url__pants__.json") + with open(direct_url_file , "w") as f: + json.dump(direct_url_contents, f) + + +def build_editable_wheel(pkg, build_dir, metadata_path, dist_dir, wheel_path): + """Build the editable wheel, including .pth and RECORD files.""" + + _record = [] + def record(file_path, file_arcname): + """Calculate an entry for the RECORD file (required by the wheel spec).""" + with open(file_path, "rb") as f: + file_digest = hashlib.sha256(f.read()).digest() + file_hash = "sha256=" + base64.urlsafe_b64encode(file_digest).decode().rstrip("=") + file_size = str(os.stat(file_path).st_size) + _record.append(",".join([file_arcname, file_hash, file_size])) + + with zipfile.ZipFile(os.path.join(dist_dir, wheel_path), "w") as whl: + pth_file_arcname = pkg + "__pants__.pth" + record(pth_file_path, pth_file_arcname) + whl.write(pth_file_path, pth_file_arcname) + + # The following for loop is loosely based on: + # wheel.wheelfile.WheelFile.write_files (by @argonholm MIT license) + for root, dirnames, filenames in os.walk(os.path.join(build_dir, metadata_path)): + dirnames.sort() + for name in sorted(filenames): + path = os.path.normpath(os.path.join(root, name)) + if os.path.isfile(path): + arcname = os.path.relpath(path, build_dir).replace(os.path.sep, "/") + record(path, arcname) + whl.write(path, arcname) + + record_path = os.path.join(metadata_path, "RECORD") + _record.append(_record_path + ",,") + _record.append("") # "" to add newline at eof + whl.writestr(record_path, os.linesep.join(_record)) + + +def main(build_backend, dist_dir, pth_file_path, wheel_config_settings, tags, direct_url): + backend = import_build_backend(build_backend) + + build_dir = "build" + mkdir(dist_dir) + mkdir(build_dir) + + pkg, version, metadata_path = prepare_dist_info(backend, build_dir, wheel_config_settings) + remove_record_files(build_dir, metadata_path) + write_wheel_file(build_dir, metadata_path) + write_direct_url_file(direct_url, build_dir, metadata_path) + + wheel_path = "{}-{}-0.editable-{}.whl".format(pkg, version, tags) + build_editable_wheel(pkg, build_dir, metadata_path, dist_dir, wheel_path) + print("editable_path: {editable_path}".format(editable_path=wheel_path)) + + +if __name__ == "__main__": + with open(sys.argv[1], "r") as f: + settings = json.load(f) + + main(**settings) From fb4fbcdd8ceb43ad1d125291bd0b2cf290c2e2a7 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 17:07:18 -0500 Subject: [PATCH 26/32] update INSTALLER file afeter installing PEP-660 editable wheels --- src/python/pants/backend/python/goals/export.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index b733aa812e4..75f17bde258 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -320,17 +320,19 @@ async def do_export( [ # Replace pip's direct_url.json (which points to the temp editable wheel) # with ours (which points to build_dir sources and is marked "editable"). + # Also update INSTALLER file to indicate that pants installed it. "sh", "-c", " ".join( [ - f"mv -f {src} {dst};" - for src, dst in zip( + f"mv -f {src} {dst}; echo pants > {installer};" + for src, dst, installer in zip( [ os.path.join(d, "direct_url__pants__.json") for d in dist_info_dirs ], [os.path.join(d, "direct_url.json") for d in dist_info_dirs], + [os.path.join(d, "INSTALLER") for d in dist_info_dirs], ) ] ), From 26d00916d1d74f561ff51512577bef8ed20b84b2 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 17:09:16 -0500 Subject: [PATCH 27/32] rename option to [export].py_editable_in_resolve --- src/python/pants/backend/python/goals/export.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 75f17bde258..fbcfe900d34 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -127,7 +127,7 @@ class ExportPluginOptions: removal_hint="Set the `[export].py_resolve_format` option to 'symlinked_immutable_virtualenv'", ) - py_editables_in_resolves = StrListOption( + py_editable_in_resolve = 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( @@ -380,7 +380,7 @@ async def export_virtualenv_for_resolve( ) ) - if resolve in export_subsys.options.py_editables_in_resolves: + if resolve in export_subsys.options.py_editable_in_resolve: editable_local_dists = await Get( EditableLocalDists, EditableLocalDistsRequest(resolve=resolve) ) From c50724e01b91044f5147df057964266df62429a8 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 17:42:25 -0500 Subject: [PATCH 28/32] reformat and misc fixes --- .../pants/backend/python/util_rules/dists.py | 1 + .../python/util_rules/local_dists_pep660.py | 23 ++++----- .../backend/python/util_rules/scripts/BUILD | 2 + .../scripts/pep660_backend_wrapper.py | 48 ++++++++++--------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/dists.py b/src/python/pants/backend/python/util_rules/dists.py index 4088713cd28..dfdb768a190 100644 --- a/src/python/pants/backend/python/util_rules/dists.py +++ b/src/python/pants/backend/python/util_rules/dists.py @@ -146,6 +146,7 @@ class DistBuildResult: _BACKEND_SHIM_BOILERPLATE = """ # DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS +import errno import os import {build_backend_module} diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 59220f1daa1..9a473dff569 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -10,16 +10,11 @@ from collections import defaultdict from dataclasses import dataclass -from pants.backend.python.dependency_inference import get_scripts_digest from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import PythonDistributionFieldSet from pants.backend.python.target_types import PythonProvidesField, PythonResolveField from pants.backend.python.util_rules import package_dists -from pants.backend.python.util_rules.dists import ( - BuildBackendError, - DistBuildRequest, - distutils_repr, -) +from pants.backend.python.util_rules.dists import BuildBackendError, DistBuildRequest from pants.backend.python.util_rules.dists import rules as dists_rules from pants.backend.python.util_rules.package_dists import ( DependencyOwner, @@ -49,6 +44,7 @@ from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel from pants.util.osutil import is_macos_big_sur +from pants.util.resources import read_resource from pants.util.strutil import softwrap logger = logging.getLogger(__name__) @@ -72,7 +68,9 @@ def dump_backend_wrapper_json( ) -> bytes: """Build the settings json for our PEP 517 / PEP 660 wrapper script.""" - def clean_config_settings(cs: FrozenDict[str, tuple[str, ...]] | None) -> dict[str, list[str]] | None: + def clean_config_settings( + cs: FrozenDict[str, tuple[str, ...]] | None + ) -> dict[str, list[str]] | None: # setuptools.build_meta expects list values and chokes on tuples. # We assume/hope that other backends accept lists as well. return None if cs is None else {k: list(v) for k, v in cs.items()} @@ -91,7 +89,7 @@ def clean_config_settings(cs: FrozenDict[str, tuple[str, ...]] | None) -> dict[s "platform_compatibility_tags": "-".join([lang_tag, abi_tag, platform_tag]), "direct_url": direct_url, } - return json.dumps(settings) + return json.dumps(settings).encode() @rule @@ -137,15 +135,12 @@ async def run_pep660_build( pth_file_name = "__pants__.pth" pth_file_path = os.path.join(request.working_directory, pth_file_name) - # This is the setuptools dist directory, not Pants's, so we hardcode to dist/. dist_dir = "dist" dist_output_dir = os.path.join(dist_dir, request.output_path) backend_wrapper_json = "backend_wrapper.json" - backend_wrapper_json_path = os.path.join(request.working_directory, backend_wrapper_json) backend_wrapper_name = "backend_wrapper.py" - backend_wrapper_path = os.path.join(request.working_directory, backend_wrapper_name) backend_wrapper_content = read_resource(_scripts_package, "pep660_backend_wrapper.py") assert backend_wrapper_content is not None @@ -167,9 +162,7 @@ async def run_pep660_build( # The backend_wrapper has its own digest for cache reuse. Get( Digest, - CreateDigest( - [FileContent(backend_wrapper_name, backend_wrapper_content)] - ), + CreateDigest([FileContent(backend_wrapper_name, backend_wrapper_content)]), ), # Note that this pex has no entrypoint. We use it to run our wrapper, which # in turn imports from and invokes the build backend. @@ -182,7 +175,7 @@ async def run_pep660_build( pex_path=request.extra_build_time_requirements, interpreter_constraints=request.interpreter_constraints, ), - ) + ), ) merged_digest = await Get( diff --git a/src/python/pants/backend/python/util_rules/scripts/BUILD b/src/python/pants/backend/python/util_rules/scripts/BUILD index db46e8d6c97..752b6d575e9 100644 --- a/src/python/pants/backend/python/util_rules/scripts/BUILD +++ b/src/python/pants/backend/python/util_rules/scripts/BUILD @@ -1 +1,3 @@ +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). python_sources() diff --git a/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py b/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py index e41f964b6c9..75b9931adf8 100644 --- a/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py +++ b/src/python/pants/backend/python/util_rules/scripts/pep660_backend_wrapper.py @@ -2,13 +2,15 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import base64 +import errno import hashlib import importlib +import json import os import shutil +import sys import zipfile - _WHEEL_TEMPLATE = """\ Wheel-Version: 1.0 Generator: pantsbuild.pants @@ -27,7 +29,7 @@ def import_build_backend(build_backend): def mkdir(directory): """Python 2.7 doesn't have the exist_ok arg on os.makedirs().""" try: - os.makedirs(d) + os.makedirs(directory) except OSError as e: if e.errno != errno.EEXIST: raise @@ -36,10 +38,9 @@ def mkdir(directory): def prepare_dist_info(backend, build_dir, wheel_config_settings): """Use PEP 660 or PEP 517 backend methods to create .dist-info directory. - PEP 660 defines `prepare_metadata_for_build_editable`. If PEP 660 is not - supported, we fall back to PEP 517's `prepare_metadata_for_build_wheel`. - PEP 517, however, says that method is optional. So finally we fall back - to using `build_wheel` and then extract the dist-info directory and then + PEP 660 defines `prepare_metadata_for_build_editable`. If PEP 660 is not supported, we fall back + to PEP 517's `prepare_metadata_for_build_wheel`. PEP 517, however, says that method is optional. + So finally we fall back to using `build_wheel` and then extract the dist-info directory and then delete the extra wheel file (like one of PEP 517's examples). """ prepare_metadata = getattr( @@ -49,7 +50,7 @@ def prepare_dist_info(backend, build_dir, wheel_config_settings): ) if prepare_metadata is not None: print("prepare_metadata: " + str(prepare_metadata)) - metadata_path = _prepare_metadata(build_dir, wheel_config_settings) + metadata_path = prepare_metadata(build_dir, wheel_config_settings) else: # Optional PEP 517 method not defined. Use build_wheel instead. wheel_path = backend.build_wheel(build_dir, wheel_config_settings) @@ -57,10 +58,10 @@ def prepare_dist_info(backend, build_dir, wheel_config_settings): dist_info_files = [n for n in whl.namelist() if ".dist-info/" in n] whl.extractall(build_dir, dist_info_files) metadata_path = os.path.dirname(dist_info_files[0]) - return standardize_dist_info_path(metadata_path) + return standardize_dist_info_path(build_dir, metadata_path) -def standardize_dist_info_path(metadata_path): +def standardize_dist_info_path(build_dir, metadata_path): """Make sure dist-info dir is named pkg-version.dist-info. Returns the package name, version, and update metadata_path @@ -76,12 +77,14 @@ def standardize_dist_info_path(metadata_path): lines = f.readlines() for line in lines: if line.startswith("Version: "): - version = line[len("Version: "):].strip() + version = line[len("Version: ") :].strip() break # Standardize the name of the dist-info directory per Binary distribution format spec. old_metadata_path = metadata_path metadata_path = pkg + "-" + version + ".dist-info" - shutil.move(os.path.join(build_dir, old_metadata_path), os.path.join(build_dir, metadata_path)) + shutil.move( + os.path.join(build_dir, old_metadata_path), os.path.join(build_dir, metadata_path) + ) return pkg, version, metadata_path @@ -92,7 +95,7 @@ def remove_record_files(build_dir, metadata_path): os.unlink(os.path.join(build_dir, metadata_path, file)) -def write_wheel_file(build_dir, metadata_path): +def write_wheel_file(tags, build_dir, metadata_path): metadata_wheel_file = os.path.join(build_dir, metadata_path, "WHEEL") if not os.path.exists(metadata_wheel_file): with open(metadata_wheel_file, "w") as f: @@ -102,22 +105,23 @@ def write_wheel_file(build_dir, metadata_path): def write_direct_url_file(direct_url, build_dir, metadata_path): """Create a direct_url.json file for later use during wheel install. - We abuse PEX to get the PEP 660 editable wheels into the virtualenv, and then use - pip to actually install the wheel. But PEX and pip do not know that this is an - editable install. We cannot add direct_url.json directly to the wheel because - that must be added by the wheel installer. So we will rename this file to - 'direct_url.json' after pip has installed everything else. + We abuse PEX to get the PEP 660 editable wheels into the virtualenv, and then use pip to + actually install the wheel. But PEX and pip do not know that this is an editable install. We + cannot add direct_url.json directly to the wheel because that must be added by the wheel + installer. So we will rename this file to 'direct_url.json' after pip has installed everything + else. """ direct_url_contents = {"url": direct_url, "dir_info": {"editable": True}} direct_url_file = os.path.join(build_dir, metadata_path, "direct_url__pants__.json") - with open(direct_url_file , "w") as f: + with open(direct_url_file, "w") as f: json.dump(direct_url_contents, f) -def build_editable_wheel(pkg, build_dir, metadata_path, dist_dir, wheel_path): +def build_editable_wheel(pkg, build_dir, metadata_path, dist_dir, wheel_path, pth_file_path): """Build the editable wheel, including .pth and RECORD files.""" _record = [] + def record(file_path, file_arcname): """Calculate an entry for the RECORD file (required by the wheel spec).""" with open(file_path, "rb") as f: @@ -143,7 +147,7 @@ def record(file_path, file_arcname): whl.write(path, arcname) record_path = os.path.join(metadata_path, "RECORD") - _record.append(_record_path + ",,") + _record.append(record_path + ",,") _record.append("") # "" to add newline at eof whl.writestr(record_path, os.linesep.join(_record)) @@ -157,11 +161,11 @@ def main(build_backend, dist_dir, pth_file_path, wheel_config_settings, tags, di pkg, version, metadata_path = prepare_dist_info(backend, build_dir, wheel_config_settings) remove_record_files(build_dir, metadata_path) - write_wheel_file(build_dir, metadata_path) + write_wheel_file(tags, build_dir, metadata_path) write_direct_url_file(direct_url, build_dir, metadata_path) wheel_path = "{}-{}-0.editable-{}.whl".format(pkg, version, tags) - build_editable_wheel(pkg, build_dir, metadata_path, dist_dir, wheel_path) + build_editable_wheel(pkg, build_dir, metadata_path, dist_dir, wheel_path, pth_file_path) print("editable_path: {editable_path}".format(editable_path=wheel_path)) From e4f07487f094bf6ecb451132f85281137cc813f4 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 19:45:17 -0500 Subject: [PATCH 29/32] fix arg name --- .../pants/backend/python/goals/export_integration_test.py | 2 +- src/python/pants/backend/python/goals/export_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/goals/export_integration_test.py b/src/python/pants/backend/python/goals/export_integration_test.py index 75b47a74dcd..5a73b5aada1 100644 --- a/src/python/pants/backend/python/goals/export_integration_test.py +++ b/src/python/pants/backend/python/goals/export_integration_test.py @@ -110,7 +110,7 @@ def test_export(py_resolve_format: PythonResolveExportFormat) -> None: "generate-lockfiles", "export", *(f"--resolve={name}" for name in resolve_names), - "--export-py-editables-in-resolves=['a']", + "--export-py-editable-in-resolve=['a']", ], config=build_config(tmpdir, py_resolve_format), ).assert_success() diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index faf931f87e3..3d4b6c76696 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -208,7 +208,7 @@ def test_export_venv_new_codepath( "--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']", + "--export-py-editable-in-resolve=['a', 'b']", format_flag, ], env_inherit={"PATH", "PYENV_ROOT"}, From 9090edf3dba2a2680fb5fee27f0c33b8094c9554 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 8 Apr 2023 22:51:41 -0500 Subject: [PATCH 30/32] add deps --- src/python/pants/backend/python/util_rules/BUILD | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/BUILD b/src/python/pants/backend/python/util_rules/BUILD index c4b26a9160b..cd3c7118ce0 100644 --- a/src/python/pants/backend/python/util_rules/BUILD +++ b/src/python/pants/backend/python/util_rules/BUILD @@ -1,7 +1,11 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -python_sources() +python_sources( + overrides={ + "local_dists_pep660.py": dict(dependencies=["./scripts"]), + } +) python_tests( name="tests", From 15fbc1e1fc92f2a2f36e15495e47549bc80e62a2 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sun, 9 Apr 2023 00:27:43 -0500 Subject: [PATCH 31/32] backend wrapper bugs --- .../pants/backend/python/util_rules/local_dists_pep660.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660.py b/src/python/pants/backend/python/util_rules/local_dists_pep660.py index 9a473dff569..72ddbe479fa 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660.py @@ -86,7 +86,7 @@ def clean_config_settings( "dist_dir": dist_dir, "pth_file_path": pth_file_path, "wheel_config_settings": clean_config_settings(request.wheel_config_settings), - "platform_compatibility_tags": "-".join([lang_tag, abi_tag, platform_tag]), + "tags": "-".join([lang_tag, abi_tag, platform_tag]), "direct_url": direct_url, } return json.dumps(settings).encode() @@ -140,7 +140,9 @@ async def run_pep660_build( dist_output_dir = os.path.join(dist_dir, request.output_path) backend_wrapper_json = "backend_wrapper.json" + backend_wrapper_json_path = os.path.join(request.working_directory, backend_wrapper_json) backend_wrapper_name = "backend_wrapper.py" + backend_wrapper_path = os.path.join(request.working_directory, backend_wrapper_name) backend_wrapper_content = read_resource(_scripts_package, "pep660_backend_wrapper.py") assert backend_wrapper_content is not None @@ -151,7 +153,7 @@ async def run_pep660_build( [ FileContent(pth_file_path, pth_file_contents.encode()), FileContent( - backend_wrapper_json, + backend_wrapper_json_path, dump_backend_wrapper_json( dist_output_dir, pth_file_name, direct_url, request ), @@ -162,7 +164,7 @@ async def run_pep660_build( # The backend_wrapper has its own digest for cache reuse. Get( Digest, - CreateDigest([FileContent(backend_wrapper_name, backend_wrapper_content)]), + CreateDigest([FileContent(backend_wrapper_path, backend_wrapper_content)]), ), # Note that this pex has no entrypoint. We use it to run our wrapper, which # in turn imports from and invokes the build backend. From 82501033b90da7dfd35df98fd450c0034116c71e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 10 Apr 2023 14:43:19 -0500 Subject: [PATCH 32/32] make explicit BUILD dep more precise --- src/python/pants/backend/python/util_rules/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/BUILD b/src/python/pants/backend/python/util_rules/BUILD index cd3c7118ce0..d8b31bdafde 100644 --- a/src/python/pants/backend/python/util_rules/BUILD +++ b/src/python/pants/backend/python/util_rules/BUILD @@ -3,7 +3,7 @@ python_sources( overrides={ - "local_dists_pep660.py": dict(dependencies=["./scripts"]), + "local_dists_pep660.py": dict(dependencies=["./scripts/pep660_backend_wrapper.py"]), } )