Support pex_binary addresses in provides=setup_py(entry_points) f…

…ield (#12414)

With the new `entry_points` field on `python_distribution` that supports arbitrary entry points for the `setuptools.setup()` call, including getting the entry point method from a `pex_binary` targets entry point and dependency inference, this PR brings the same features to the `setup_py(entry_points)` object field.

By uniting all entry point processing to the `resolve_python_distribution_entry_points` and `inject_python_distribution_dependencies` rules, it is easier to treat all the different sources of entry points the same.

Legacy `with_binaries()` processing in the `generate_chroot` rule can thus be removed in favor of the updated python distribution rules mentioned above.

[ci skip-rust]
[ci skip-build-wheels]

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()
     }
 

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"),
     )

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
 
         """,
     )

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):