Fix a bug in the python venv export logic. (#15294) (#15297)

Previously, and partly as a legacy of an older implementation, we acted as if the requirements pex
had interpreter constraints baked into it. We relied on this when detecting the version of that interpreter.

But a requirements pex is internal-only, and so has no interpreter constraints. So in practice we
were picking whatever interpreter was used to run the pex, and that may not have been compatible
with the relevant constraints.

Now we always use a compatible interpreter.

[ci skip-rust]

[ci skip-build-wheels]

src/python/pants/backend/python/goals/export.py

@@ -12,8 +12,9 @@
 from pants.backend.python.subsystems.setup import PythonSetup
 from pants.backend.python.target_types import PythonResolveField
 from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
-from pants.backend.python.util_rules.pex import Pex, PexProcess, PexRequest
+from pants.backend.python.util_rules.pex import Pex, PexRequest
 from pants.backend.python.util_rules.pex_cli import PexPEX
+from pants.backend.python.util_rules.pex_environment import PythonExecutable
 from pants.backend.python.util_rules.pex_from_targets import RequirementsPexRequest
 from pants.core.goals.export import (
     ExportError,
@@ -26,7 +27,7 @@
 from pants.engine.engine_aware import EngineAwareParameter
 from pants.engine.internals.native_engine import AddPrefix, Digest, MergeDigests
 from pants.engine.internals.selectors import Get, MultiGet
-from pants.engine.process import ProcessResult
+from pants.engine.process import Process, ProcessResult
 from pants.engine.rules import collect_rules, rule
 from pants.engine.target import Target
 from pants.engine.unions import UnionMembership, UnionRule, union
@@ -86,39 +87,28 @@ async def export_virtualenv(
             request.root_python_targets, python_setup
         ) or InterpreterConstraints(python_setup.interpreter_constraints)
 
-    min_interpreter = interpreter_constraints.snap_to_minimum(python_setup.interpreter_universe)
-    if not min_interpreter:
-        err_msg = (
-            (
-                f"The resolve '{request.resolve}' (from `[python].resolves`) has invalid interpreter "
-                f"constraints, which are set via `[python].resolves_to_interpreter_constraints`: "
-                f"{interpreter_constraints}. Could not determine the minimum compatible interpreter."
-            )
-            if request.resolve
-            else (
-                "The following interpreter constraints were computed for all the targets for which "
-                f"export was requested: {interpreter_constraints}. There is no python interpreter "
-                "compatible with these constraints. Please restrict the target set to one that shares "
-                "a compatible interpreter."
-            )
-        )
-        raise ExportError(err_msg)
-
     requirements_pex = await Get(
         Pex,
         RequirementsPexRequest(
             (tgt.address for tgt in request.root_python_targets),
-            hardcoded_interpreter_constraints=min_interpreter,
+            hardcoded_interpreter_constraints=interpreter_constraints,
         ),
     )
 
+    # Note that an internal-only pex will always have the `python` field set.
+    # See the build_pex() rule in pex.py.
+    interpreter = cast(PythonExecutable, requirements_pex.python)
+
     # Get the full python version (including patch #), so we can use it as the venv name.
     res = await Get(
         ProcessResult,
-        PexProcess(
-            pex=requirements_pex,
+        Process(
             description="Get interpreter version",
-            argv=["-c", "import sys; print('.'.join(str(x) for x in sys.version_info[0:3]))"],
+            argv=[
+                interpreter.path,
+                "-c",
+                "import sys; print('.'.join(str(x) for x in sys.version_info[0:3]))",
+            ],
         ),
     )
     py_version = res.stdout.strip().decode()
@@ -132,12 +122,13 @@ async def export_virtualenv(
     merged_digest = await Get(Digest, MergeDigests([pex_pex.digest, requirements_pex.digest]))
     pex_pex_path = os.path.join("{digest_root}", pex_pex.exe)
     return ExportResult(
-        f"virtualenv for the resolve '{request.resolve}' (using {min_interpreter})",
+        f"virtualenv for the resolve '{request.resolve}' (using Python {py_version})",
         dest,
         digest=merged_digest,
         post_processing_cmds=[
             PostProcessingCommand(
                 [
+                    interpreter.path,
                     pex_pex_path,
                     os.path.join("{digest_root}", requirements_pex.name),
                     "venv",
@@ -155,8 +146,15 @@ async def export_virtualenv(
 
 @rule
 async def export_tool(request: ExportPythonTool, pex_pex: PexPEX) -> ExportResult:
+    # TODO: Unify export_virtualenv() and export_tool(), since their implementations mostly overlap.
     dest = os.path.join("python", "virtualenvs", "tools")
     pex = await Get(Pex, PexRequest, request.pex_request)
+    if not request.pex_request.internal_only:
+        raise ExportError(f"The PexRequest for {request.resolve_name} must be internal_only.")
+
+    # Note that an internal-only pex will always have the `python` field set.
+    # See the build_pex() rule in pex.py.
+    interpreter = cast(PythonExecutable, pex.python)
 
     # NOTE: We add a unique-per-tool prefix to the pex_pex path to avoid conflicts when
     # multiple tools are concurrently exporting. Without this prefix all the `export_tool`
@@ -174,6 +172,7 @@ async def export_tool(request: ExportPythonTool, pex_pex: PexPEX) -> ExportResul
         post_processing_cmds=[
             PostProcessingCommand(
                 [
+                    interpreter.path,
                     os.path.join(pex_pex_dest, pex_pex.exe),
                     os.path.join("{digest_root}", pex.name),
                     "venv",

src/python/pants/backend/python/goals/export_test.py

@@ -69,7 +69,9 @@ def run(enable_resolves: bool) -> ExportResults:
             assert len(result.post_processing_cmds) == 2
 
             ppc0 = result.post_processing_cmds[0]
-            assert ppc0.argv == (
+            assert ppc0.argv[1:] == (
+                # 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.
                 os.path.join("{digest_root}", ".", "pex"),
                 os.path.join("{digest_root}", "requirements.pex"),
                 "venv",

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

@@ -217,6 +217,12 @@ def __post_init__(self):
                 f"Given platform constraints {self.platforms} for internal only pex request: "
                 f"{self}."
             )
+        if self.internal_only and self.complete_platforms:
+            raise ValueError(
+                "Internal only PEXes can only constrain interpreters with interpreter_constraints."
+                f"Given complete_platform constraints {self.complete_platforms} for internal only "
+                f"pex request: {self}."
+            )
         if self.python and self.platforms:
             raise ValueError(
                 "Only one of platforms or a specific interpreter may be set. Got "
@@ -347,6 +353,7 @@ async def build_pex(
     # `--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 or request.complete_platforms:
+        # Note that this means that this is not an internal-only pex.
         # TODO(#9560): consider validating that these platforms are valid with the interpreter
         #  constraints.
         argv.extend(request.platforms.generate_pex_arg_list())