Override interpreter constraints if global option is passed down (pan…

…tsbuild#6387)

I commandeered @alanbato's PR at pantsbuild#6250

## Problem
The original issue is documented at pantsbuild#6081, an attempt to solve this was pantsbuild#6234, but we've decided to take another path instead.

## Solution
Instead of relying on interpreter constraints when building a pex in the repl task, we're checking if the --python-setup-interpreter-constraints is flagged. If so, it overrides the interpreter filters at interpreter selection time.

## Result
Users should now be able to further filter the python interpreter to use even if the task does not declare an option to do so (like in the repl task)

src/python/pants/backend/python/interpreter_cache.py

@@ -77,6 +77,7 @@ def select_interpreter_for_targets(self, targets):
     """Pick an interpreter compatible with all the specified targets."""
     tgts_by_compatibilities = defaultdict(list)
     filters = set()
+
     for target in targets:
       if isinstance(target, PythonTarget):
         c = self._python_setup.compatibility_or_constraints(target)
@@ -86,7 +87,7 @@ def select_interpreter_for_targets(self, targets):
     allowed_interpreters = set(self.setup(filters=filters))
 
     # Constrain allowed_interpreters based on each target's compatibility requirements.
-    for compatibility in tgts_by_compatibilities.keys():
+    for compatibility in tgts_by_compatibilities:
       compatible_with_target = set(self._matching(allowed_interpreters, compatibility))
       allowed_interpreters &= compatible_with_target
 

src/python/pants/backend/python/subsystems/python_setup.py

@@ -133,7 +133,12 @@ def scratch_dir(self):
     return os.path.join(self.get_options().pants_workdir, *self.options_scope.split('.'))
 
   def compatibility_or_constraints(self, target):
-    """Return either the compatibility of the given target, or the interpreter constraints."""
+    """
+    Return either the compatibility of the given target, or the interpreter constraints.
+    If interpreter constraints are supplied by the CLI flag, return those only.
+    """
+    if self.get_options().is_flagged('interpreter_constraints'):
+      return tuple(self.interpreter_constraints)
     return tuple(target.compatibility or self.interpreter_constraints)
 
   def setuptools_requirement(self):

testprojects/src/python/interpreter_selection/python_3_selection_testing/BUILD

@@ -67,3 +67,29 @@ python_tests(
   ],
   compatibility=['CPython>2.7.6,<3']
 )
+
+python_binary(
+  name='main_py23',
+  source='main_py23.py',
+  compatibility=['CPython>2.7,<4'],
+  dependencies=[
+    ':lib_py23'
+  ]
+)
+
+python_library(
+  name='lib_py23',
+  sources=['lib_py23.py'],
+  compatibility=['CPython>2.7,<4']
+)
+
+python_tests(
+  name='test_py23',
+  sources=[
+    'test_py23.py',
+  ],
+  dependencies=[
+    ':main_py23'
+  ],
+  compatibility=['CPython>2.7,<4']
+)

testprojects/src/python/interpreter_selection/python_3_selection_testing/lib_py23.py

@@ -0,0 +1,19 @@
+# coding=utf-8
+# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+
+def say_something():
+  print('I am a python 2/3 compatible library method.')
+  # Note that ascii exists as a built-in in Python 3 and
+  # does not exist in Python 2.
+  try:
+    ret = ascii
+  except NameError:
+    ret = 'Python2'
+  else:
+    ret = 'Python3'
+  return ret

testprojects/src/python/interpreter_selection/python_3_selection_testing/main_py23.py

@@ -0,0 +1,23 @@
+# coding=utf-8
+# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+import sys
+
+from interpreter_selection.python_3_selection_testing.lib_py23 import say_something
+
+
+# A simple example to test building/running/testing a python 2 binary target
+
+
+def main():
+  v = sys.version_info
+  print(sys.executable)
+  print('%d.%d.%d' % v[0:3])
+  return say_something()
+
+if __name__ == '__main__':
+  main()

testprojects/src/python/interpreter_selection/python_3_selection_testing/test_py23.py

@@ -0,0 +1,22 @@
+# coding=utf-8
+# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+import sys
+
+from interpreter_selection.python_3_selection_testing.main_py23 import main
+
+
+def test_main():
+  print(sys.executable)
+  v = sys.version_info
+  # Note that ascii exists as a built-in in Python 3 and
+  # does not exist in Python 2
+  ret = main()
+  if v[0] == '3':
+    assert ret == 'Python3'
+  else:
+    assert ret == 'Python2'

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

@@ -147,3 +147,49 @@ def test_pants_test_interpreter_selection_with_pexrc(self):
     else:
       print('Could not find both python {} and python {} on system. Skipping.'.format(py27, py3))
       self.skipTest('Missing neccesary Python interpreters on system.')
+
+  def test_pants_test_interpreter_selection_with_option_2(self):
+    """
+    Test that the pants test goal properly constrains the SelectInterpreter task to Python 2
+    using the '--python-setup-interpreter-constraints' option.
+    """
+    if self.has_python_version('2.7'):
+      with temporary_dir() as interpreters_cache:
+        pants_ini_config = {
+          'python-setup': {
+            'interpreter_constraints': ['CPython>=2.7,<4'],
+            'interpreter_cache_dir': interpreters_cache,
+          }
+        }
+      pants_run_2 = self.run_pants(
+        command=[
+          'test',
+          '{}:test_py2'.format(os.path.join(self.testproject,'python_3_selection_testing')),
+          '--python-setup-interpreter-constraints=["CPython<3"]',
+        ],
+        config=pants_ini_config
+      )
+      self.assert_success(pants_run_2)
+
+  def test_pants_test_interpreter_selection_with_option_3(self):
+    """
+    Test that the pants test goal properly constrains the SelectInterpreter task to Python 3
+    using the '--python-setup-interpreter-constraints' option.
+    """
+    if self.has_python_version('3'):
+      with temporary_dir() as interpreters_cache:
+        pants_ini_config = {
+          'python-setup': {
+            'interpreter_constraints': ['CPython>=2.7,<4'],
+            'interpreter_cache_dir': interpreters_cache,
+          }
+        }
+      pants_run_3 = self.run_pants(
+            command=[
+              'test',
+              '{}:test_py3'.format(os.path.join(self.testproject,'python_3_selection_testing')),
+              '--python-setup-interpreter-constraints=["CPython>=3"]',
+            ],
+            config=pants_ini_config
+          )
+      self.assert_success(pants_run_3)

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

@@ -15,9 +15,30 @@ def test_run_repl(self):
     command = ['repl',
                'testprojects/src/python/interpreter_selection:echo_interpreter_version_lib',
                '--python-setup-interpreter-constraints=CPython>=2.7,<3',
-               '--python-setup-interpreter-constraints=CPython>=3.3',
                '--quiet']
     program = 'from interpreter_selection.echo_interpreter_version import say_hello; say_hello()'
     pants_run = self.run_pants(command=command, stdin_data=program)
     output_lines = pants_run.stdout_data.rstrip().split('\n')
     self.assertIn('echo_interpreter_version loaded successfully.', output_lines)
+
+  @ensure_daemon
+  def test_run_repl_with_2(self):
+    # Run a Python 2 repl on a Python 2/3 library target.
+    command = ['repl',
+               'testprojects/src/python/interpreter_selection:echo_interpreter_version_lib',
+               '--python-setup-interpreter-constraints=["CPython<3"]',
+               '--quiet']
+    program = 'from interpreter_selection.echo_interpreter_version import say_hello; say_hello()'
+    pants_run = self.run_pants(command=command, stdin_data=program)
+    self.assertRegexpMatches(pants_run.stdout_data, r'2\.\d\.\d')
+
+  @ensure_daemon
+  def test_run_repl_with_3(self):
+    # Run a Python 3 repl on a Python 2/3 library target. Avoid some known-to-choke-on interpreters.
+    command = ['repl',
+               'testprojects/src/python/interpreter_selection:echo_interpreter_version_lib',
+               '--python-setup-interpreter-constraints=["CPython>=3.3"]',
+               '--quiet']
+    program = 'from interpreter_selection.echo_interpreter_version import say_hello; say_hello()'
+    pants_run = self.run_pants(command=command, stdin_data=program)
+    self.assertRegexpMatches(pants_run.stdout_data, r'3\.\d\.\d')

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

@@ -44,11 +44,38 @@ def test_run_27_and_then_3(self):
       )
       self.assert_success(pants_run_3)
 
+  def test_run_3_by_option(self):
+    if self.skip_if_no_python('3'):
+      return
+
+    with temporary_dir() as interpreters_cache:
+      pants_ini_config = {'python-setup': {'interpreter_constraints': ["CPython>=2.7,<4"],
+        'interpreter_cache_dir': interpreters_cache}}
+      pants_run_3 = self.run_pants(
+        command=['run', '{}:echo_interpreter_version_3'.format(self.testproject),
+        '--python-setup-interpreter-constraints=["CPython>=3"]'],
+        config=pants_ini_config
+      )
+      self.assert_success(pants_run_3)
+
+  def test_run_2_by_option(self):
+    if self.skip_if_no_python('2'):
+      return
+
+    with temporary_dir() as interpreters_cache:
+      pants_ini_config = {'python-setup': {'interpreter_constraints': ["CPython>=2.7,<4"],
+        'interpreter_cache_dir': interpreters_cache}}
+      pants_run_2 = self.run_pants(
+        command=['run', '{}:echo_interpreter_version_2.7'.format(self.testproject),
+        '--python-setup-interpreter-constraints=["CPython<3"]'],
+        config=pants_ini_config
+      )
+      self.assert_success(pants_run_2)
+
   def test_die(self):
     command = ['run',
                '{}:die'.format(self.testproject),
-               '--python-setup-interpreter-constraints=CPython>=2.7,<3',
-               '--python-setup-interpreter-constraints=CPython>=3.3',
+               '--python-setup-interpreter-constraints=["CPython>=2.7,<3", ">=3.3"]',
                '--quiet']
     pants_run = self.run_pants(command=command)
     assert pants_run.returncode == 57
@@ -181,10 +208,13 @@ def _run_echo_version(self, version):
     binary_target = '{}:{}'.format(self.testproject, binary_name)
     # Build a pex.
     # Avoid some known-to-choke-on interpreters.
+    if version == '3':
+      constraint = '["CPython>=3.3"]'
+    else:
+      constraint = '["CPython>=2.7,<3"]'
     command = ['run',
                binary_target,
-               '--python-setup-interpreter-constraints=CPython>=2.7,<3',
-               '--python-setup-interpreter-constraints=CPython>=3.3',
+               '--python-setup-interpreter-constraints={}'.format(constraint),
                '--quiet']
     pants_run = self.run_pants(command=command)
     return pants_run.stdout_data.rstrip().split('\n')[-1]

tests/python/pants_test/python/test_interpreter_selection_integration.py

@@ -14,14 +14,11 @@
 class InterpreterSelectionIntegrationTest(PantsRunIntegrationTest):
   testproject = 'testprojects/src/python/interpreter_selection'
 
-  def test_conflict_via_compatibility(self):
+  def test_cli_option_wins_compatibility_conflict(self):
     # Tests that targets with compatibility conflicts collide.
     binary_target = '{}:deliberately_conficting_compatibility'.format(self.testproject)
     pants_run = self._build_pex(binary_target)
-    self.assert_failure(pants_run,
-                        'Unexpected successful build of {binary}.'.format(binary=binary_target))
-    self.assertIn('Unable to detect a suitable interpreter for compatibilities',
-                  pants_run.stdout_data)
+    self.assert_success(pants_run, 'Failed to build {binary}.'.format(binary=binary_target))
 
   def test_conflict_via_config(self):
     # Tests that targets with compatibility conflict with targets with default compatibility.
@@ -65,7 +62,7 @@ def _echo_version(self, version):
       }
       binary_name = 'echo_interpreter_version_{}'.format(version)
       binary_target = '{}:{}'.format(self.testproject, binary_name)
-      pants_run = self._build_pex(binary_target, config)
+      pants_run = self._build_pex(binary_target, config, version=version)
       self.assert_success(pants_run, 'Failed to build {binary}.'.format(binary=binary_target))
 
       # Run the built pex.
@@ -74,11 +71,14 @@ def _echo_version(self, version):
       (stdout_data, _) = proc.communicate()
       return stdout_data
 
-  def _build_pex(self, binary_target, config=None, args=None):
+  def _build_pex(self, binary_target, config=None, args=None, version='2.7'):
     # By default, Avoid some known-to-choke-on interpreters.
+    if version == '3':
+      constraint = '["CPython>=3.3"]'
+    else:
+      constraint = '["CPython>=2.7,<3"]'
     args = list(args) if args is not None else [
-          '--python-setup-interpreter-constraints=CPython>=2.7,<3',
-          '--python-setup-interpreter-constraints=CPython>=3.3',
+          '--python-setup-interpreter-constraints={}'.format(constraint)
         ]
     command = ['binary', binary_target] + args
     return self.run_pants(command=command, config=config)