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

Add runtime check for valid Python interpreter #7365

Merged
merged 13 commits into from Mar 14, 2019
@@ -7,6 +7,7 @@
import importlib
import locale
import os
import sys
import warnings
from builtins import object
from textwrap import dedent
@@ -23,6 +24,14 @@ class PantsLoader(object):
class InvalidLocaleError(Exception):
"""Raised when a valid locale can't be found."""

class InvalidInterpreter(Exception):
"""Raised when trying to run Pants with an unsupported Python version."""

@staticmethod
def _is_supported_interpreter(major_version, minor_version):
return (major_version == 2 and minor_version == 7) \
or (major_version == 3 and minor_version >= 6)

@staticmethod
def setup_warnings():
# We want to present warnings to the user, set this up before importing any of our own code,
@@ -49,7 +58,7 @@ def ensure_locale(cls):
# libraries called by Pants may fail with more obscure errors.
encoding = locale.getpreferredencoding()
if encoding.lower() != 'utf-8' and os.environ.get(cls.ENCODING_IGNORE_ENV_VAR, None) is None:
raise cls.InvalidLocaleError(dedent("""\
raise cls.InvalidLocaleError(dedent("""
This conversation was marked as resolved by Eric-Arellano

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 12, 2019

Author Contributor

I removed the \ so that the error prints on a new line. Otherwise the formatting looks bad when printed.

This is what the two error messages in this file look like now, rather than the first line being on the same line as __main__.InvalidInterpreter: and messing up the indent:

__main__.InvalidInterpreter:
You are trying to run Pants with an unsupported Python interpreter. Pants
requires a Python 2.7 or a Python 3.6+ interpreter to be discoverable on
your PATH to run.

If you still get this error after ensuring at least one of these interpreters
is discoverable on your PATH, you may need to modify your build script
(e.g. `./pants`) to properly set up a virtual environment with the correct
interpreter. We recommend following our setup guide and using our setup script
as a starting point: https://www.pantsbuild.org/setup_repo.html.
Your system's preferred encoding is `{}`, but Pants requires `UTF-8`.
Specifically, Python's `locale.getpreferredencoding()` must resolve to `UTF-8`.
@@ -62,6 +71,23 @@ def ensure_locale(cls):
""".format(encoding, cls.ENCODING_IGNORE_ENV_VAR)
))

@classmethod
def ensure_valid_interpreter(cls):
"""Runtime check that user is using a supported Python version."""
py_major, py_minor = sys.version_info[0:2]
if not cls._is_supported_interpreter(py_major, py_minor):
raise cls.InvalidInterpreter(dedent("""
You are trying to run Pants with an unsupported Python interpreter. Pants
requires a Python 2.7 or a Python 3.6+ interpreter to be discoverable on
your PATH to run.
If you still get this error after ensuring at least one of these interpreters
is discoverable on your PATH, you may need to modify your build script
(e.g. `./pants`) to properly set up a virtual environment with the correct
interpreter. We recommend following our setup guide and using our setup script
as a starting point: https://www.pantsbuild.org/setup_repo.html.
"""))

@staticmethod
def determine_entrypoint(env_var, default):
return os.environ.pop(env_var, default)
@@ -78,6 +104,7 @@ def load_and_execute(entrypoint):
@classmethod
def run(cls):
cls.setup_warnings()
cls.ensure_valid_interpreter()
cls.ensure_locale()
entrypoint = cls.determine_entrypoint(cls.ENTRYPOINT_ENV_VAR, cls.DEFAULT_ENTRYPOINT)
cls.load_and_execute(entrypoint)
@@ -10,9 +10,22 @@
from pants.util.process_handler import subprocess


PY_27 = '2.7'
PY_2 = '2'
PY_3 = '3'

PY_26 = '2.6'
PY_27 = '2.7'
PY_34 = '3.4'
PY_35 = '3.5'
PY_36 = '3.6'
PY_37 = '3.7'
PY_38 = '3.8'


def find_all_pythons_present():
"""Return set of all Python versions present on the system."""
likely_versions_present = {PY_26, PY_27, PY_34, PY_35, PY_36, PY_37, PY_38}
return {version for version in likely_versions_present if has_python_version(version)}


def has_python_version(version):
@@ -36,12 +49,22 @@ def python_interpreter_path(version):
command = ['python{}'.format(version), '-c', 'import sys; print(sys.executable)']
py_path = subprocess.check_output(command).decode('utf-8').strip()
return os.path.realpath(py_path)
except subprocess.CalledProcessError:
except (subprocess.CalledProcessError, FileNotFoundError):
return None


def skip_unless_any_pythons_present(*versions):
"""A decorator that only runs the decorated test method if any of the specified pythons are present.
:param string *versions: Python version strings, such as 2.7, 3.
"""
if any(v for v in versions if has_python_version(v)):
return skipIf(False, 'At least one of the expected python versions found.')
return skipIf(True, 'Could not find at least one of the required pythons from {} on the system. Skipping.'.format(versions))


def skip_unless_pythons(*versions):
This conversation was marked as resolved by Eric-Arellano

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 12, 2019

Author Contributor

Note I really want to rename all the functions in this file to skip_unless_*_present. All the docstrings already use the word "present", and I think adding it to the name would make it much clearer in calling files what the decorator is doing. Avoided doing this here for scope creep.

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 12, 2019

Author Contributor

Done in #7366!

"""A decorator that only runs the decorated test method if the specified pythons are present.
"""A decorator that only runs the decorated test method if all of the specified pythons are present.
:param string *versions: Python version strings, such as 2.7, 3.
"""
@@ -5,6 +5,7 @@ python_tests(
dependencies=[
'src/python/pants/bin',
'tests/python/pants_test:int-test',
'tests/python/pants_test/backend/python:interpreter_selection_utils',
],
tags = {'integration'},
)
@@ -5,10 +5,14 @@
from __future__ import absolute_import, division, print_function, unicode_literals

from pants.bin.pants_loader import PantsLoader
from pants_test.backend.python.interpreter_selection_utils import (PY_26, PY_34, PY_35,
find_all_pythons_present,
skip_unless_any_pythons_present)
from pants_test.pants_run_integration_test import PantsRunIntegrationTest


class LoaderIntegrationTest(PantsRunIntegrationTest):

def test_invalid_locale(self):
bypass_env = PantsLoader.ENCODING_IGNORE_ENV_VAR
pants_run = self.run_pants(command=['help'], extra_env={'LC_ALL': 'iNvALiD-lOcALe'})
@@ -20,6 +24,16 @@ def test_invalid_locale(self):
bypass_env: '1'})
self.assert_success(pants_run)

@skip_unless_any_pythons_present(PY_26, PY_34, PY_35)
def test_invalid_interpreter(self):
This conversation was marked as resolved by Eric-Arellano

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 12, 2019

Author Contributor

Bummer. I realized this test won't work in CI because we set RUN_PANTS_FROM_PEX=1, so the line self.run_pants(command=['help'], extra_env={'PY': "python{}".format(invalid_versions_present[0])}) will run the PEX with the interpreter being used.

Even if we could figure out how to get the PEX to be ran with Py26, Py34, or Py35, we wouldn't want to because it would start bootstrapping a new venv folder and slow down CI dramatically.

Thoughts on ways around this, or should I kill the test?

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 12, 2019

Author Contributor

One potential workaround to this test failing in CI:

  • Patch PantsLoader._is_supported_interpreter() to always return True, meaning the exception will raise no matter which interpreter we're using. This would allow us to verify we do in fact we do in fact fail Pants and print a nice message.
  • We would not be testing PantsLoader._is_supported_interpreter(), though. Will write a unit test for this.

Will submit an update soon - let me know if you can think of a better design.

invalid_versions_present = [version for version in find_all_pythons_present()
if not PantsLoader._is_supported_interpreter(
*[int(version_component) for version_component in version.split(".")]
)]
pants_run = self.run_pants(command=['help'], extra_env={'PY': "python{}".format(invalid_versions_present[0])})
self.assert_failure(pants_run)
self.assertIn('unsupported Python interpreter', pants_run.stderr_data)

def test_alternate_entrypoint(self):
pants_run = self.run_pants(
command=['help'],
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.