Switch to Py2 and Py3 shards.

.travis.yml

@@ -69,6 +69,7 @@ default_test_config: &default_test_config
         - lib32z1-dev
         - gcc-multilib
         - python-dev
+        - python3
         - openssl
         - libssl-dev
   stage: Test Pants
@@ -149,21 +150,24 @@ matrix:
 
     - <<: *default_test_config
       env:
-        - SHARD="Various pants self checks and lint"
+        - SHARD="Self checks, lint, and JVM tests"
       script:
-        - ./build-support/bin/ci.sh -x -cejlpn 'Various pants self checks and lint'
+        - ./build-support/bin/ci.sh -x -celpn "${SHARD}"
 
     - <<: *default_test_config
       env:
-        - SHARD="Unit tests for pants and pants-plugins - shard 1"
+        - SHARD="Py2 - Unit tests for pants and pants-plugins"
       script:
-        - ./build-support/bin/ci.sh -x -efkmrcnt -u 0/2 "${SHARD}"
+        - ./build-support/bin/ci.sh -x -efkmrjcnt "${SHARD}"
 
     - <<: *default_test_config
       env:
-        - SHARD="Unit tests for pants and pants-plugins - shard 2"
+        - SHARD="Py3 - Unit tests for pants and pants-plugins"
+        # Use less verbose logs, in order to not hit the log-length limit.
+        - PYTEST_PASSTHRU_ARGS="--tb=line"
       script:
-        - ./build-support/bin/ci.sh -x -efkmrcnt -u 1/2 "${SHARD}"
+        # Allowed to fail.
+        - ./build-support/bin/ci.sh -x -3efkmrjcnt "${SHARD}" || exit 0
 
     - <<: *default_test_config
       env:

build-support/bin/ci.sh

@@ -13,8 +13,10 @@ function usage() {
   cat <<EOF
 Runs commons tests for local or hosted CI.
 
-Usage: $0 (-h|-fxbkmsrjlpuyncia)
+Usage: $0 (-h|-3fxbkmsrjlpuyncia)
  -h           print out this help message
+ -3           After pants is bootstrapped, set --python-setup-interpreter-constraints such that any
+              python tests run with Python 3.
  -f           skip python code formatting checks
  -x           skip bootstrap clean-all (assume bootstrapping from a
               fresh clone)
@@ -60,10 +62,12 @@ bootstrap_compile_args=(
 python_unit_shard="0/1"
 python_contrib_shard="0/1"
 python_intg_shard="0/1"
+python_three="false"
 
-while getopts "hfxbkmrjlpeu:ny:ci:tz" opt; do
+while getopts "h3fxbkmrjlpeu:ny:ci:tz" opt; do
   case ${opt} in
     h) usage ;;
+    3) python_three="true" ;;
     f) skip_pre_commit_checks="true" ;;
     x) skip_bootstrap_clean="true" ;;
     b) skip_bootstrap="true" ;;
@@ -120,6 +124,16 @@ if [[ "${skip_bootstrap:-false}" == "false" ]]; then
   end_travis_section
 fi
 
+# NB: Ordering matters here. We (currently) always bootstrap a Python 2 pex.
+if [[ "${python_three:-false}" == "true" ]]; then
+  # The 3.4 end of this constraint is necessary to jive with the travis ubuntu trusty image.
+  banner "Setting interpreter constraints for 3!"
+  export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS='["CPython>=3.4,<4"]'
+  # FIXME: Clear interpreters, otherwise this constraint does not end up applying due to a cache
+  # bug between the `./pants binary` and further runs.
+  ./pants.pex clean-all
+fi
+
 if [[ "${skip_sanity_checks:-false}" == "false" ]]; then
   start_travis_section "SanityCheck" "Sanity checking bootstrapped pants and repo BUILD files"
   sanity_tests=(

pants-plugins/tests/python/internal_backend_test/sitegen/test_sitegen.py

@@ -5,9 +5,11 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import json
+import sys
 import unittest
 
 import bs4
+import pytest
 from future.utils import PY3
 
 from internal_backend.sitegen.tasks import sitegen
@@ -191,6 +193,7 @@ def test_transforms_not_discard_page_tocs(self):
     self.assertIn('DEPTH=1 LINK=one TEXT=Section One', rendered)
     self.assertIn('DEPTH=1 LINK=two TEXT=Section Two', rendered)
 
+  @pytest.mark.skipif(sys.version_info >= (3,0), reason="TODO: See #6062.")
   def test_site_toc(self):
     # Our "site" has a simple outline.
     # Do we get the correct info from that to generate

src/python/pants/engine/native.py

@@ -19,7 +19,7 @@
 
 from pants.engine.selectors import Get, constraint_for
 from pants.util.contextutil import temporary_dir
-from pants.util.dirutil import safe_mkdir, safe_mkdtemp
+from pants.util.dirutil import read_file, safe_mkdir, safe_mkdtemp
 from pants.util.memo import memoized_property
 from pants.util.objects import datatype
 
@@ -275,6 +275,50 @@
 }
 '''
 
+# NB: This is a "patch" applied to CFFI's generated sources to remove the ifdefs that would
+# usually cause only one of the two module definition functions to be defined. Instead, we define
+# both. Since `patch` is not available in all relevant environments (notably, many docker images),
+# this is accomplished using string replacement. To (re)-generate this patch, fiddle with the
+# unmodified output of `ffibuilder.emit_c_code`.
+CFFI_C_PATCH_BEFORE = '''
+#  ifdef _MSC_VER
+     PyMODINIT_FUNC
+#  if PY_MAJOR_VERSION >= 3
+     PyInit_native_engine(void) { return NULL; }
+#  else
+     initnative_engine(void) { }
+#  endif
+#  endif
+#elif PY_MAJOR_VERSION >= 3
+PyMODINIT_FUNC
+PyInit_native_engine(void)
+{
+  return _cffi_init("native_engine", 0x2601, &_cffi_type_context);
+}
+#else
+PyMODINIT_FUNC
+initnative_engine(void)
+{
+  _cffi_init("native_engine", 0x2601, &_cffi_type_context);
+}
+#endif
+'''
+CFFI_C_PATCH_AFTER = '''
+#endif
+
+PyObject* // PyMODINIT_FUNC for PY3
+wrapped_PyInit_native_engine(void)
+{
+  return _cffi_init("native_engine", 0x2601, &_cffi_type_context);
+}
+
+void // PyMODINIT_FUNC for PY2
+wrapped_initnative_engine(void)
+{
+  _cffi_init("native_engine", 0x2601, &_cffi_type_context);
+}
+'''
+
 
 def get_build_cflags():
   """Synthesize a CFLAGS env var from the current python env for building of C modules."""
@@ -314,14 +358,15 @@ def bootstrap_c_source(output_dir, module_name=NATIVE_ENGINE_MODULE):
     # (it kept the binary working) but inconvenient (it was relying on unspecified behavior, it meant
     # our binaries couldn't be stripped which inflated them by 2~3x, and it reduced the amount of LTO
     # we could use, which led to unmeasured performance hits).
-    def rename_symbol_in_file(f):
-      with open(f, 'r') as fh:
-        for line in fh:
-          if line.startswith('init{}'.format(module_name)) or line.startswith('PyInit_{}'.format(module_name)):
-            yield 'wrapped_' + line
-          else:
-            yield line
-    file_content = ''.join(rename_symbol_in_file(temp_c_file))
+    #
+    # We additionally remove the ifdefs that apply conditional `init` logic for Py2 vs Py3, in order
+    # to define a module that is loadable by either 2 or 3.
+    # TODO: Because PyPy uses the same `init` function name regardless of the python version, this
+    # trick does not work there: we leave its conditional in place.
+    file_content = read_file(temp_c_file).decode('utf-8')
+    if CFFI_C_PATCH_BEFORE not in file_content:
+      raise Exception('The patch for the CFFI generated code will not apply cleanly.')
+    file_content = file_content.replace(CFFI_C_PATCH_BEFORE, CFFI_C_PATCH_AFTER)
 
   _replace_file(c_file, file_content)
 

src/rust/engine/src/cffi_externs.rs

@@ -1,17 +1,25 @@
-// This file creates the unmangled symbol initnative_engine which cffi will use as the entry point
-// to this shared library.
+// This file creates the unmangled symbols initnative_engine and PyInit_native_engine, which cffi
+// will use as the entry point to this shared library in python 2 and python 3, respectively.
 //
-// It calls the extern'd wrapped_initnative_engine (generated in
+// It calls the extern'd wrapped_initnative_engine and wrapped_PyInit_native_engine (generated in
 // src/rust/engine/src/cffi/native_engine.c by build-support/native-engine/bootstrap_cffi.py).
 // This is a bit awkward and fiddly, but necessary because rust doesn't currently have a way to
 // re-export symbols from C libraries, other than this.
 // See https://github.com/rust-lang/rust/issues/36342
 
+use std::os::raw;
+
 extern "C" {
   pub fn wrapped_initnative_engine();
+  pub fn wrapped_PyInit_native_engine() -> *mut raw::c_void;
 }
 
 #[no_mangle]
 pub extern "C" fn initnative_engine() {
   unsafe { wrapped_initnative_engine() }
 }
+
+#[no_mangle]
+pub unsafe extern "C" fn PyInit_native_engine() -> *mut raw::c_void {
+  wrapped_PyInit_native_engine()
+}

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

@@ -17,6 +17,7 @@
 from pants.build_graph.files import Files
 from pants.build_graph.resources import Resources
 from pants.source.source_root import SourceRootConfig
+from pants.util.contextutil import temporary_dir
 from pants_test.task_test_base import TaskTestBase
 
 
@@ -106,19 +107,22 @@ def test_gather_files(self):
     self._assert_content_not_in_pex(pex, self.resources)
 
   def _gather_sources(self, target_roots):
-    context = self.context(target_roots=target_roots, for_subsystems=[PythonSetup, PythonRepos])
-
-    # We must get an interpreter via the cache, instead of using PythonInterpreter.get() directly,
-    # to ensure that the interpreter has setuptools and wheel support.
-    interpreter = PythonInterpreter.get()
-    interpreter_cache = PythonInterpreterCache(PythonSetup.global_instance(),
-                                               PythonRepos.global_instance(),
-                                               logger=context.log.debug)
-    interpreters = interpreter_cache.setup(paths=[os.path.dirname(interpreter.binary)],
-                                           filters=[str(interpreter.identity.requirement)])
-    context.products.get_data(PythonInterpreter, lambda: interpreters[0])
-
-    task = self.create_task(context)
-    task.execute()
-
-    return context.products.get_data(GatherSources.PYTHON_SOURCES)
+    with temporary_dir() as cache_dir:
+      context = self.context(target_roots=target_roots,
+                             for_subsystems=[PythonSetup, PythonRepos],
+                             options={PythonSetup.options_scope: {'interpreter_cache_dir': cache_dir}})
+
+      # We must get an interpreter via the cache, instead of using PythonInterpreter.get() directly,
+      # to ensure that the interpreter has setuptools and wheel support.
+      interpreter = PythonInterpreter.get()
+      interpreter_cache = PythonInterpreterCache(PythonSetup.global_instance(),
+                                                PythonRepos.global_instance(),
+                                                logger=context.log.debug)
+      interpreters = interpreter_cache.setup(paths=[os.path.dirname(interpreter.binary)],
+                                            filters=[str(interpreter.identity.requirement)])
+      context.products.get_data(PythonInterpreter, lambda: interpreters[0])
+
+      task = self.create_task(context)
+      task.execute()
+
+      return context.products.get_data(GatherSources.PYTHON_SOURCES)

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

@@ -16,7 +16,7 @@
 from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
 from pants.backend.python.tasks.resolve_requirements import ResolveRequirements
 from pants.base.build_environment import get_buildroot
-from pants.util.contextutil import temporary_file
+from pants.util.contextutil import temporary_dir, temporary_file
 from pants.util.process_handler import subprocess
 from pants_test.task_test_base import TaskTestBase
 
@@ -114,23 +114,26 @@ def _fake_target(self, spec, requirement_strs):
                             requirements=requirements)
 
   def _resolve_requirements(self, target_roots, options=None):
-    context = self.context(target_roots=target_roots, options=options,
-                           for_subsystems=[PythonSetup, PythonRepos])
-
-    # We must get an interpreter via the cache, instead of using PythonInterpreter.get() directly,
-    # to ensure that the interpreter has setuptools and wheel support.
-    interpreter = PythonInterpreter.get()
-    interpreter_cache = PythonInterpreterCache(PythonSetup.global_instance(),
-                                               PythonRepos.global_instance(),
-                                               logger=context.log.debug)
-    interpreters = interpreter_cache.setup(paths=[os.path.dirname(interpreter.binary)],
-                                           filters=[str(interpreter.identity.requirement)])
-    context.products.get_data(PythonInterpreter, lambda: interpreters[0])
-
-    task = self.create_task(context)
-    task.execute()
-
-    return context.products.get_data(ResolveRequirements.REQUIREMENTS_PEX)
+    with temporary_dir() as cache_dir:
+      options = options or {}
+      options.setdefault(PythonSetup.options_scope, {})['interpreter_cache_dir'] = cache_dir
+      context = self.context(target_roots=target_roots, options=options,
+                             for_subsystems=[PythonSetup, PythonRepos])
+
+      # We must get an interpreter via the cache, instead of using PythonInterpreter.get() directly,
+      # to ensure that the interpreter has setuptools and wheel support.
+      interpreter = PythonInterpreter.get()
+      interpreter_cache = PythonInterpreterCache(PythonSetup.global_instance(),
+                                                PythonRepos.global_instance(),
+                                                logger=context.log.debug)
+      interpreters = interpreter_cache.setup(paths=[os.path.dirname(interpreter.binary)],
+                                            filters=[str(interpreter.identity.requirement)])
+      context.products.get_data(PythonInterpreter, lambda: interpreters[0])
+
+      task = self.create_task(context)
+      task.execute()
+
+      return context.products.get_data(ResolveRequirements.REQUIREMENTS_PEX)
 
   def _exercise_module(self, pex, expected_module):
     with temporary_file() as f: