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,24 @@ 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]
current_version = '.'.join(map(str, sys.version_info[0:2]))
if not PantsLoader.is_supported_interpreter(py_major, py_minor):
This conversation was marked as resolved by Eric-Arellano

This comment has been minimized.

Copy link
@stuhood

stuhood Mar 12, 2019

Member

I'm not really sure that we should do this at all. If this ever triggered, it would be because of a bug in the setup, script (which we should fix in the setup script), or due to someone's internal scripts choosing some other Python version (which, imo, would be on them).

But if we do do this, we should make sure that there is a way for someone to set a variable (or better yet, a pants.ini setting) to bypass the check.

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 12, 2019

Author Contributor

The most probable scenario is they use our updated setup script properly, and then run PYTHON=python3.5 ./pants.

This is totally valid in the setup repo to do and should always be valid. We don't want the setup script to pin which versions Pants supports, especially because it's so difficult to get people to consume updates to the script since they curl it and often modify after.

It is also totally valid to set in pants.ini the value pants_engine_python_version: 3.5. The checker doesn't belong there, because it's easy to circumvent unintentionally (see #7363 (comment)).

Pants might work with 3.5, but it also might not and could be buggy. We should at least warn users they're tredding dangerous waters. If they choose to use the bypass and try it out, totally fine with us. Better UX than randomly hitting SyntaxErrors and bugs from using an unsupported version, and them having no idea why.

I'm totally fine with adding an env var bypass. Good idea!

raise cls.InvalidInterpreter(dedent("""
You are trying to run Pants with Python {}, which is unsupported.
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.
""".format(current_version)))

@staticmethod
def determine_entrypoint(env_var, default):
return os.environ.pop(env_var, default)
@@ -78,6 +105,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)
@@ -2,6 +2,16 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_tests(
sources='test_loader.py',
dependencies=[
'src/python/pants/bin',
'3rdparty/python:future',
],
)

python_tests(
name='integration',
sources='test_loader_integration.py',
dependencies=[
'src/python/pants/bin',
'tests/python/pants_test:int-test',
@@ -0,0 +1,28 @@
# coding=utf-8
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import sys
from builtins import map, str
from unittest import TestCase, mock

from pants.bin.pants_loader import PantsLoader


class LoaderTest(TestCase):

def test_is_supported_interpreter(self):
unsupported_versions = {(2, 6), (3, 3), (3, 4), (3, 5), (4, 0)}
supported_versions = {(2, 7), (3, 6), (3, 7), (3, 8)}
self.assertTrue(all(not PantsLoader.is_supported_interpreter(major, minor) for major, minor in unsupported_versions))
self.assertTrue(all(PantsLoader.is_supported_interpreter(major, minor) for major, minor in supported_versions))

def test_ensure_valid_interpreter(self):
current_interpreter_version = '.'.join(map(str, sys.version_info[0:2]))
with mock.patch.object(PantsLoader, 'is_supported_interpreter', return_value=False):
with self.assertRaises(PantsLoader.InvalidInterpreter) as e:
PantsLoader.ensure_valid_interpreter()
self.assertIn("unsupported Python interpreter", str(e))
self.assertIn(current_interpreter_version, str(e))
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.