PEX behaving differently on Python 3 for python_eval.py

[Eric-Arellano]

Problem

Several of the tests are supposed to raise errors, but aren’t, such as https://github.com/pantsbuild/pants/blob/master/contrib/python/tests/python/pants_test/contrib/python/checks/tasks/test_python_eval.py#L180

We isolated it to this line https://github.com/pantsbuild/pants/blob/master/contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py#L168. pex.run() is returning status code 0 in Py3, when it should be 1.

Steps to reproduce

1. ./pants clean-all
2. ./pants --python-setup-interpreter-constraints='["CPython>=3.5"]' --no-test-pytest-timeouts test contrib/python/tests/python/pants_test/contrib/python/checks/tasks:

Things not an issue

Stdout

We override stdout in pex.run(), which I thought would cause issues because stdout takes bytes in Py2 but unicode in Py3. No longer passing the parameters doesn’t work though.

Interpreter mismatch

the interpreter arg to PEX() and PEXBuilder() is for 2.7, but for pex_info.build_properties it’s 3

I thought this mismatch would cause issues? But I changed python_setup.py to require py3, which makes interpreter be py3, and it still fails.

Pex 1.4.5

Using this version vs. others isn't the breaking point.

Issue is likely how PEX is built

@wisechengyi found that after the PEX is built, it doesn't matter which interpreter you use. If the PEX was created with Python 2, it will always work. If it was created with Python 3, it will never work.

To reproduce, apply this diff

diff --git a/contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py b/contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py
index 393df7a..7a19dc4 100644
--- a/contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py
+++ b/contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py
@@ -158,7 +158,8 @@ class PythonEval(LintTaskMixin, ResolveRequirementsTaskBase):
           pex_info.pex_path = ':'.join(pex.path() for pex in (reqs_pex, srcs_pex) if pex)
           builder = PEXBuilder(safe_path, interpreter, pex_info=pex_info)
           builder.freeze()
-
+      import pdb
+      pdb.set_trace()
       pex = PEX(exec_pex_path, interpreter)
 
       with self.context.new_workunit(name='eval',

and get the exec_pex_path to get the chroot, then run the PEX with your system's Python like python2.7 <exec_pex_path>.

[jsirois]

Noted I started digging on this but I'm blocked here for a bit on hitting https://setuptools.readthedocs.io/en/latest/history.html#v38-4-1 for the cryptography distribution. I'll see if I can find time to fix that and circle back.

[stuhood]

@jsirois : You can avoid that one by downgrading to python 3.6, iirc.

[jsirois]

Sure, but that's not something we can in-general expect of folks. I'll try to fix things between pants & pex wrt setuptools deps so this works.

[jsirois]

Filed #6363 for the python 3.7 issue and I'll change us from <4 to <3.7 until that's fixed.

[jsirois]

@Eric-Arellano just checking in that this is still an issue. I could not repro locally yesterday.

[Eric-Arellano]

@jsirois Yes I think so. You'll have to apply this diff:

diff --git a/contrib/python/tests/python/pants_test/contrib/python/checks/tasks/test_python_eval.py b/contrib/python/tests/python/pants_test/contrib/python/checks/tasks/test_python_eval.py
index 3df47257c..5fa18254d 100644
--- a/contrib/python/tests/python/pants_test/contrib/python/checks/tasks/test_python_eval.py
+++ b/contrib/python/tests/python/pants_test/contrib/python/checks/tasks/test_python_eval.py
@@ -15,8 +15,6 @@ from pants_test.backend.python.tasks.python_task_test_base import PythonTaskTest
 from pants.contrib.python.checks.tasks.python_eval import PythonEval


-# TODO(python3port): https://github.com/pantsbuild/pants/issues/6354. Fix before switching fully to Py3.
-@pytest.mark.skipif(PY3, reason='PEX issue when using Python 3. https://github.com/pantsbuild/pants/issues/6354')
 class PythonEvalTest(PythonTaskTestBase):

   @classmethod

We're skipping the tests if PY3 to allow CI to run everything else.

[jsirois]

This was a failure to decode pkgutil.get_data as utf-8 bytes. We have more, but I left those alone to stick to this issue:

$ git grep "pkgutil\.get_data" | grep -ivE "utf-8|utf8"
src/python/pants/backend/project_info/tasks/idea_plugin_gen.py:      Generator(pkgutil.get_data(__name__, self.project_template), project=configured_project))
src/python/pants/backend/project_info/tasks/idea_plugin_gen.py:      Generator(pkgutil.get_data(__name__, self.workspace_template), workspace=configured_workspace))
src/python/pants/base/mustache.py:    template_text = pkgutil.get_data(self._package_name, path)
src/python/pants/java/distribution/distribution.py:          fp.write(pkgutil.get_data(__name__, 'SystemProperties.class'))
src/python/pants/reporting/reporting_server.py:      content = pkgutil.get_data(__name__, os.path.join('assets', relpath))

From that list, the SystemProperties.class load can be excluded - that is in fact binary.