diff --git a/src/python/pants/backend/python/goals/setup_py.py b/src/python/pants/backend/python/goals/setup_py.py index ccb730a933f..b76b03cb983 100644 --- a/src/python/pants/backend/python/goals/setup_py.py +++ b/src/python/pants/backend/python/goals/setup_py.py @@ -16,14 +16,11 @@ from pants.backend.python.macros.python_artifact import PythonArtifact from pants.backend.python.subsystems.setuptools import Setuptools from pants.backend.python.target_types import ( - PexEntryPointField, PythonDistributionEntryPointsField, PythonProvidesField, PythonRequirementsField, PythonSources, - ResolvedPexEntryPoint, ResolvedPythonDistributionEntryPoints, - ResolvePexEntryPointRequest, ResolvePythonDistributionEntryPointsRequest, SetupPyCommandsField, ) @@ -563,72 +560,48 @@ async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot: } ) - # Collect any `pex_binary` targets from `setup_py().with_binaries()` - key_to_binary_spec = exported_target.provides.binaries - binaries = await Get( - Targets, UnparsedAddressInputs(key_to_binary_spec.values(), owning_address=target.address) - ) - entry_point_requests = [] - for binary in binaries: - if not binary.has_field(PexEntryPointField): - raise InvalidEntryPoint( - "Expected addresses to `pex_binary` targets in `.with_binaries()` for the " - f"`provides` field for {exported_addr}, but found {binary.address} with target " - f"type {binary.alias}." - ) - entry_point = binary[PexEntryPointField].value - url = "https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html#the-console-scripts-entry-point" - if not entry_point.function: - raise InvalidEntryPoint( - "Every `pex_binary` used in `with_binaries()` for the `provides()` field for " - f"{exported_addr} must end in the format `:my_func` for the `entry_point` field, " - f"but {binary.address} set it to {entry_point.spec!r}. For example, set " - f"`entry_point='{entry_point.module}:main'. See {url}." - ) - entry_point_requests.append(ResolvePexEntryPointRequest(binary[PexEntryPointField])) - - binary_entry_points = await MultiGet( - Get(ResolvedPexEntryPoint, ResolvePexEntryPointRequest, request) - for request in entry_point_requests - ) - - entry_points_from_with_binaries = { - key: binary_entry_point.val.spec - for key, binary_entry_point in zip(key_to_binary_spec.keys(), binary_entry_points) - if binary_entry_point.val is not None - } - - # Collect entry points from `python_distribution(entry_points=...)` - entry_points_from_field = {} - if exported_target.target.has_field(PythonDistributionEntryPointsField): - entry_points_field = await Get( + # Resolve entry points from python_distribution(entry_points=...) and from + # python_distribution(provides=setup_py(entry_points=...).with_binaries(...) + resolved_from_entry_points_field, resolved_from_provides_field = await MultiGet( + Get( ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest( - exported_target.target[PythonDistributionEntryPointsField] + entry_points_field=exported_target.target.get(PythonDistributionEntryPointsField) ), - ) + ), + Get( + ResolvedPythonDistributionEntryPoints, + ResolvePythonDistributionEntryPointsRequest( + provides_field=exported_target.target.get(PythonProvidesField) + ), + ), + ) - entry_points_from_field = { + def _format_entry_points( + resolved: ResolvedPythonDistributionEntryPoints, + ) -> Dict[str, Dict[str, str]]: + return { category: {ep_name: ep_val.entry_point.spec for ep_name, ep_val in entry_points.items()} - for category, entry_points in entry_points_field.val.items() + for category, entry_points in resolved.val.items() } - # Collect any entry points from the setup_py() object. Note that this was already normalized in - # `python_artifacts.py`. - entry_points_from_provides = setup_kwargs.get("entry_points", {}) - # Gather entry points with source description for any error messages when merging them. entry_point_sources = { - f"{exported_addr}'s field `provides=setup_py().with_binaries()`": { - "console_scripts": entry_points_from_with_binaries - }, - f"{exported_addr}'s field `entry_points`": entry_points_from_field, - f"{exported_addr}'s field `provides=setup_py(..., entry_points={...})`": entry_points_from_provides, + f"{exported_addr}'s field `entry_points`": _format_entry_points( + resolved_from_entry_points_field + ), + f"{exported_addr}'s field `provides=setup_py()`": _format_entry_points( + resolved_from_provides_field + ), } + # Merge all collected entry points and add them to the dist's entry points. - entry_points = merge_entry_points(*list(entry_point_sources.items())) - if entry_points: - setup_kwargs["entry_points"] = entry_points + all_entry_points = merge_entry_points(*list(entry_point_sources.items())) + if all_entry_points: + setup_kwargs["entry_points"] = { + category: [f"{name} = {entry_point}" for name, entry_point in entry_points.items()] + for category, entry_points in all_entry_points.items() + } # Generate the setup script. setup_py_content = SETUP_BOILERPLATE.format( @@ -1011,7 +984,7 @@ def has_args(call_node: ast.Call, required_arg_ids: Tuple[str, ...]) -> bool: def merge_entry_points( *all_entry_points_with_descriptions_of_source: Tuple[str, Dict[str, Dict[str, str]]] -) -> Dict[str, List[str]]: +) -> Dict[str, Dict[str, str]]: """Merge all entry points, throwing ValueError if there are any conflicts.""" merged = cast( # this gives us a two level deep defaultdict with the inner values being of list type @@ -1024,22 +997,22 @@ def merge_entry_points( for ep_name, entry_point in entry_points.items(): merged[category][ep_name].append((description_of_source, entry_point)) - def _merge_entry_point( + def _check_entry_point_single_source( category: str, name: str, entry_points_with_source: List[Tuple[str, str]] - ): + ) -> Tuple[str, str]: if len(entry_points_with_source) > 1: raise ValueError( f"Multiple entry_points registered for {category} {name} in: " f"{', '.join(ep_source for ep_source, _ in entry_points_with_source)}" ) - for _, entry_point in entry_points_with_source: - return f"{name}={entry_point}" + _, entry_point = entry_points_with_source[0] + return name, entry_point return { - category: [ - _merge_entry_point(category, name, entry_points_with_source) + category: dict( + _check_entry_point_single_source(category, name, entry_points_with_source) for name, entry_points_with_source in merged_entry_points.items() - ] + ) for category, merged_entry_points in merged.items() } diff --git a/src/python/pants/backend/python/goals/setup_py_test.py b/src/python/pants/backend/python/goals/setup_py_test.py index b6adffb86d4..ae64a786edc 100644 --- a/src/python/pants/backend/python/goals/setup_py_test.py +++ b/src/python/pants/backend/python/goals/setup_py_test.py @@ -230,15 +230,15 @@ def test_merge_entry_points() -> None: }, } expect = { - "console_scripts": [ - "foo_tool=foo.bar.baz:Tool.main", - "foo_qux=foo.baz.qux", - "foo_main=foo.qux.bin:main", - ], - "foo_plugins": [ - "qux=foo.qux", - "foo-bar=foo.bar:plugin", - ], + "console_scripts": { + "foo_tool": "foo.bar.baz:Tool.main", + "foo_qux": "foo.baz.qux", + "foo_main": "foo.qux.bin:main", + }, + "foo_plugins": { + "qux": "foo.qux", + "foo-bar": "foo.bar:plugin", + }, } assert merge_entry_points(*list(sources.items())) == expect @@ -346,7 +346,7 @@ def test_generate_chroot(chroot_rule_runner: RuleRunner) -> None: "namespace_packages": ("foo",), "package_data": {"foo": ("resources/js/code.js",)}, "install_requires": ("baz==1.1.1",), - "entry_points": {"console_scripts": ["foo_main=foo.qux.bin:main"]}, + "entry_points": {"console_scripts": ["foo_main = foo.qux.bin:main"]}, }, Address("src/python/foo", target_name="foo-dist"), ) @@ -384,7 +384,8 @@ def test_generate_chroot_entry_points(chroot_rule_runner: RuleRunner) -> None: name='foo', version='1.2.3', entry_points={ "console_scripts":{ - "foo_qux":"foo.baz.qux", + "foo_qux":"foo.baz.qux:main", + "foo_bin":":foo-bin", }, "foo_plugins":[ "foo-bar=foo.bar:plugin", @@ -422,16 +423,17 @@ def test_generate_chroot_entry_points(chroot_rule_runner: RuleRunner) -> None: "install_requires": tuple(), "entry_points": { "console_scripts": [ - "foo_main=foo.qux.bin:main", - "foo_tool=foo.bar.baz:Tool.main", - "bin_tool=foo.qux.bin:main", - "bin_tool2=foo.qux.bin:main", - "hello=foo.bin:main", - "foo_qux=foo.baz.qux", + "foo_tool = foo.bar.baz:Tool.main", + "bin_tool = foo.qux.bin:main", + "bin_tool2 = foo.qux.bin:main", + "hello = foo.bin:main", + "foo_qux = foo.baz.qux:main", + "foo_bin = foo.bin:main", + "foo_main = foo.qux.bin:main", ], "foo_plugins": [ - "qux=foo.qux", - "foo-bar=foo.bar:plugin", + "qux = foo.qux", + "foo-bar = foo.bar:plugin", ], }, }, @@ -516,7 +518,7 @@ def test_binary_shorthand(chroot_rule_runner: RuleRunner) -> None: "namespace_packages": (), "install_requires": (), "package_data": {}, - "entry_points": {"console_scripts": ["foo=project.app:func"]}, + "entry_points": {"console_scripts": ["foo = project.app:func"]}, }, Address("src/python/project", target_name="dist"), ) diff --git a/src/python/pants/backend/python/macros/python_artifact.py b/src/python/pants/backend/python/macros/python_artifact.py index 6526e594636..81d8745fc99 100644 --- a/src/python/pants/backend/python/macros/python_artifact.py +++ b/src/python/pants/backend/python/macros/python_artifact.py @@ -123,10 +123,7 @@ def __str__(self) -> str: As before, Pants will infer a dependency on the `pex_binary`. You can confirm this by running - ./pants dependencies path/to:python_distribution. - - For more on command line scripts and entry points, see: - https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html#the-console-scripts-entry-point + ./pants dependencies path/to:python_distribution """, ) diff --git a/src/python/pants/backend/python/target_types.py b/src/python/pants/backend/python/target_types.py index 9689c3f8d2b..56d650a1fde 100644 --- a/src/python/pants/backend/python/target_types.py +++ b/src/python/pants/backend/python/target_types.py @@ -706,7 +706,7 @@ class PythonDistributionDependencies(Dependencies): supports_transitive_excludes = True -class PythonProvidesField(ScalarField, ProvidesField): +class PythonProvidesField(ScalarField, ProvidesField, AsyncFieldMixin): expected_type = PythonArtifact expected_type_help = "setup_py(name='my-dist', **kwargs)" value: PythonArtifact @@ -808,9 +808,21 @@ def pex_binary_addresses(self) -> Addresses: @dataclass(frozen=True) class ResolvePythonDistributionEntryPointsRequest: """Looks at the entry points to see if it is a setuptools entry point, or a BUILD target address - that should be resolved into a setuptools entry point.""" + that should be resolved into a setuptools entry point. - entry_points_field: PythonDistributionEntryPointsField + If the `entry_points_field` is present, inspect the specified entry points. + If the `provides_field` is present, inspect the `provides_field.kwargs["entry_points"]`. + + This is to support inspecting one or the other depending on use case, using the same + logic for resolving pex_binary addresses etc. + """ + + entry_points_field: Optional[PythonDistributionEntryPointsField] = None + provides_field: Optional[PythonProvidesField] = None + + def __post_init__(self): + # Must provide at least one of these fields. + assert self.entry_points_field or self.provides_field class SetupPyCommandsField(StringSequenceField): diff --git a/src/python/pants/backend/python/target_types_rules.py b/src/python/pants/backend/python/target_types_rules.py index d7f58b5062a..f40fa6d7bd5 100644 --- a/src/python/pants/backend/python/target_types_rules.py +++ b/src/python/pants/backend/python/target_types_rules.py @@ -11,12 +11,13 @@ import logging import os.path from collections import defaultdict +from itertools import chain from textwrap import dedent -from typing import DefaultDict, Dict, Generator, Optional, Tuple +from typing import DefaultDict, Dict, Generator, Optional, Tuple, cast from pants.backend.python.dependency_inference.module_mapper import PythonModule, PythonModuleOwners from pants.backend.python.dependency_inference.rules import PythonInferSubsystem, import_rules -from pants.backend.python.goals.setup_py import InvalidEntryPoint +from pants.backend.python.goals.setup_py import InvalidEntryPoint, merge_entry_points from pants.backend.python.target_types import ( EntryPoint, PexBinaryDependencies, @@ -160,8 +161,11 @@ async def inject_pex_binary_entry_point_dependency( # ----------------------------------------------------------------------------------------------- +_EntryPointsDictType = Dict[str, Dict[str, str]] + + def _classify_entry_points( - all_entry_points: FrozenDict[str, FrozenDict[str, str]] + all_entry_points: _EntryPointsDictType, ) -> Generator[Tuple[bool, str, str, str], None, None]: """Looks at each entry point to see if it is a target address or not. @@ -181,12 +185,38 @@ def _classify_entry_points( async def resolve_python_distribution_entry_points( request: ResolvePythonDistributionEntryPointsRequest, ) -> ResolvedPythonDistributionEntryPoints: - field_value = request.entry_points_field.value - if field_value is None: + if request.entry_points_field: + if request.entry_points_field.value is None: + return ResolvedPythonDistributionEntryPoints() + address = request.entry_points_field.address + all_entry_points = cast(_EntryPointsDictType, request.entry_points_field.value) + + elif request.provides_field: + address = request.provides_field.address + provides_field_value = cast( + _EntryPointsDictType, request.provides_field.value.kwargs.get("entry_points") or {} + ) + + with_binaries = request.provides_field.value.binaries + if with_binaries: + all_entry_points = merge_entry_points( + ( + f"{address}'s field `provides=setup_py(entry_points={...})`", + provides_field_value, + ), + ( + f"{address}'s field `provides=setup_py().with_binaries(...)", + {"console_scripts": with_binaries}, + ), + ) + elif provides_field_value: + all_entry_points = provides_field_value + else: + return ResolvedPythonDistributionEntryPoints() + else: return ResolvedPythonDistributionEntryPoints() - address = request.entry_points_field.address - classified_entry_points = list(_classify_entry_points(field_value)) + classified_entry_points = list(_classify_entry_points(all_entry_points)) # Pick out all target addresses up front, so we can use MultiGet later. # @@ -209,7 +239,7 @@ async def resolve_python_distribution_entry_points( # Check that we only have targets with a pex entry_point field. for target in targets: if not target.has_field(PexEntryPointField): - raise InvalidFieldException( + raise InvalidEntryPoint( "All target addresses in the entry_points field must be for pex_binary targets, " f"but the target {address} includes the value {target.address}, which has the " f"target type {target.alias}.\n\n" @@ -281,12 +311,18 @@ async def inject_python_distribution_dependencies( return InjectedDependencies() original_tgt = await Get(WrappedTarget, Address, request.dependencies_field.address) - explicitly_provided_deps, all_entry_points = await MultiGet( + explicitly_provided_deps, distribution_entry_points, provides_entry_points = await MultiGet( Get(ExplicitlyProvidedDependencies, DependenciesRequest(original_tgt.target[Dependencies])), Get( ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest( - original_tgt.target[PythonDistributionEntryPointsField] + entry_points_field=original_tgt.target[PythonDistributionEntryPointsField] + ), + ), + Get( + ResolvedPythonDistributionEntryPoints, + ResolvePythonDistributionEntryPointsRequest( + provides_field=original_tgt.target[PythonProvidesField] ), ), ) @@ -294,7 +330,10 @@ async def inject_python_distribution_dependencies( address = original_tgt.target.address all_module_entry_points = [ (category, name, entry_point) - for category, entry_points in all_entry_points.explicit_modules.items() + for category, entry_points in chain( + distribution_entry_points.explicit_modules.items(), + provides_entry_points.explicit_modules.items(), + ) for name, entry_point in entry_points.items() ] all_module_owners = iter( @@ -322,21 +361,10 @@ async def inject_python_distribution_dependencies( ) module_owners.update(unambiguous_owners) - with_binaries = original_tgt.target[PythonProvidesField].value.binaries - if not with_binaries: - with_binaries_addresses = Addresses() - else: - # Note that we don't validate that these are all `pex_binary` targets; we don't care about - # that here. `setup_py.py` will do that validation. - with_binaries_addresses = await Get( - Addresses, - UnparsedAddressInputs( - with_binaries.values(), owning_address=request.dependencies_field.address - ), - ) - return InjectedDependencies( - Addresses(module_owners) + with_binaries_addresses + all_entry_points.pex_binary_addresses + Addresses(module_owners) + + distribution_entry_points.pex_binary_addresses + + provides_entry_points.pex_binary_addresses ) diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index 9a348b6a24c..0c8aff53262 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -32,6 +32,7 @@ PythonTestsTimeout, ResolvedPexEntryPoint, ResolvePexEntryPointRequest, + ResolvePythonDistributionEntryPointsRequest, TypeStubsModuleMappingField, parse_requirements_file, ) @@ -366,6 +367,12 @@ def test_parse_requirements_file() -> None: } +def test_resolve_python_distribution_entry_points_required_fields() -> None: + with pytest.raises(AssertionError): + # either `entry_points_field` or `provides_field` is required + ResolvePythonDistributionEntryPointsRequest() + + def test_inject_python_distribution_dependencies() -> None: rule_runner = RuleRunner( rules=[ @@ -429,6 +436,22 @@ def test_inject_python_distribution_dependencies() -> None: } } ) + + python_distribution( + name="third_dep2", + provides=setup_py( + name="my-third", + entry_points={ + "console_scripts":{ + "my-cmd": ":my_binary", + "main": "project.app:main", + }, + "color-plugins":{ + "my-ansi-colors": "colors", + } + } + ) + ) """ ), ) @@ -470,6 +493,15 @@ def assert_injected(address: Address, expected: List[Address]) -> None: ], ) + assert_injected( + Address("project", target_name="third_dep2"), + [ + Address("", target_name="ansicolors"), + Address("project", target_name="my_binary"), + Address("project", relative_file_path="app.py", target_name="my_library"), + ], + ) + @pytest.mark.parametrize( ["raw_value", "expected"],