Always use the global interpreter for PythonExecutionTaskBase. (#7801)

The problems observed in #7775 and #7563 had a third (and hopefully final: I'll audit before we land this) buddy in the `PythonRun` case.

The tests in https://github.com/pantsbuild/pants/blob/817f7f6aedad8143fe7b2cf86559019254230da4/tests/python/pants_test/backend/python/tasks/test_python_run_integration.py#L134-L184 attempt to cover this case, but they were not using mixed contexts (contexts containing `(2-or-3, 3-only)` and `(2-only, 2-or-3)` constraints).

Similar to #7775 and #7563, do not include transitive constraints when `./pants run`ing a binary. Clarify the documentation of `PythonExecutionTaskBase`. Expand coverage of the mixed context case in existing tests.

The tests linked above fail before this change for `PythonRun`, and succeed afterward.

src/python/pants/backend/python/tasks/pytest_prep.py

@@ -94,7 +94,7 @@ def execute(self):
       return
     pex_info = PexInfo.default()
     pex_info.entry_point = 'pytest'
-    pytest_binary = self.create_pex(pex_info, pin_selected_interpreter=True)
+    pytest_binary = self.create_pex(pex_info)
     interpreter = self.context.products.get_data(PythonInterpreter)
     self.context.products.register_data(self.PytestBinary,
                                         self.PytestBinary(interpreter, pytest_binary))

src/python/pants/backend/python/tasks/pytest_run.py

@@ -19,6 +19,7 @@
 from pants.backend.python.targets.python_tests import PythonTests
 from pants.backend.python.tasks.gather_sources import GatherSources
 from pants.backend.python.tasks.pytest_prep import PytestPrep
+from pants.backend.python.tasks.python_execution_task_base import ensure_interpreter_search_path_env
 from pants.base.build_environment import get_buildroot
 from pants.base.exceptions import ErrorWhileTesting, TaskError
 from pants.base.fingerprint_strategy import DefaultFingerprintStrategy
@@ -483,7 +484,7 @@ def _test_runner(self, workdirs, test_targets, sources_map):
                                           pytest_binary.pex) as coverage_args:
         yield pytest_binary, [conftest] + coverage_args, get_pytest_rootdir
 
-  def _constrain_pytest_interpreter_search_path(self):
+  def _ensure_pytest_interpreter_search_path(self):
     """Return an environment for invoking a pex which ensures the use of the selected interpreter.
 
     When creating the merged pytest pex, we already have an interpreter, and we only invoke that pex
@@ -492,12 +493,7 @@ def _constrain_pytest_interpreter_search_path(self):
     compatible with the interpreter being used to invoke the merged pytest pex.
     """
     pytest_binary = self.context.products.get_data(PytestPrep.PytestBinary)
-    chosen_interpreter_binary_path = pytest_binary.interpreter.binary
-    return {
-      'PEX_IGNORE_RCFILES': '1',
-      'PEX_PYTHON': chosen_interpreter_binary_path,
-      'PEX_PYTHON_PATH': chosen_interpreter_binary_path,
-    }
+    return ensure_interpreter_search_path_env(pytest_binary.interpreter)
 
   def _do_run_tests_with_args(self, pex, args):
     try:
@@ -535,7 +531,7 @@ def _do_run_tests_with_args(self, pex, args):
                                      cmd=safe_shlex_join(pex.cmdline(args)),
                                      labels=[WorkUnitLabel.TOOL, WorkUnitLabel.TEST]) as workunit:
         # NB: Constrain the pex environment to ensure the use of the selected interpreter!
-        env.update(self._constrain_pytest_interpreter_search_path())
+        env.update(self._ensure_pytest_interpreter_search_path())
         rc = self.spawn_and_wait(pex, workunit=workunit, args=args, setsid=True, env=env)
         return PytestResult.rc(rc)
     except ErrorWhileTesting:
@@ -756,7 +752,7 @@ def _pex_run(self, pex, workunit_name, args, env):
                                    cmd=' '.join(pex.cmdline(args)),
                                    labels=[WorkUnitLabel.TOOL, WorkUnitLabel.TEST]) as workunit:
       # NB: Constrain the pex environment to ensure the use of the selected interpreter!
-      env.update(self._constrain_pytest_interpreter_search_path())
+      env.update(self._ensure_pytest_interpreter_search_path())
       process = self._spawn(pex, workunit, args, setsid=False, env=env)
       return process.wait()
 

src/python/pants/backend/python/tasks/python_execution_task_base.py

@@ -11,7 +11,6 @@
 from pex.interpreter import PythonInterpreter
 from pex.pex import PEX
 
-from pants.backend.python.subsystems.pex_build_util import is_python_target
 from pants.backend.python.subsystems.python_setup import PythonSetup
 from pants.backend.python.targets.python_distribution import PythonDistribution
 from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
@@ -25,6 +24,31 @@
 from pants.util.objects import datatype
 
 
+def ensure_interpreter_search_path_env(interpreter):
+  """Produces an environment dict that ensures that the given interpreter is discovered at runtime.
+
+  At build time, a pex contains constraints on which interpreter version ranges are legal, but
+  at runtime it will apply those constraints to the current (PEX_PYTHON_)PATH to locate a
+  relevant interpreter.
+
+  This environment is for use at runtime to ensure that a particular interpreter (which should
+  match the constraints that were provided at build time) is locatable. PEX no longer allows
+  for forcing use of a particular interpreter (the PEX_PYTHON_PATH is additive), so use
+  of this method does not guarantee that the given interpreter is used: rather, that it is
+  definitely considered for use.
+
+  Subclasses of PythonExecutionTaskBase can use `self.ensure_interpreter_search_path_env`
+  to get the relevant interpreter, but this method is exposed as static for cases where
+  the building of the pex is separated from the execution of the pex.
+  """
+  chosen_interpreter_binary_path = interpreter.binary
+  return {
+    'PEX_IGNORE_RCFILES': '1',
+    'PEX_PYTHON': chosen_interpreter_binary_path,
+    'PEX_PYTHON_PATH': chosen_interpreter_binary_path,
+  }
+
+
 class PythonExecutionTaskBase(ResolveRequirementsTaskBase):
   """Base class for tasks that execute user Python code in a PEX environment.
 
@@ -83,22 +107,23 @@ def extra_files(self):
     """
     return ()
 
-  # TODO: remove `pin_selected_interpreter` arg and constrain all resulting pexes to a single ==
-  # interpreter constraint for the globally selected interpreter! This also requries porting
-  # `PythonRun` to set 'PEX_PYTHON_PATH' and 'PEX_PYTHON' when invoking the resulting pex, see
-  # https://github.com/pantsbuild/pants/pull/7563.
-  def create_pex(self, pex_info=None, pin_selected_interpreter=False):
+  def ensure_interpreter_search_path_env(self):
+    """See ensure_interpreter_search_path_env."""
+    return ensure_interpreter_search_path_env(self.context.products.get_data(PythonInterpreter))
+
+  def create_pex(self, pex_info=None):
     """Returns a wrapped pex that "merges" other pexes produced in previous tasks via PEX_PATH.
 
+    This method always creates a PEX to run locally on the current platform and selected
+    interpreter: to create a pex that is distributable to other environments, use the pex_build_util
+    Subsystem.
+
     The returned pex will have the pexes from the ResolveRequirements and GatherSources tasks mixed
     into it via PEX_PATH. Any 3rdparty requirements declared with self.extra_requirements() will
     also be resolved for the global interpreter, and added to the returned pex via PEX_PATH.
 
     :param pex_info: An optional PexInfo instance to provide to self.merged_pex().
     :type pex_info: :class:`pex.pex_info.PexInfo`, or None
-    :param bool pin_selected_interpreter: If True, the produced pex will have a single ==
-                                          interpreter constraint applied to it, for the global
-                                          interpreter selected by the SelectInterpreter
     task. Otherwise, all of the interpreter constraints from all python targets will applied.
     :rtype: :class:`pex.pex.PEX`
     """
@@ -134,16 +159,8 @@ def create_pex(self, pex_info=None, pin_selected_interpreter=False):
           # in the target set's dependency closure.
           pexes = [extra_requirements_pex] + pexes
 
-        if pin_selected_interpreter:
-          constraints = {str(interpreter.identity.requirement)}
-        else:
-          constraints = {
-            constraint for rt in relevant_targets if is_python_target(rt)
-            for constraint in PythonSetup.global_instance().compatibility_or_constraints(rt)
-          }
-          self.context.log.debug('target set {} has constraints: {}'
-                                 .format(relevant_targets, constraints))
-
+        # NB: See docstring. We always use the previous selected interpreter.
+        constraints = {str(interpreter.identity.requirement)}
 
         with self.merged_pex(path, pex_info, interpreter, pexes, constraints) as builder:
           for extra_file in self.extra_files():

src/python/pants/backend/python/tasks/python_repl.py

@@ -56,6 +56,7 @@ def setup_repl_session(self, targets):
 
   def _run_repl(self, pex, **pex_run_kwargs):
     env = pex_run_kwargs.pop('env', os.environ).copy()
+    env.update(self.ensure_interpreter_search_path_env())
     pex.run(env=env, **pex_run_kwargs)
 
   # N.B. **pex_run_kwargs is used by tests only.

src/python/pants/backend/python/tasks/python_run.py

@@ -40,12 +40,15 @@ def execute(self):
         args.extend(safe_shlex_split(arg))
       args += self.get_passthru_args()
 
+      env = os.environ.copy()
+      env.update(self.ensure_interpreter_search_path_env())
+
       self.context.release_lock()
       cmdline = ' '.join(pex.cmdline(args))
       with self.context.new_workunit(name='run',
                                      cmd=cmdline,
                                      labels=[WorkUnitLabel.TOOL, WorkUnitLabel.RUN]):
-        po = pex.run(blocking=False, args=args, env=os.environ.copy())
+        po = pex.run(blocking=False, args=args, env=env)
         try:
           result = po.wait()
           if result != 0:

testprojects/src/python/interpreter_selection/python_3_selection_testing/BUILD

@@ -21,7 +21,10 @@ python_binary(
   source='main_py3.py',
   compatibility=['CPython>3'],
   dependencies=[
-    ':lib_py3'
+    # We depend on libraries with both narrow and wide ranges in order to confirm that the narrower
+    # range is the one selected.
+    ':lib_py3',
+    ':lib_py23',
   ]
 )
 
@@ -47,7 +50,10 @@ python_binary(
   source='main_py2.py',
   compatibility=['CPython>2.7.6,<3'],
   dependencies=[
-    ':lib_py2'
+    # We depend on libraries with both narrow and wide ranges in order to confirm that the narrower
+    # range is the one selected.
+    ':lib_py2',
+    ':lib_py23',
   ]
 )
 

tests/python/pants_test/backend/python/tasks/test_python_run_integration.py

@@ -143,14 +143,14 @@ def test_pants_run_interpreter_selection_with_pexrc(self):
           config=pants_ini_config
         )
         self.assert_success(pants_run_27)
-        self.assertIn(py27_path, pants_run_27.stdout_data)
+        self.assertIn('I am a python 2 library method.', pants_run_27.stdout_data)
         pants_run_3 = self.run_pants(
           command=['run', '{}:main_py3'.format(os.path.join(self.testproject,
                                                             'python_3_selection_testing'))],
           config=pants_ini_config
         )
         self.assert_success(pants_run_3)
-        self.assertIn(py3_path, pants_run_3.stdout_data)
+        self.assertIn('I am a python 3 library method.', pants_run_3.stdout_data)
 
   @skip_unless_python27_and_python3_present
   def test_pants_binary_interpreter_selection_with_pexrc(self):