-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Drop Python 2 support #1918
Drop Python 2 support #1918
Changes from all commits
456086a
8decc12
7f2a926
7d6f32b
023ccbe
00d12b5
8637a20
b000d5b
173ee9e
97097fd
75f7921
ccaf7ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
data_files = [] | ||
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: I hate spacing 😄 |
||
# 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', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You guys are not lazy enough. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It isn't installed on my desktop :p I decided I preferred to be explicit about the Python 2 exception for the limited subset of files where we still need it. I'm willing to change it if you think it's better, but I consider this innocuous. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't need to be on your desktop since it's in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer keeping it in a way that works with the standard library ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for sharing. I actually don't mind that much. I'm just being too lazy and I prefer one line over 4. Specially since it has to be repeated over a lot of (yet to come) tests files |
||
import os | ||
import shutil | ||
import sys | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor let's be lazy here too and just: 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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to have comments about this. Best would be some unit tests. I can deal with this later