Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always use the global interpreter for PythonExecutionTaskBase. #7801

Conversation

Projects
None yet
2 participants
@stuhood
Copy link
Member

commented May 24, 2019

Problem

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

@skip_unless_python27_and_python3_present
def test_pants_run_interpreter_selection_with_pexrc(self):
py27_path, py3_path = python_interpreter_path(PY_27), python_interpreter_path(PY_3)
with setup_pexrc_with_pex_python_path([py27_path, py3_path]):
with temporary_dir() as interpreters_cache:
pants_ini_config = {'python-setup': {'interpreter_cache_dir': interpreters_cache}}
pants_run_27 = self.run_pants(
command=['run', '{}:main_py2'.format(os.path.join(self.testproject,
'python_3_selection_testing'))],
config=pants_ini_config
)
self.assert_success(pants_run_27)
self.assertIn(py27_path, 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)
@skip_unless_python27_and_python3_present
def test_pants_binary_interpreter_selection_with_pexrc(self):
py27_path, py3_path = python_interpreter_path(PY_27), python_interpreter_path(PY_3)
with setup_pexrc_with_pex_python_path([py27_path, py3_path]):
with temporary_dir() as interpreters_cache:
pants_ini_config = {'python-setup': {'interpreter_cache_dir': interpreters_cache}}
pants_run_27 = self.run_pants(
command=['binary', '{}:main_py2'.format(os.path.join(self.testproject,
'python_3_selection_testing'))],
config=pants_ini_config
)
self.assert_success(pants_run_27)
pants_run_3 = self.run_pants(
command=['binary', '{}:main_py3'.format(os.path.join(self.testproject,
'python_3_selection_testing'))],
config=pants_ini_config
)
self.assert_success(pants_run_3)
# Ensure proper interpreter constraints were passed to built pexes.
py2_pex = os.path.join(os.getcwd(), 'dist', 'main_py2.pex')
py3_pex = os.path.join(os.getcwd(), 'dist', 'main_py3.pex')
py2_info = PexInfo.from_pex(py2_pex)
py3_info = PexInfo.from_pex(py3_pex)
self.assertIn('CPython>2.7.6,<3', py2_info.interpreter_constraints)
self.assertIn('CPython>3', py3_info.interpreter_constraints)
# Cleanup created pexes.
os.remove(py2_pex)
os.remove(py3_pex)
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).

Solution

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

Result

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

@stuhood stuhood requested review from cosmicexplorer, jsirois and Eric-Arellano and removed request for cosmicexplorer and jsirois May 24, 2019

@stuhood

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

This is now reviewable.

@stuhood stuhood added this to the 1.16.x milestone May 24, 2019

@stuhood stuhood requested a review from jsirois May 24, 2019

@stuhood

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

The test failure is related to #7640, in the sense that we've selected an interpreter in the SelectInterpreter task that is not visible to pex later on in the PythonRun task. I've incorporated the same fix from #7563 here in the topmost commit.

Take a step toward #7640 in order to fix CI (where the selected inter…
…preter from build time is not locatable at runtime).

@stuhood stuhood force-pushed the twitter:stuhood/always-use-the-global-interpreter-for-python-execution-task-base branch from 0dc80ac to 1b1a724 May 24, 2019

@stuhood stuhood merged commit 762e0b6 into pantsbuild:master May 25, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@stuhood stuhood deleted the twitter:stuhood/always-use-the-global-interpreter-for-python-execution-task-base branch May 25, 2019

stuhood added a commit that referenced this pull request May 25, 2019

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.