Code improvements around NDK download (#961)

* Cleaned up some variable references
* Improved how user build dir is accessed
* Minor code style improvements
* Fixed dist_dir calculation following p4a changes

buildozer/__init__.py

@@ -26,10 +26,10 @@ from fnmatch import fnmatch
 
 from pprint import pformat
 
-try:
+try:  # Python 3
     from urllib.request import FancyURLopener
     from configparser import SafeConfigParser
-except ImportError:
+except ImportError:  # Python 2
     from urllib import FancyURLopener
     from ConfigParser import SafeConfigParser
 try:
@@ -143,14 +143,6 @@ class Buildozer(object):
         except Exception:
             pass
 
-        build_dir = self.config.getdefault('buildozer', 'builddir', None)
-        if build_dir:
-            # for backwards compatibility, append .buildozer to builddir
-            build_dir = join(build_dir, '.buildozer')
-        self.build_dir = self.config.getdefault('buildozer', 'build_dir', build_dir)
-
-        if self.build_dir:
-            self.build_dir = realpath(join(self.root_dir, self.build_dir))
         self.user_bin_dir = self.config.getdefault('buildozer', 'bin_dir', None)
         if self.user_bin_dir:
             self.user_bin_dir = realpath(join(self.root_dir, self.user_bin_dir))
@@ -879,10 +871,27 @@ class Buildozer(object):
     def root_dir(self):
         return realpath(dirname(self.specfilename))
 
+    @property
+    def user_build_dir(self):
+        """The user-provided build dir, if any."""
+        # Check for a user-provided build dir
+        # Check the (deprecated) builddir token, for backwards compatibility
+        build_dir = self.config.getdefault('buildozer', 'builddir', None)
+        if build_dir is not None:
+            # for backwards compatibility, append .buildozer to builddir
+            build_dir = join(build_dir, '.buildozer')
+        build_dir = self.config.getdefault('buildozer', 'build_dir', build_dir)
+
+        if build_dir is not None:
+            build_dir = realpath(join(self.root_dir, build_dir))
+
+        return build_dir
+
     @property
     def buildozer_dir(self):
-        if self.build_dir:
+        '''The directory in which to run the app build.'''
-            return self.build_dir
+        if self.user_build_dir is not None:
+            return self.user_build_dir
         return join(self.root_dir, '.buildozer')
 
     @property
@@ -1107,6 +1116,23 @@ class Buildozer(object):
                 return
             rmtree(self.global_buildozer_dir)
 
+    def cmd_appclean(self, *args):
+        '''Clean the .buildozer folder in the app directory.
+
+        This command specifically refuses to delete files in a
+        user-specified build directory, to avoid accidentally deleting
+        more than the user intends.
+        '''
+        if self.user_build_dir is not None:
+            self.error(
+                ('Failed: build_dir is specified as {} in the buildozer config. `appclean` will '
+                 'not attempt to delete files in a user-specified build directory.').format(self.user_build_dir))
+        elif exists(self.buildozer_dir):
+            self.info('Deleting {}'.format(self.buildozer_dir))
+            rmtree(self.buildozer_dir)
+        else:
+            self.error('{} already deleted, skipping.'.format(self.buildozer_dir))
+
     def cmd_help(self, *args):
         '''Show the Buildozer help.
         '''

buildozer/targets/android.py

@@ -1,11 +1,6 @@
 '''
-Android target, based on python-for-android project (old toolchain)
+Android target, based on python-for-android project
 '''
-#
-# Android target
-# Thanks for Renpy (again) for its install_sdk.py and plat.py in the PGS4A
-# project!
-#
 
 import sys
 if sys.platform == 'win32':
@@ -16,9 +11,14 @@ WSL = 'Microsoft' in uname()[2]
 
 ANDROID_API = '27'
 ANDROID_MINAPI = '21'
-ANDROID_NDK_VERSION = '17c'
 APACHE_ANT_VERSION = '1.9.4'
 
+# This constant should *not* be updated, it is used only in the case
+# that python-for-android cannot provide a recommendation, which in
+# turn only happens if the python-for-android is old and probably
+# doesn't support any newer NDK.
+DEFAULT_ANDROID_NDK_VERSION = '17c'
+
 import traceback
 import os
 import io
@@ -47,26 +47,28 @@ DEPRECATED_TOKENS = (('app', 'android.sdk'), )
 # does.
 DEFAULT_SDK_TAG = '4333796'
 
+DEFAULT_ARCH = 'armeabi-v7a'
+
 MSG_P4A_RECOMMENDED_NDK_ERROR = (
-    "WARNING: Unable to find recommended android's NDK for current "
+    "WARNING: Unable to find recommended Android NDK for current "
-    "installation of p4...so returning the recommended buildozer's one, which "
+    "installation of python-for-android, defaulting to the default "
-    "is android's NDK r{android_ndk}".format(android_ndk=ANDROID_NDK_VERSION)
+    "version r{android_ndk}".format(android_ndk=DEFAULT_ANDROID_NDK_VERSION)
 )
 
 
 class TargetAndroid(Target):
     targetname = 'android'
-    p4a_directory = "python-for-android"
+    p4a_directory_name = "python-for-android"
     p4a_fork = 'kivy'
     p4a_branch = 'master'
     p4a_apk_cmd = "apk --debug --bootstrap="
-    p4a_best_ndk_version = None
+    p4a_recommended_ndk_version = None
     extra_p4a_args = ''
 
     def __init__(self, *args, **kwargs):
         super(TargetAndroid, self).__init__(*args, **kwargs)
         self._arch = self.buildozer.config.getdefault(
-            'app', 'android.arch', "armeabi-v7a")
+            'app', 'android.arch', DEFAULT_ARCH)
         self._build_dir = join(
             self.buildozer.platform_dir, 'build-{}'.format(self._arch))
         executable = sys.executable or 'python'
@@ -102,32 +104,39 @@ class TargetAndroid(Target):
                 self.buildozer.error(error)
 
     def _p4a(self, cmd, **kwargs):
-        if not hasattr(self, "pa_dir"):
+        kwargs.setdefault('cwd', self.p4a_dir)
-            self.pa_dir = join(self.buildozer.platform_dir, self.p4a_directory)
-        kwargs.setdefault('cwd', self.pa_dir)
         return self.buildozer.cmd(self._p4a_cmd + cmd + self.extra_p4a_args, **kwargs)
 
     @property
-    def p4a_best_android_ndk(self):
+    def p4a_dir(self):
+        """The directory where python-for-android is/will be installed."""
+
+        # Default p4a dir
+        p4a_dir = join(self.buildozer.platform_dir, self.p4a_directory_name)
+
+        # Possibly overriden by user setting
+        system_p4a_dir = self.buildozer.config.getdefault('app', 'p4a.source_dir')
+        if system_p4a_dir:
+            p4a_dir = expanduser(system_p4a_dir)
+
+        return p4a_dir
+
+    @property
+    def p4a_recommended_android_ndk(self):
         """
         Return the p4a's recommended android's NDK version, depending on the
         p4a version used for our buildozer build. In case that we don't find
         it, we will return the buildozer's recommended one, defined by global
-        variable `ANDROID_NDK_VERSION`.
+        variable `DEFAULT_ANDROID_NDK_VERSION`.
         """
-        if self.p4a_best_ndk_version is not None:
         # make sure to read p4a version only the first time
-            return self.p4a_best_ndk_version
+        if self.p4a_recommended_ndk_version is not None:
-
+            return self.p4a_recommended_ndk_version
-        if not hasattr(self, "pa_dir"):
-            pa_dir = join(self.buildozer.platform_dir, self.p4a_directory)
-        else:
-            pa_dir = self.pa_dir
 
         # check p4a's recommendation file, and in case that exists find the
         # recommended android's NDK version, otherwise return buildozer's one
-        ndk_version = ANDROID_NDK_VERSION
+        ndk_version = DEFAULT_ANDROID_NDK_VERSION
-        rec_file = join(pa_dir, "pythonforandroid", "recommendations.py")
+        rec_file = join(self.p4a_dir, "pythonforandroid", "recommendations.py")
         if not os.path.isfile(rec_file):
             self.buildozer.error(MSG_P4A_RECOMMENDED_NDK_ERROR)
             return ndk_version
@@ -144,7 +153,7 @@ class TargetAndroid(Target):
                         ndk_version
                     )
                 )
-                self.p4a_best_ndk_version = ndk_version
+                self.p4a_recommended_ndk_version = ndk_version
                 break
         return ndk_version
 
@@ -163,7 +172,7 @@ class TargetAndroid(Target):
     @property
     def android_ndk_version(self):
         return self.buildozer.config.getdefault('app', 'android.ndk',
-                                                self.p4a_best_android_ndk)
+                                                self.p4a_recommended_android_ndk)
 
     @property
     def android_api(self):
@@ -433,7 +442,7 @@ class TargetAndroid(Target):
                 archive = 'android-ndk-r{0}-linux-{1}.tar.bz2'
             is_64 = (os.uname()[4] == 'x86_64')
         else:
-            raise SystemError('Unsupported platform: {0}'.format(platform))
+            raise SystemError('Unsupported platform: {}'.format(platform))
 
         architecture = 'x86_64' if is_64 else 'x86'
         unpacked = 'android-ndk-r{0}'
@@ -677,27 +686,27 @@ class TargetAndroid(Target):
         p4a_branch = self.buildozer.config.getdefault(
             'app', 'p4a.branch', self.p4a_branch
         )
-        self.pa_dir = pa_dir = join(self.buildozer.platform_dir,
+
-                                    self.p4a_directory)
+        p4a_dir = self.p4a_dir
         system_p4a_dir = self.buildozer.config.getdefault('app',
                                                           'p4a.source_dir')
         if system_p4a_dir:
-            self.pa_dir = pa_dir = expanduser(system_p4a_dir)
+            # Don't install anything, just check that the dir does exist
-            if not self.buildozer.file_exists(pa_dir):
+            if not self.buildozer.file_exists(p4a_dir):
                 self.buildozer.error(
                     'Path for p4a.source_dir does not exist')
                 self.buildozer.error('')
                 raise BuildozerException()
         else:
             # check that fork/branch has not been changed
-            if self.buildozer.file_exists(pa_dir):
+            if self.buildozer.file_exists(p4a_dir):
                 cur_fork = cmd(
                     'git config --get remote.origin.url',
                     get_stdout=True,
-                    cwd=pa_dir,
+                    cwd=p4a_dir,
                 )[0].split('/')[3]
                 cur_branch = cmd(
-                    'git branch -vv', get_stdout=True, cwd=pa_dir
+                    'git branch -vv', get_stdout=True, cwd=p4a_dir
                 )[0].split()[1]
                 if any([cur_fork != p4a_fork, cur_branch != p4a_branch]):
                     self.buildozer.info(
@@ -705,9 +714,9 @@ class TargetAndroid(Target):
                             cur_fork, cur_branch
                         )
                     )
-                    rmtree(pa_dir)
+                    rmtree(p4a_dir)
 
-            if not self.buildozer.file_exists(pa_dir):
+            if not self.buildozer.file_exists(p4a_dir):
                 cmd(
                     (
                         'git clone -b {p4a_branch} --single-branch '
@@ -716,31 +725,31 @@ class TargetAndroid(Target):
                     ).format(
                         p4a_branch=p4a_branch,
                         p4a_fork=p4a_fork,
-                        p4a_dir=self.p4a_directory,
+                        p4a_dir=self.p4a_directory_name,
                     ),
                     cwd=self.buildozer.platform_dir,
                 )
             elif self.platform_update:
-                cmd('git clean -dxf', cwd=pa_dir)
+                cmd('git clean -dxf', cwd=p4a_dir)
                 current_branch = cmd('git rev-parse --abbrev-ref HEAD',
-                                     get_stdout=True, cwd=pa_dir)[0].strip()
+                                     get_stdout=True, cwd=p4a_dir)[0].strip()
                 if current_branch == p4a_branch:
-                    cmd('git pull', cwd=pa_dir)
+                    cmd('git pull', cwd=p4a_dir)
                 else:
                     cmd('git fetch --tags origin {0}:{0}'.format(p4a_branch),
-                        cwd=pa_dir)
+                        cwd=p4a_dir)
-                    cmd('git checkout {}'.format(p4a_branch), cwd=pa_dir)
+                    cmd('git checkout {}'.format(p4a_branch), cwd=p4a_dir)
 
         # also install dependencies (currently, only setup.py knows about it)
         # let's extract them.
         try:
-            with open(join(self.pa_dir, "setup.py")) as fd:
+            with open(join(self.p4a_dir, "setup.py")) as fd:
                 setup = fd.read()
                 deps = re.findall("^\s*install_reqs = (\[[^\]]*\])", setup, re.DOTALL | re.MULTILINE)[0]
                 deps = ast.literal_eval(deps)
         except IOError:
             self.buildozer.error('Failed to read python-for-android setup.py at {}'.format(
-                join(self.pa_dir, 'setup.py')))
+                join(self.p4a_dir, 'setup.py')))
             exit(1)
         pip_deps = []
         for dep in deps:
@@ -788,8 +797,27 @@ class TargetAndroid(Target):
     def get_available_packages(self):
         return True
 
-    def get_dist_dir(self, dist_name):
+    def get_dist_dir(self, dist_name, arch):
-        return join(self._build_dir, 'dists', "{}__{}".format(dist_name, self._arch))
+        """Find the dist dir with the given name and target arch, if one
+        already exists, otherwise return a new dist_dir name.
+        """
+        expected_dist_name = generate_dist_folder_name(dist_name, arch_names=[arch])
+
+        # If the expected dist name does exist, simply use that
+        expected_dist_dir = join(self._build_dir, 'dists', expected_dist_name)
+        if exists(expected_dist_dir):
+            return expected_dist_dir
+
+        # For backwards compatibility, check if a directory without
+        # the arch exists. If so, this is probably the target dist.
+        old_dist_dir = join(self._build_dir, 'dists', dist_name)
+        if exists(old_dist_dir):
+            return old_dist_dir
+        matching_dirs = glob.glob(join(self._build_dir, 'dist', '{}*'.format(dist_name)))
+
+        # If no directory has been found yet, our dist probably
+        # doesn't exist yet, so use the expected name
+        return expected_dist_dir
 
     def get_local_recipes_dir(self):
         local_recipes = self.buildozer.config.getdefault('app', 'p4a.local_recipes')
@@ -928,7 +956,7 @@ class TargetAndroid(Target):
             print('To set up p4a in this shell session, execute:')
             print('    alias p4a=$(buildozer {} p4a --alias 2>&1 >/dev/null)'
                   .format(self.targetname))
-            sys.stderr.write('PYTHONPATH={} {}\n'.format(self.pa_dir, self._p4a_cmd))
+            sys.stderr.write('PYTHONPATH={} {}\n'.format(self.p4a_dir, self._p4a_cmd))
         else:
             self._p4a(' '.join(args) if args else '')
 
@@ -957,7 +985,8 @@ class TargetAndroid(Target):
 
     def build_package(self):
         dist_name = self.buildozer.config.get('app', 'package.name')
-        dist_dir = self.get_dist_dir(dist_name)
+        arch = self.buildozer.config.getdefault('app', 'android.arch', DEFAULT_ARCH)
+        dist_dir = self.get_dist_dir(dist_name, arch)
         config = self.buildozer.config
         package = self._get_package()
         version = self.buildozer.get_version()
@@ -995,8 +1024,6 @@ class TargetAndroid(Target):
             ("--name", quote(config.get('app', 'title'))),
             ("--version", version),
             ("--package", package),
-            ("--sdk", config.getdefault('app', 'android.api',
-                                        self.android_api)),
             ("--minsdk", config.getdefault('app', 'android.minapi',
                                            self.android_minapi)),
             ("--ndk-api", config.getdefault('app', 'android.minapi',
@@ -1128,8 +1155,9 @@ class TargetAndroid(Target):
 
         if is_gradle_build:
             # on gradle build, the apk use the package name, and have no version
-            apk = u'{packagename}__{arch}-{mode}.apk'.format(
+            packagename = basename(dist_dir)  # gradle specifically uses the folder name
-                packagename=packagename, arch=self._arch, mode=mode)
+            apk = u'{packagename}-{mode}.apk'.format(
+                packagename=packagename, mode=mode)
             apk_dir = join(dist_dir, "build", "outputs", "apk", mode_sign)
         else:
             # on ant, the apk use the title, and have version
@@ -1312,3 +1340,27 @@ class TargetAndroid(Target):
 def get_target(buildozer):
     buildozer.targetname = "android"
     return TargetAndroid(buildozer)
+
+def generate_dist_folder_name(base_dist_name, arch_names=None):
+    """Generate the distribution folder name to use, based on a
+    combination of the input arguments.
+
+    WARNING: This function is copied from python-for-android. It would
+    be preferable to have a proper interface, either importing the p4a
+    code or having a p4a dist dir query option.
+
+    Parameters
+    ----------
+    base_dist_name : str
+        The core distribution identifier string
+    arch_names : list of str
+        The architecture compile targets
+
+    """
+    if arch_names is None:
+        arch_names = ["no_arch_specified"]
+
+    return '{}__{}'.format(
+        base_dist_name,
+        '_'.join(arch_names)
+    )

buildozer/targets/osx.py

@@ -159,7 +159,7 @@ class TargetOSX(Target):
         self.buildozer.info('{}.dmg created'.format(package_name))
         self.buildozer.info('moving {}.dmg to bin.'.format(package_name))
         binpath = join(
-            self.buildozer.build_dir or
+            self.buildozer.user_build_dir or
             dirname(abspath(self.buildozer.specfilename)), 'bin')
         check_output(
             ('cp', '-a', package_name+'.dmg', binpath),
@@ -264,4 +264,3 @@ class TargetOSX(Target):
 
 def get_target(buildozer):
     return TargetOSX(buildozer)
-

tests/scripts/test_client.py

@@ -1,9 +1,12 @@
 import sys
-import mock
 import unittest
 from buildozer import BuildozerCommandException
 from buildozer.scripts import client
 
+try:
+    from unittest import mock  # Python 3
+except ImportError:
+    import mock  # Python 2
 
 class TestClient(unittest.TestCase):
 

tests/test_buildozer.py

@@ -1,15 +1,16 @@
 import re
 import os
 import codecs
-import mock
 import unittest
 import buildozer as buildozer_module
 from buildozer import Buildozer, IS_PY3
 from six import StringIO
 import tempfile
 
+import mock
+
 from buildozer.targets.android import (
-    TargetAndroid, ANDROID_NDK_VERSION, MSG_P4A_RECOMMENDED_NDK_ERROR
+    TargetAndroid, DEFAULT_ANDROID_NDK_VERSION, MSG_P4A_RECOMMENDED_NDK_ERROR
 )
 
 
@@ -203,25 +204,25 @@ class TestBuildozer(unittest.TestCase):
                 mock.call(command_output)
             ]
 
-    def test_p4a_best_ndk_version_default_value(self):
+    def test_p4a_recommended_ndk_version_default_value(self):
         self.set_specfile_log_level(self.specfile.name, 1)
         buildozer = Buildozer(self.specfile.name, 'android')
-        assert buildozer.target.p4a_best_ndk_version is None
+        assert buildozer.target.p4a_recommended_ndk_version is None
 
-    def test_p4a_best_android_ndk_error(self):
+    def test_p4a_recommended_android_ndk_error(self):
         self.set_specfile_log_level(self.specfile.name, 1)
         buildozer = Buildozer(self.specfile.name, 'android')
 
         with mock.patch('sys.stdout', new_callable=StringIO) as mock_stdout:
-            ndk_version = buildozer.target.p4a_best_android_ndk
+            ndk_version = buildozer.target.p4a_recommended_android_ndk
         assert MSG_P4A_RECOMMENDED_NDK_ERROR in mock_stdout.getvalue()
         # and we should get the default android's ndk version of buildozer
-        assert ndk_version == ANDROID_NDK_VERSION
+        assert ndk_version == DEFAULT_ANDROID_NDK_VERSION
 
     @mock.patch('buildozer.targets.android.os.path.isfile')
     @mock.patch('buildozer.targets.android.os.path.exists')
-    @mock.patch('buildozer.targets.android.open')
+    @mock.patch('buildozer.targets.android.open', create=True)
-    def test_p4a_best_android_ndk_found(
+    def test_p4a_recommended_android_ndk_found(
             self, mock_open, mock_exists, mock_isfile
     ):
         self.set_specfile_log_level(self.specfile.name, 1)
@@ -234,16 +235,16 @@ class TestBuildozer(unittest.TestCase):
                     expected_ndk=expected_ndk)
             ).return_value
         ]
-        ndk_version = buildozer.target.p4a_best_android_ndk
+        ndk_version = buildozer.target.p4a_recommended_android_ndk
-        pa_dir = os.path.join(
+        p4a_dir = os.path.join(
-            buildozer.platform_dir, buildozer.target.p4a_directory)
+            buildozer.platform_dir, buildozer.target.p4a_directory_name)
         mock_open.assert_called_once_with(
-            os.path.join(pa_dir, "pythonforandroid", "recommendations.py"), 'r'
+            os.path.join(p4a_dir, "pythonforandroid", "recommendations.py"), 'r'
         )
         assert ndk_version == expected_ndk
 
         # now test that we only read one time p4a file, so we call again to
-        # `p4a_best_android_ndk` and we should still have one call to `open`
+        # `p4a_recommended_android_ndk` and we should still have one call to `open`
         # file, the performed above
-        ndk_version = buildozer.target.p4a_best_android_ndk
+        ndk_version = buildozer.target.p4a_recommended_android_ndk
         mock_open.assert_called_once()