Fix V2 binary still using the current platform when platforms is …

…specified (#9563)

### Problem

When specifying `platforms` on a `python_binary`, you'd expect the built wheel to only have those specified platforms. But, it currently will always include the `current` platform.

This is because we are unconditionally passing the flag `--interpreter-constraint` to Pex, which forces `current` to be used, per pex-tool/pex#957. As explained there, Pex is behaving correctly; the issue is with Pants.

### Solution

When `platforms` are set, ignore interpreter constraints. 

Platforms already embed interpreter constraints in them, e.g. `linux-x86_64-cp-37-cp37m` saying that it needs compatibility with CPython 3.7. So, `platforms` is an override over interpreter constraints.

This change only applies when using the target with `./pants run` and `./pants binary`. Other goals like `./pants test` will ignore the `platforms` value and use interpreter constraints like normal.

[ci skip-rust-tests]
[ci skip-jvm-tests]

src/python/pants/backend/python/rules/pex.py

@@ -9,10 +9,9 @@
 
 from pants.backend.python.rules.download_pex_bin import DownloadedPexBin
 from pants.backend.python.rules.hermetic_pex import HermeticPex
-from pants.backend.python.rules.targets import (
-    PythonInterpreterCompatibility,
-    PythonRequirementsField,
-)
+from pants.backend.python.rules.targets import PythonInterpreterCompatibility
+from pants.backend.python.rules.targets import PythonPlatforms as PythonPlatformsField
+from pants.backend.python.rules.targets import PythonRequirementsField
 from pants.backend.python.rules.util import parse_interpreter_constraint
 from pants.backend.python.subsystems.python_native_code import PexBuildEnvironment
 from pants.backend.python.subsystems.subprocess_environment import SubprocessEncodingEnvironment
@@ -181,6 +180,26 @@ def generate_pex_arg_list(self) -> List[str]:
         return args
 
 
+@frozen_after_init
+@dataclass(unsafe_hash=True)
+class PexPlatforms:
+    platforms: FrozenOrderedSet[str]
+
+    def __init__(self, platforms: Optional[Iterable[str]] = None) -> None:
+        self.platforms = FrozenOrderedSet(sorted(platforms or ()))
+
+    @classmethod
+    def create_from_platforms_field(cls, field: PythonPlatformsField) -> "PexPlatforms":
+        # TODO(#9562): wire to `--python-setup-platforms` once we know when/where it should be used.
+        return cls(field.value or ())
+
+    def generate_pex_arg_list(self) -> List[str]:
+        args = []
+        for platform in sorted(self.platforms):
+            args.extend(["--platform", platform])
+        return args
+
+
 @frozen_after_init
 @dataclass(unsafe_hash=True)
 class PexRequest:
@@ -189,6 +208,7 @@ class PexRequest:
     output_filename: str
     requirements: PexRequirements
     interpreter_constraints: PexInterpreterConstraints
+    platforms: PexPlatforms
     sources: Optional[Digest]
     additional_inputs: Optional[Digest]
     entry_point: Optional[str]
@@ -203,6 +223,7 @@ def __init__(
         output_filename: str,
         requirements: PexRequirements = PexRequirements(),
         interpreter_constraints=PexInterpreterConstraints(),
+        platforms=PexPlatforms(),
         sources: Optional[Digest] = None,
         additional_inputs: Optional[Digest] = None,
         entry_point: Optional[str] = None,
@@ -212,6 +233,7 @@ def __init__(
         self.output_filename = output_filename
         self.requirements = requirements
         self.interpreter_constraints = interpreter_constraints
+        self.platforms = platforms
         self.sources = sources
         self.additional_inputs = additional_inputs
         self.entry_point = entry_point
@@ -299,7 +321,6 @@ async def create_pex(
     argv = [
         "--output-file",
         request.output_filename,
-        *request.interpreter_constraints.generate_pex_arg_list(),
         # NB: In setting `--no-pypi`, we rely on the default value of `--python-repos-indexes`
         # including PyPI, which will override `--no-pypi` and result in using PyPI in the default
         # case. Why set `--no-pypi`, then? We need to do this so that
@@ -310,6 +331,16 @@ async def create_pex(
         *request.additional_args,
     ]
 
+    # NB: If `--platform` is specified, this signals that the PEX should not be built locally.
+    # `--interpreter-constraint` only makes sense in the context of building locally. These two
+    # flags are mutually exclusive. See https://github.com/pantsbuild/pex/issues/957.
+    if request.platforms.platforms:
+        # TODO(#9560): consider validating that these platforms are valid with the interpreter
+        #  constraints.
+        argv.extend(request.platforms.generate_pex_arg_list())
+    else:
+        argv.extend(request.interpreter_constraints.generate_pex_arg_list())
+
     pex_debug = PexDebug(log_level)
     argv.extend(pex_debug.iter_pex_args())
 
@@ -407,7 +438,7 @@ async def create_pex(
 
 @rule
 async def two_step_create_pex(two_step_pex_request: TwoStepPexRequest) -> TwoStepPex:
-    """Create a pex in two steps: a requirements-only pex and then a full pex from it."""
+    """Create a PEX in two steps: a requirements-only PEX and then a full PEX from it."""
     request = two_step_pex_request.pex_request
     req_pex_name = "__requirements.pex"
 
@@ -419,6 +450,7 @@ async def two_step_create_pex(two_step_pex_request: TwoStepPexRequest) -> TwoSte
             output_filename=req_pex_name,
             requirements=request.requirements,
             interpreter_constraints=request.interpreter_constraints,
+            platforms=request.platforms,
             # TODO: Do we need to pass all the additional args to the requirements pex creation?
             #  Some of them may affect resolution behavior, but others may be irrelevant.
             #  For now we err on the side of caution.
@@ -432,7 +464,7 @@ async def two_step_create_pex(two_step_pex_request: TwoStepPexRequest) -> TwoSte
         additional_inputs = None
         additional_args = request.additional_args
 
-    # Now create a full pex on top of the requirements pex.
+    # Now create a full PEX on top of the requirements PEX.
     full_pex_request = dataclasses.replace(
         request,
         requirements=PexRequirements(),

src/python/pants/backend/python/rules/pex_from_targets.py

@@ -8,6 +8,7 @@
 from pants.backend.python.rules.importable_python_sources import ImportablePythonSources
 from pants.backend.python.rules.pex import (
     PexInterpreterConstraints,
+    PexPlatforms,
     PexRequest,
     PexRequirements,
     TwoStepPexRequest,
@@ -38,6 +39,7 @@ class PexFromTargetsRequest:
     addresses: Addresses
     output_filename: str
     entry_point: Optional[str]
+    platforms: PexPlatforms
     additional_args: Tuple[str, ...]
     additional_requirements: Tuple[str, ...]
     include_source_files: bool
@@ -53,6 +55,7 @@ def __init__(
         *,
         output_filename: str,
         entry_point: Optional[str] = None,
+        platforms: PexPlatforms = PexPlatforms(),
         additional_args: Iterable[str] = (),
         additional_requirements: Iterable[str] = (),
         include_source_files: bool = True,
@@ -63,6 +66,7 @@ def __init__(
         self.addresses = addresses
         self.output_filename = output_filename
         self.entry_point = entry_point
+        self.platforms = platforms
         self.additional_args = tuple(additional_args)
         self.additional_requirements = tuple(additional_requirements)
         self.include_source_files = include_source_files
@@ -129,6 +133,7 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
         output_filename=request.output_filename,
         requirements=requirements,
         interpreter_constraints=interpreter_constraints,
+        platforms=request.platforms,
         entry_point=request.entry_point,
         sources=merged_input_digest,
         additional_inputs=request.additional_inputs,

src/python/pants/backend/python/rules/pex_test.py

@@ -14,6 +14,7 @@
 from pants.backend.python.rules.pex import (
     Pex,
     PexInterpreterConstraints,
+    PexPlatforms,
     PexRequest,
     PexRequirements,
 )
@@ -168,6 +169,7 @@ def create_pex_and_get_all_data(
         requirements=PexRequirements(),
         entry_point=None,
         interpreter_constraints=PexInterpreterConstraints(),
+        platforms=PexPlatforms(),
         sources: Optional[Digest] = None,
         additional_inputs: Optional[Digest] = None,
         additional_pants_args: Tuple[str, ...] = (),
@@ -177,6 +179,7 @@ def create_pex_and_get_all_data(
             output_filename="test.pex",
             requirements=requirements,
             interpreter_constraints=interpreter_constraints,
+            platforms=platforms,
             entry_point=entry_point,
             sources=sources,
             additional_inputs=additional_inputs,
@@ -210,6 +213,7 @@ def create_pex_and_get_pex_info(
         requirements=PexRequirements(),
         entry_point=None,
         interpreter_constraints=PexInterpreterConstraints(),
+        platforms=PexPlatforms(),
         sources: Optional[Digest] = None,
         additional_pants_args: Tuple[str, ...] = (),
         additional_pex_args: Tuple[str, ...] = (),
@@ -220,6 +224,7 @@ def create_pex_and_get_pex_info(
                 requirements=requirements,
                 entry_point=entry_point,
                 interpreter_constraints=interpreter_constraints,
+                platforms=platforms,
                 sources=sources,
                 additional_pants_args=additional_pants_args,
                 additional_pex_args=additional_pex_args,
@@ -238,22 +243,22 @@ def test_pex_execution(self) -> None:
         pex_output = self.create_pex_and_get_all_data(entry_point="main", sources=sources)
 
         pex_files = pex_output["files"]
-        self.assertTrue("pex" not in pex_files)
-        self.assertTrue("main.py" in pex_files)
-        self.assertTrue("subdir/sub.py" in pex_files)
+        assert "pex" not in pex_files
+        assert "main.py" in pex_files
+        assert "subdir/sub.py" in pex_files
 
         init_subsystem(PythonSetup)
         python_setup = PythonSetup.global_instance()
         env = {"PATH": create_path_env_var(python_setup.interpreter_search_paths)}
 
-        req = Process(
+        process = Process(
             argv=("python", "test.pex"),
             env=env,
             input_files=pex_output["pex"].directory_digest,
             description="Run the pex and make sure it works",
         )
-        result = self.request_single_product(ProcessResult, req)
-        self.assertEqual(result.stdout, b"from main\n")
+        result = self.request_single_product(ProcessResult, process)
+        assert result.stdout == b"from main\n"
 
     def test_resolves_dependencies(self) -> None:
         requirements = PexRequirements(["six==1.12.0", "jsonschema==2.6.0", "requests==2.23.0"])
@@ -298,6 +303,26 @@ def test_additional_args(self) -> None:
         pex_info = self.create_pex_and_get_pex_info(additional_pex_args=("--not-zip-safe",))
         assert pex_info["zip_safe"] is False
 
+    def test_platforms(self) -> None:
+        # We use Python 2.7, rather than Python 3, to ensure that the specified platform is
+        # actually used.
+        platforms = PexPlatforms(["linux-x86_64-cp-27-cp27mu"])
+        constraints = PexInterpreterConstraints(["CPython>=2.7,<3", "CPython>=3.6"])
+        pex_output = self.create_pex_and_get_all_data(
+            requirements=PexRequirements(["cryptography==2.9"]),
+            platforms=platforms,
+            interpreter_constraints=constraints,
+        )
+        assert any(
+            "cryptography-2.9-cp27-cp27mu-manylinux2010_x86_64.whl" in fp
+            for fp in pex_output["files"]
+        )
+        assert not any("cryptography-2.9-cp27-cp27m-" in fp for fp in pex_output["files"])
+        assert not any("cryptography-2.9-cp35-abi3" in fp for fp in pex_output["files"])
+
+        # NB: Platforms override interpreter constraints.
+        assert pex_output["info"]["interpreter_constraints"] == []
+
     def test_additional_inputs(self) -> None:
         # We use pex's --preamble-file option to set a custom premable from a file.
         # This verifies that the file was indeed provided as additional input to the pex call.

src/python/pants/backend/python/rules/python_create_binary.py

@@ -4,7 +4,7 @@
 from dataclasses import dataclass
 from typing import Optional, Tuple
 
-from pants.backend.python.rules.pex import TwoStepPex
+from pants.backend.python.rules.pex import PexPlatforms, TwoStepPex
 from pants.backend.python.rules.pex_from_targets import (
     PexFromTargetsRequest,
     TwoStepPexFromTargetsRequest,
@@ -18,8 +18,8 @@
     PexZipSafe,
     PythonBinarySources,
     PythonEntryPoint,
-    PythonPlatforms,
 )
+from pants.backend.python.rules.targets import PythonPlatforms as PythonPlatformsField
 from pants.backend.python.targets.python_binary import PythonBinary
 from pants.engine.addressable import Addresses
 from pants.engine.rules import UnionRule, rule
@@ -41,7 +41,7 @@ class PythonBinaryConfiguration(BinaryConfiguration):
     inherit_path: PexInheritPath
     shebang: PexShebang
     zip_safe: PexZipSafe
-    platforms: PythonPlatforms
+    platforms: PythonPlatformsField
 
     def generate_additional_args(self) -> Tuple[str, ...]:
         args = []
@@ -55,8 +55,6 @@ def generate_additional_args(self) -> Tuple[str, ...]:
             args.append(f"--inherit-path={self.inherit_path.value}")
         if self.shebang.value is not None:
             args.append(f"--python-shebang={self.shebang.value}")
-        if self.platforms.value is not None:
-            args.extend([f"--platform={platform}" for platform in self.platforms.value])
         return tuple(args)
 
 
@@ -82,6 +80,7 @@ async def create_python_binary(config: PythonBinaryConfiguration) -> CreatedBina
             PexFromTargetsRequest(
                 addresses=Addresses([config.address]),
                 entry_point=entry_point,
+                platforms=PexPlatforms.create_from_platforms_field(config.platforms),
                 output_filename=output_filename,
                 additional_args=config.generate_additional_args(),
                 description=f"Building {output_filename}",