Merge pull request #1640 from inclement/improve_ndk_version_check

Added separate module for checking user SDK, NDK, API etc.

Author: AndreMiras

pythonforandroid/build.py

@@ -15,10 +15,9 @@
 from pythonforandroid.logger import (info, warning, info_notify, info_main, shprint)
 from pythonforandroid.archs import ArchARM, ArchARMv7_a, ArchAarch_64, Archx86, Archx86_64
 from pythonforandroid.recipe import CythonRecipe, Recipe
-
-DEFAULT_ANDROID_API = 15
-
-DEFAULT_NDK_API = 21
+from pythonforandroid.recommendations import (
+    check_ndk_version, check_target_api, check_ndk_api,
+    RECOMMENDED_NDK_API, RECOMMENDED_TARGET_API)
 
 
 class Context(object):
@@ -140,19 +139,6 @@ def ndk_api(self):
     def ndk_api(self, value):
         self._ndk_api = value
 
-    @property
-    def ndk_ver(self):
-        '''The version of the NDK being used for compilation.'''
-        if self._ndk_ver is None:
-            raise ValueError('Tried to access ndk_ver but it has not '
-                             'been set - this should not happen, something '
-                             'went wrong!')
-        return self._ndk_ver
-
-    @ndk_ver.setter
-    def ndk_ver(self, value):
-        self._ndk_ver = value
-
     @property
     def sdk_dir(self):
         '''The path to the Android SDK.'''
@@ -183,7 +169,6 @@ def prepare_build_environment(self,
                                   user_sdk_dir,
                                   user_ndk_dir,
                                   user_android_api,
-                                  user_ndk_ver,
                                   user_ndk_api):
         '''Checks that build dependencies exist and sets internal variables
         for the Android SDK etc.
@@ -237,17 +222,12 @@ def prepare_build_environment(self,
             info('Found Android API target in $ANDROIDAPI: {}'.format(android_api))
         else:
             info('Android API target was not set manually, using '
-                 'the default of {}'.format(DEFAULT_ANDROID_API))
-            android_api = DEFAULT_ANDROID_API
+                 'the default of {}'.format(RECOMMENDED_TARGET_API))
+            android_api = RECOMMENDED_TARGET_API
         android_api = int(android_api)
         self.android_api = android_api
 
-        if self.android_api >= 21 and self.archs[0].arch == 'armeabi':
-            raise BuildInterruptingException(
-                'Asked to build for armeabi architecture with API '
-                '{}, but API 21 or greater does not support armeabi'.format(
-                    self.android_api),
-                instructions='You probably want to build with --arch=armeabi-v7a instead')
+        check_target_api(android_api, self.archs[0].arch)
 
         if exists(join(sdk_dir, 'tools', 'bin', 'avdmanager')):
             avdmanager = sh.Command(join(sdk_dir, 'tools', 'bin', 'avdmanager'))
@@ -306,47 +286,7 @@ def prepare_build_environment(self,
             raise BuildInterruptingException('Android NDK dir was not specified')
         self.ndk_dir = realpath(ndk_dir)
 
-        # Find the NDK version, and check it against what the NDK dir
-        # seems to report
-        ndk_ver = None
-        if user_ndk_ver:
-            ndk_ver = user_ndk_ver
-            if ndk_dir is not None:
-                info('Got NDK version from from user argument: {}'.format(ndk_ver))
-        if ndk_ver is None:
-            ndk_ver = environ.get('ANDROIDNDKVER', None)
-            if ndk_ver is not None:
-                info('Got NDK version from $ANDROIDNDKVER: {}'.format(ndk_ver))
-
-        self.ndk = 'google'
-
-        try:
-            with open(join(ndk_dir, 'RELEASE.TXT')) as fileh:
-                reported_ndk_ver = fileh.read().split(' ')[0].strip()
-        except IOError:
-            pass
-        else:
-            if reported_ndk_ver.startswith('crystax-ndk-'):
-                reported_ndk_ver = reported_ndk_ver[12:]
-                self.ndk = 'crystax'
-            if ndk_ver is None:
-                ndk_ver = reported_ndk_ver
-                info(('Got Android NDK version from the NDK dir: {}').format(ndk_ver))
-            else:
-                if ndk_ver != reported_ndk_ver:
-                    warning('NDK version was set as {}, but checking '
-                            'the NDK dir claims it is {}.'.format(
-                                ndk_ver, reported_ndk_ver))
-                    warning('The build will try to continue, but it may '
-                            'fail and you should check '
-                            'that your setting is correct.')
-                    warning('If the NDK dir result is correct, you don\'t '
-                            'need to manually set the NDK ver.')
-        if ndk_ver is None:
-            warning('Android NDK version could not be found. This probably'
-                    'won\'t cause any problems, but if necessary you can'
-                    'set it with `--ndk-version=...`.')
-        self.ndk_ver = ndk_ver
+        check_ndk_version(ndk_dir)
 
         ndk_api = None
         if user_ndk_api:
@@ -356,21 +296,14 @@ def prepare_build_environment(self,
             ndk_api = environ.get('NDKAPI', None)
             info('Found Android API target in $NDKAPI')
         else:
-            ndk_api = min(self.android_api, DEFAULT_NDK_API)
+            ndk_api = min(self.android_api, RECOMMENDED_NDK_API)
             warning('NDK API target was not set manually, using '
                     'the default of {} = min(android-api={}, default ndk-api={})'.format(
-                        ndk_api, self.android_api, DEFAULT_NDK_API))
+                        ndk_api, self.android_api, RECOMMENDED_NDK_API))
         ndk_api = int(ndk_api)
         self.ndk_api = ndk_api
 
-        if self.ndk_api > self.android_api:
-            raise BuildInterruptingException(
-                'Target NDK API is {}, higher than the target Android API {}.'.format(
-                    self.ndk_api, self.android_api),
-                instructions=('The NDK API is a minimum supported API number and must be lower '
-                              'than the target Android API'))
-
-        info('Using {} NDK {}'.format(self.ndk.capitalize(), self.ndk_ver))
+        check_ndk_api(ndk_api, self.android_api)
 
         virtualenv = None
         if virtualenv is None:
@@ -483,7 +416,6 @@ def __init__(self):
         self._ndk_dir = None
         self._android_api = None
         self._ndk_api = None
-        self._ndk_ver = None
         self.ndk = None
 
         self.toolchain_prefix = None

pythonforandroid/recommendations.py

@@ -0,0 +1,107 @@
+"""Simple functions for checking dependency versions."""
+
+from distutils.version import LooseVersion
+from os.path import join
+from pythonforandroid.logger import info, warning
+from pythonforandroid.util import BuildInterruptingException
+
+# We only check the NDK major version
+MIN_NDK_VERSION = 17
+MAX_NDK_VERSION = 17
+
+RECOMMENDED_NDK_VERSION = '17c'
+OLD_NDK_MESSAGE = 'Older NDKs may not be compatible with all p4a features.'
+NEW_NDK_MESSAGE = 'Newer NDKs may not be fully supported by p4a.'
+
+
+def check_ndk_version(ndk_dir):
+    # Check the NDK version against what is currently recommended
+    version = read_ndk_version(ndk_dir)
+
+    if version is None:
+        return  # if we failed to read the version, just don't worry about it
+
+    major_version = version.version[0]
+
+    info('Found NDK revision {}'.format(version))
+
+    if major_version < MIN_NDK_VERSION:
+        warning('Minimum recommended NDK version is {}'.format(
+            RECOMMENDED_NDK_VERSION))
+        warning(OLD_NDK_MESSAGE)
+    elif major_version > MAX_NDK_VERSION:
+        warning('Maximum recommended NDK version is {}'.format(
+            RECOMMENDED_NDK_VERSION))
+        warning(NEW_NDK_MESSAGE)
+
+
+def read_ndk_version(ndk_dir):
+    """Read the NDK version from the NDK dir, if possible"""
+    try:
+        with open(join(ndk_dir, 'source.properties')) as fileh:
+            ndk_data = fileh.read()
+    except IOError:
+        info('Could not determine NDK version, no source.properties '
+             'in the NDK dir')
+        return
+
+    for line in ndk_data.split('\n'):
+        if line.startswith('Pkg.Revision'):
+            break
+    else:
+        info('Could not parse $NDK_DIR/source.properties, not checking '
+             'NDK version')
+        return
+
+    # Line should have the form "Pkg.Revision = ..."
+    ndk_version = LooseVersion(line.split('=')[-1].strip())
+
+    return ndk_version
+
+
+MIN_TARGET_API = 26
+
+# highest version tested to work fine with SDL2
+# should be a good default for other bootstraps too
+RECOMMENDED_TARGET_API = 27
+
+ARMEABI_MAX_TARGET_API = 21
+OLD_API_MESSAGE = (
+    'Target APIs lower than 26 are no longer supported on Google Play, '
+    'and are not recommended. Note that the Target API can be higher than '
+    'your device Android version, and should usually be as high as possible.')
+
+
+def check_target_api(api, arch):
+    """Warn if the user's target API is less than the current minimum
+    recommendation
+    """
+
+    if api >= ARMEABI_MAX_TARGET_API and arch == 'armeabi':
+        raise BuildInterruptingException(
+            'Asked to build for armeabi architecture with API '
+            '{}, but API {} or greater does not support armeabi'.format(
+                api, ARMEABI_MAX_TARGET_API),
+            instructions='You probably want to build with --arch=armeabi-v7a instead')
+
+    if api < MIN_TARGET_API:
+        warning('Target API {} < {}'.format(api, MIN_TARGET_API))
+        warning(OLD_API_MESSAGE)
+
+
+MIN_NDK_API = 21
+RECOMMENDED_NDK_API = 21
+OLD_NDK_API_MESSAGE = ('NDK API less than {} is not supported'.format(MIN_NDK_API))
+
+
+def check_ndk_api(ndk_api, android_api):
+    """Warn if the user's NDK is too high or low."""
+    if ndk_api > android_api:
+        raise BuildInterruptingException(
+            'Target NDK API is {}, higher than the target Android API {}.'.format(
+                ndk_api, android_api),
+            instructions=('The NDK API is a minimum supported API number and must be lower '
+                          'than the target Android API'))
+
+    if ndk_api < MIN_NDK_API:
+        warning(OLD_NDK_API_MESSAGE)

pythonforandroid/toolchain.py

@@ -7,8 +7,10 @@
 """
 
 from __future__ import print_function
+from os import environ
 from pythonforandroid import __version__
-from pythonforandroid.build import DEFAULT_NDK_API, DEFAULT_ANDROID_API
+from pythonforandroid.recommendations import (
+    RECOMMENDED_NDK_API, RECOMMENDED_TARGET_API)
 from pythonforandroid.util import BuildInterruptingException, handle_build_exception
 
 
@@ -139,7 +141,6 @@ def wrapper_func(self, args):
         ctx.prepare_build_environment(user_sdk_dir=self.sdk_dir,
                                       user_ndk_dir=self.ndk_dir,
                                       user_android_api=self.android_api,
-                                      user_ndk_ver=self.ndk_version,
                                       user_ndk_api=self.ndk_api)
         dist = self._dist
         if dist.needs_build:
@@ -258,16 +259,16 @@ def __init__(self):
             default=0,
             type=int,
             help=('The Android API level to build against defaults to {} if '
-                  'not specified.').format(DEFAULT_ANDROID_API))
+                  'not specified.').format(RECOMMENDED_TARGET_API))
         generic_parser.add_argument(
-            '--ndk-version', '--ndk_version', dest='ndk_version', default='',
-            help=('The version of the Android NDK. This is optional: '
-                  'we try to work it out automatically from the ndk_dir.'))
+            '--ndk-version', '--ndk_version', dest='ndk_version', default=None,
+            help=('DEPRECATED: the NDK version is now found automatically or '
+                  'not at all.'))
         generic_parser.add_argument(
             '--ndk-api', type=int, default=None,
             help=('The Android API level to compile against. This should be your '
                   '*minimal supported* API, not normally the same as your --android-api. '
-                  'Defaults to min(ANDROID_API, {}) if not specified.').format(DEFAULT_NDK_API))
+                  'Defaults to min(ANDROID_API, {}) if not specified.').format(RECOMMENDED_NDK_API))
         generic_parser.add_argument(
             '--symlink-java-src', '--symlink_java_src',
             action='store_true',
@@ -360,6 +361,11 @@ def add_parser(subparsers, *args, **kwargs):
                 kwargs.pop('aliases')
             return subparsers.add_parser(*args, **kwargs)
 
+        parser_recommendations = add_parser(
+            subparsers,
+            'recommendations',
+            parents=[generic_parser],
+            help='List recommended p4a dependencies')
         parser_recipes = add_parser(
             subparsers,
             'recipes',
@@ -533,13 +539,14 @@ def add_parser(subparsers, *args, **kwargs):
                 requirements.append(requirement)
             args.requirements = u",".join(requirements)
 
+        self.warn_on_deprecated_args(args)
+
         self.ctx = Context()
         self.storage_dir = args.storage_dir
         self.ctx.setup_dirs(self.storage_dir)
         self.sdk_dir = args.sdk_dir
         self.ndk_dir = args.ndk_dir
         self.android_api = args.android_api
-        self.ndk_version = args.ndk_version
         self.ndk_api = args.ndk_api
         self.ctx.symlink_java_src = args.symlink_java_src
         self.ctx.java_build_tool = args.java_build_tool
@@ -552,6 +559,19 @@ def add_parser(subparsers, *args, **kwargs):
         # Each subparser corresponds to a method
         getattr(self, args.subparser_name.replace('-', '_'))(args)
 
+    def warn_on_deprecated_args(self, args):
+        """
+        Print warning messages for any deprecated arguments that were passed.
+        """
+
+        # NDK version is now determined automatically
+        if args.ndk_version is not None:
+            warning('--ndk-version is deprecated and no longer necessary, '
+                    'the value you passed is ignored')
+        if 'ANDROIDNDKVER' in environ:
+            warning('$ANDROIDNDKVER is deprecated and no longer necessary, '
+                    'the value you set is ignored')
+
     def hook(self, name):
         if not self.args.hook:
             return
@@ -959,7 +979,6 @@ def sdk_tools(self, args):
         ctx.prepare_build_environment(user_sdk_dir=self.sdk_dir,
                                       user_ndk_dir=self.ndk_dir,
                                       user_android_api=self.android_api,
-                                      user_ndk_ver=self.ndk_version,
                                       user_ndk_api=self.ndk_api)
         android = sh.Command(join(ctx.sdk_dir, 'tools', args.tool))
         output = android(
@@ -987,7 +1006,6 @@ def _adb(self, commands):
         ctx.prepare_build_environment(user_sdk_dir=self.sdk_dir,
                                       user_ndk_dir=self.ndk_dir,
                                       user_android_api=self.android_api,
-                                      user_ndk_ver=self.ndk_version,
                                       user_ndk_api=self.ndk_api)
         if platform in ('win32', 'cygwin'):
             adb = sh.Command(join(ctx.sdk_dir, 'platform-tools', 'adb.exe'))

testapps/on_device_unit_tests/buildozer.spec

@@ -153,7 +153,7 @@ android.whitelist = unittest/*
 #android.add_activites = com.example.ExampleActivity
 
 # (str) python-for-android branch to use, defaults to master
-#p4a.branch = master
+p4a.branch = master
 
 # (str) OUYA Console category. Should be one of GAME or APP
 # If you leave this blank, OUYA support will not be enabled

testapps/on_device_unit_tests/test_app/tests/test_requirements.py

@@ -47,4 +47,4 @@ class PyjniusTestCase(PythonTestMixIn, TestCase):
 
     def test_run_module(self):
         from jnius import autoclass
-        autoclass('org.kivy.PythonActivity')
+        autoclass('org.kivy.android.PythonActivity')