Drop Python 2 support (kivy#1918)

inclement · web-flow · commit 40b1164e7447 · 2019-08-04T17:42:41.000+01:00
* Added build failure if run under Python 2

* Made Python 2 check more robust

* Style fixes

* Changed minimum python3 minor version to 3.4

* Added a deprecation warning log to the Python 2 recipe during build

* Updated tox.ini to run py2 tests only on android module

* Added tests for Python version checking

* Added test for Python 2 running long enough to exit nicely

* Pep8 and code style improvements

* Added descriptions for flake8 exceptions

* Hardcoded python2 tests in tox.ini

* Removed E127 and E129 global disable
diff --git a/pythonforandroid/bdistapk.py b/pythonforandroid/bdistapk.py
@@ -75,7 +75,7 @@ def finalize_options(self):
     def run(self):
         self.prepare_build_dir()
 
-        from pythonforandroid.toolchain import main
+        from pythonforandroid.entrypoints import main
         sys.argv[1] = 'apk'
         main()
 
diff --git a/pythonforandroid/entrypoints.py b/pythonforandroid/entrypoints.py
@@ -0,0 +1,20 @@
+from pythonforandroid.recommendations import check_python_version
+from pythonforandroid.util import BuildInterruptingException, handle_build_exception
+
+
+def main():
+    """
+    Main entrypoint for running python-for-android as a script.
+    """
+
+    try:
+        # Check the Python version before importing anything heavier than
+        # the util functions.  This lets us provide a nice message about
+        # incompatibility rather than having the interpreter crash if it
+        # reaches unsupported syntax from a newer Python version.
+        check_python_version()
+
+        from pythonforandroid.toolchain import ToolchainCL
+        ToolchainCL()
+    except BuildInterruptingException as exc:
+        handle_build_exception(exc)
diff --git a/pythonforandroid/logger.py b/pythonforandroid/logger.py
@@ -6,6 +6,8 @@
 from math import log10
 from collections import defaultdict
 from colorama import Style as Colo_Style, Fore as Colo_Fore
+
+# six import left for Python 2 compatibility during initial Python version check
 import six
 
 # This codecs change fixes a bug with log output, but crashes under python3
diff --git a/pythonforandroid/recipes/python2/__init__.py b/pythonforandroid/recipes/python2/__init__.py
@@ -1,7 +1,7 @@
 from os.path import join, exists
 from pythonforandroid.recipe import Recipe
 from pythonforandroid.python import GuestPythonRecipe
-from pythonforandroid.logger import shprint
+from pythonforandroid.logger import shprint, warning
 import sh
 
 
@@ -57,6 +57,11 @@ def prebuild_arch(self, arch):
             self.apply_patch(join('patches', 'enable-openssl.patch'), arch.arch)
             shprint(sh.touch, patch_mark)
 
+    def build_arch(self, arch):
+        warning('DEPRECATION: Support for the Python 2 recipe will be '
+                'removed in 2020, please upgrade to Python 3.')
+        super().build_arch(arch)
+
     def set_libs_flags(self, env, arch):
         env = super(Python2Recipe, self).set_libs_flags(env, arch)
         if 'libffi' in self.ctx.recipe_build_order:
diff --git a/pythonforandroid/recommendations.py b/pythonforandroid/recommendations.py
@@ -1,7 +1,9 @@
 """Simple functions for checking dependency versions."""
 
+import sys
 from distutils.version import LooseVersion
 from os.path import join
+
 from pythonforandroid.logger import info, warning
 from pythonforandroid.util import BuildInterruptingException
 
@@ -182,3 +184,37 @@ def check_ndk_api(ndk_api, android_api):
 
     if ndk_api < MIN_NDK_API:
         warning(OLD_NDK_API_MESSAGE)
+
+
+MIN_PYTHON_MAJOR_VERSION = 3
+MIN_PYTHON_MINOR_VERSION = 4
+MIN_PYTHON_VERSION = LooseVersion('{major}.{minor}'.format(major=MIN_PYTHON_MAJOR_VERSION,
+                                                           minor=MIN_PYTHON_MINOR_VERSION))
+PY2_ERROR_TEXT = (
+    'python-for-android no longer supports running under Python 2. Either upgrade to '
+    'Python {min_version} or higher (recommended), or revert to python-for-android 2019.07.08. '
+    'Note that you *can* still target Python 2 on Android by including python2 in your '
+    'requirements.').format(
+        min_version=MIN_PYTHON_VERSION)
+
+PY_VERSION_ERROR_TEXT = (
+    'Your Python version {user_major}.{user_minor} is not supported by python-for-android, '
+    'please upgrade to {min_version} or higher.'
+    ).format(
+        user_major=sys.version_info.major,
+        user_minor=sys.version_info.minor,
+        min_version=MIN_PYTHON_VERSION)
+
+
+def check_python_version():
+    # Python 2 special cased because it's a major transition. In the
+    # future the major or minor versions can increment more quietly.
+    if sys.version_info.major == 2:
+        raise BuildInterruptingException(PY2_ERROR_TEXT)
+
+    if (
+        sys.version_info.major < MIN_PYTHON_MAJOR_VERSION or
+        sys.version_info.minor < MIN_PYTHON_MINOR_VERSION
+    ):
+
+        raise BuildInterruptingException(PY_VERSION_ERROR_TEXT)
diff --git a/pythonforandroid/toolchain.py b/pythonforandroid/toolchain.py
@@ -12,7 +12,8 @@
 from pythonforandroid.pythonpackage import get_dep_names_of_package
 from pythonforandroid.recommendations import (
     RECOMMENDED_NDK_API, RECOMMENDED_TARGET_API)
-from pythonforandroid.util import BuildInterruptingException, handle_build_exception
+from pythonforandroid.util import BuildInterruptingException
+from pythonforandroid.entrypoints import main
 
 
 def check_python_dependencies():
@@ -568,6 +569,7 @@ def add_parser(subparsers, *args, **kwargs):
 
         args, unknown = parser.parse_known_args(sys.argv[1:])
         args.unknown_args = unknown
+
         if hasattr(args, "private") and args.private is not None:
             # Pass this value on to the internal bootstrap build.py:
             args.unknown_args += ["--private", args.private]
@@ -1187,12 +1189,5 @@ def build_status(self, _args):
                 print(recipe_str)
 
 
-def main():
-    try:
-        ToolchainCL()
-    except BuildInterruptingException as exc:
-        handle_build_exception(exc)
-
-
 if __name__ == "__main__":
     main()
diff --git a/pythonforandroid/util.py b/pythonforandroid/util.py
@@ -3,18 +3,17 @@
 from os import getcwd, chdir, makedirs, walk, uname
 import sh
 import shutil
-import sys
 from fnmatch import fnmatch
 from tempfile import mkdtemp
-try:
+
+# This Python version workaround left for compatibility during initial version check
+try:  # Python 3
     from urllib.request import FancyURLopener
-except ImportError:
+except ImportError:  # Python 2
     from urllib import FancyURLopener
 
 from pythonforandroid.logger import (logger, Err_Fore, error, info)
 
-IS_PY3 = sys.version_info[0] >= 3
-
 
 class WgetDownloader(FancyURLopener):
     version = ('Wget/1.17.1')
diff --git a/setup.py b/setup.py
@@ -17,6 +17,7 @@
 data_files = []
 
 
+
 # must be a single statement since buildozer is currently parsing it, refs:
 # https://github.com/kivy/buildozer/issues/722
 install_reqs = [
@@ -94,15 +95,15 @@ def recursively_include(results, directory, patterns):
       install_requires=install_reqs,
       entry_points={
           'console_scripts': [
-              'python-for-android = pythonforandroid.toolchain:main',
-              'p4a = pythonforandroid.toolchain:main',
+              'python-for-android = pythonforandroid.entrypoints:main',
+              'p4a = pythonforandroid.entrypoints:main',
               ],
           'distutils.commands': [
               'apk = pythonforandroid.bdistapk:BdistAPK',
               ],
           },
       classifiers = [
-          'Development Status :: 4 - Beta',
+          'Development Status :: 5 - Production/Stable',
           'Intended Audience :: Developers',
           'License :: OSI Approved :: MIT License',
           'Operating System :: Microsoft :: Windows',
@@ -111,7 +112,6 @@ def recursively_include(results, directory, patterns):
           'Operating System :: MacOS :: MacOS X',
           'Operating System :: Android',
           'Programming Language :: C',
-          'Programming Language :: Python :: 2',
           'Programming Language :: Python :: 3',
           'Topic :: Software Development',
           'Topic :: Utilities',
diff --git a/tests/test_androidmodule_ctypes_finder.py b/tests/test_androidmodule_ctypes_finder.py
@@ -1,6 +1,12 @@
 
-import mock
-from mock import MagicMock
+# This test is still expected to support Python 2, as it tests
+# on-Android functionality that we still maintain
+try:  # Python 3+
+    from unittest import mock
+    from unittest.mock import MagicMock
+except ImportError:  # Python 2
+    import mock
+    from mock import MagicMock
 import os
 import shutil
 import sys
diff --git a/tests/test_entrypoints_python2.py b/tests/test_entrypoints_python2.py
@@ -0,0 +1,38 @@
+
+# This test is a special case that we expect to run under Python 2, so
+# include the necessary compatibility imports:
+try:  # Python 3
+    from unittest import mock
+except ImportError:  # Python 2
+    import mock
+
+from pythonforandroid.recommendations import PY2_ERROR_TEXT
+from pythonforandroid import entrypoints
+
+
+def test_main_python2():
+    """Test that running under Python 2 leads to the build failing, while
+    running under a suitable version works fine.
+
+    Note that this test must be run *using* Python 2 to truly test
+    that p4a can reach the Python version check before importing some
+    Python-3-only syntax and crashing.
+    """
+
+    # Under Python 2, we should get a normal control flow exception
+    # that is handled properly, not any other type of crash
+    handle_exception_path = 'pythonforandroid.entrypoints.handle_build_exception'
+    with mock.patch('sys.version_info') as fake_version_info, \
+         mock.patch(handle_exception_path) as handle_build_exception:  # noqa: E127
+
+        fake_version_info.major = 2
+        fake_version_info.minor = 7
+
+        def check_python2_exception(exc):
+            """Check that the exception message is Python 2 specific."""
+            assert exc.message == PY2_ERROR_TEXT
+        handle_build_exception.side_effect = check_python2_exception
+
+        entrypoints.main()
+
+    handle_build_exception.assert_called_once()
diff --git a/tests/test_recommendations.py b/tests/test_recommendations.py
@@ -2,17 +2,13 @@
 from os.path import join
 from sys import version as py_version
 
-try:
-    from unittest import mock
-except ImportError:
-    # `Python 2` or lower than `Python 3.3` does not
-    # have the `unittest.mock` module built-in
-    import mock
+from unittest import mock
 from pythonforandroid.recommendations import (
     check_ndk_api,
     check_ndk_version,
     check_target_api,
     read_ndk_version,
+    check_python_version,
     MAX_NDK_VERSION,
     RECOMMENDED_NDK_VERSION,
     RECOMMENDED_TARGET_API,
@@ -33,7 +29,12 @@
     OLD_NDK_API_MESSAGE,
     NEW_NDK_MESSAGE,
     OLD_API_MESSAGE,
+    MIN_PYTHON_MAJOR_VERSION,
+    MIN_PYTHON_MINOR_VERSION,
+    PY2_ERROR_TEXT,
+    PY_VERSION_ERROR_TEXT,
 )
+
 from pythonforandroid.util import BuildInterruptingException
 
 running_in_py2 = int(py_version[0]) < 3
@@ -202,3 +203,37 @@ def test_check_ndk_api_warning_old_ndk(self):
                 )
             ],
         )
+
+    def test_check_python_version(self):
+        """With any version info lower than the minimum, we should get a
+        BuildInterruptingException with an appropriate message.
+        """
+        with mock.patch('sys.version_info') as fake_version_info:
+
+            # Major version is Python 2 => exception
+            fake_version_info.major = MIN_PYTHON_MAJOR_VERSION - 1
+            fake_version_info.minor = MIN_PYTHON_MINOR_VERSION
+            with self.assertRaises(BuildInterruptingException) as context:
+                check_python_version()
+            assert context.exception.message == PY2_ERROR_TEXT
+
+            # Major version too low => exception
+            # Using a float valued major version just to test the logic and avoid
+            # clashing with the Python 2 check
+            fake_version_info.major = MIN_PYTHON_MAJOR_VERSION - 0.1
+            fake_version_info.minor = MIN_PYTHON_MINOR_VERSION
+            with self.assertRaises(BuildInterruptingException) as context:
+                check_python_version()
+            assert context.exception.message == PY_VERSION_ERROR_TEXT
+
+            # Minor version too low => exception
+            fake_version_info.major = MIN_PYTHON_MAJOR_VERSION
+            fake_version_info.minor = MIN_PYTHON_MINOR_VERSION - 1
+            with self.assertRaises(BuildInterruptingException) as context:
+                check_python_version()
+            assert context.exception.message == PY_VERSION_ERROR_TEXT
+
+            # Version high enough => nothing interesting happens
+            fake_version_info.major = MIN_PYTHON_MAJOR_VERSION
+            fake_version_info.minor = MIN_PYTHON_MINOR_VERSION
+            check_python_version()
diff --git a/tox.ini b/tox.ini
@@ -9,13 +9,19 @@ deps =
     virtualenv
     py3: coveralls
     backports.tempfile
-# makes it possible to override pytest args, e.g.
-# tox -- tests/test_graph.py
+# posargs will be replaced by the tox args, so you can override pytest
+# args e.g. `tox -- tests/test_graph.py`
 commands = pytest {posargs:tests/}
 passenv = TRAVIS TRAVIS_*
 setenv =
     PYTHONPATH={toxinidir}
 
+[testenv:py27]
+# Note that the set of tests is not posargs-configurable here: we only
+# check a minimal set of Python 2 tests for the remaining Python 2
+# functionality that we support
+commands = pytest tests/test_androidmodule_ctypes_finder.py tests/test_entrypoints_python2.py
+
 [testenv:py3]
 # for py3 env we will get code coverage
 commands =
@@ -28,8 +34,11 @@ commands = flake8 pythonforandroid/ tests/ ci/
 
 [flake8]
 ignore =
-    E123, E124, E126,
-    E226,
-    E402, E501,
-    W503,
-    W504
+    E123,  # Closing bracket does not match indentation of opening bracket's line
+    E124,  # Closing bracket does not match visual indentation
+    E126,  # Continuation line over-indented for hanging indent
+    E226,  # Missing whitespace around arithmetic operator
+    E402,  # Module level import not at top of file
+    E501,  # Line too long (82 > 79 characters)
+    W503,  # Line break occurred before a binary operator
+    W504   # Line break occurred after a binary operator
