Improved code style per review comments

inclement · inclement · commit a96311274b76 · 2018-11-06T21:25:43.000Z
diff --git a/pythonforandroid/bootstraps/sdl2/build/build.py b/pythonforandroid/bootstraps/sdl2/build/build.py
@@ -240,12 +240,9 @@ def make_package(args):
 
     # Package up the private data (public not supported).
     tar_dirs = [args.private]
-    if exists('private'):
-        tar_dirs.append('private')
-    if exists('crystax_python'):
-        tar_dirs.append('crystax_python')
-    if exists('_python_bundle'):
-        tar_dirs.append('_python_bundle')
+    for python_bundle_dir in ('private', 'crystax_python', '_python_bundle'):
+        if exists(python_bundle_dir):
+            tar_dirs.append(python_bundle_Dir)
 
     if args.private:
         make_tar('src/main/assets/private.mp3', tar_dirs, args.ignore_path)
@@ -413,14 +410,14 @@ def make_package(args):
 def parse_args(args=None):
     global BLACKLIST_PATTERNS, WHITELIST_PATTERNS, PYTHON
 
-    # Get the default minsdk, equal to the NDK API that this dist it built against
+    # Get the default minsdk, equal to the NDK API that this dist is built against
     with open('dist_info.json', 'r') as fileh:
         info = json.load(fileh)
         if 'ndk_api' not in info:
-            print('Failed to read ndk_api from dist info')
-            default_android_api = 12  # The old default before ndk_api was introduced
+            print('WARNING: Failed to read ndk_api from dist info, defaulting to 12')
+            default_min_api = 12  # The old default before ndk_api was introduced
         else:
-            default_android_api = info['ndk_api']
+            default_min_api = info['ndk_api']
             ndk_api = info['ndk_api']
 
     import argparse
@@ -504,9 +501,9 @@ def parse_args(args=None):
     ap.add_argument('--sdk', dest='sdk_version', default=-1,
                     type=int, help=('Deprecated argument, does nothing'))
     ap.add_argument('--minsdk', dest='min_sdk_version',
-                    default=default_android_api, type=int,
+                    default=default_min_api, type=int,
                     help=('Minimum Android SDK version that the app supports. '
-                          'Defaults to {}.'.format(default_android_api)))
+                          'Defaults to {}.'.format(default_min_api)))
     ap.add_argument('--allow-minsdk-ndkapi-mismatch', default=False,
                     action='store_true',
                     help=('Allow the --minsdk argument to be different from '
diff --git a/pythonforandroid/bootstraps/sdl2/build/jni/src/start.c b/pythonforandroid/bootstraps/sdl2/build/jni/src/start.c
@@ -123,19 +123,18 @@ int main(int argc, char *argv[]) {
         snprintf(paths, 256,
                 "%s/stdlib.zip:%s/modules",
                 crystax_python_dir, crystax_python_dir);
-        LOGP("calculated paths to be...");
-        LOGP(paths);
     }
 
     if (dir_exists(python_bundle_dir)) {
         LOGP("_python_bundle dir exists");
         snprintf(paths, 256,
                 "%s/stdlib.zip:%s/modules",
                 python_bundle_dir, python_bundle_dir);
-        LOGP("calculated paths to be...");
-        LOGP(paths);
     }
 
+    LOGP("calculated paths to be...");
+    LOGP(paths);
+
 
     #if PY_MAJOR_VERSION >= 3
         wchar_t *wchar_paths = Py_DecodeLocale(paths, NULL);
@@ -148,7 +147,10 @@ int main(int argc, char *argv[]) {
 
         LOGP("set wchar paths...");
   } else {
-      LOGP("crystax_python does not exist");
+      // We do not expect to see crystax_python any more, so no point
+      // reminding the user about it. If it does exist, we'll have
+      // logged it earlier.
+      LOGP("_python_bundle does not exist");
   }
 
   Py_Initialize();
diff --git a/pythonforandroid/recipes/hostpython3/__init__.py b/pythonforandroid/recipes/hostpython3/__init__.py
@@ -30,25 +30,19 @@ def build_arch(self, arch):
 
         if not exists(join(build_dir, 'python')):
             with current_directory(recipe_build_dir):
-                env = {}  # The command line environment we will use
-
-
                 # Configure the build
                 with current_directory(build_dir):
                     if not exists('config.status'):
                         shprint(sh.Command(join(recipe_build_dir, 'configure')))
 
                 # Create the Setup file. This copying from Setup.dist
                 # seems to be the normal and expected procedure.
-                assert exists(join(build_dir, 'Modules')), (
-                    'Expected dir {} does not exist'.format(join(build_dir, 'Modules')))
                 shprint(sh.cp, join('Modules', 'Setup.dist'), join(build_dir, 'Modules', 'Setup'))
 
                 result = shprint(sh.make, '-C', build_dir)
         else:
             info('Skipping hostpython3 build as it has already been completed')
 
         self.ctx.hostpython = join(build_dir, 'python')
-        self.ctx.hostpgen = '/usr/bin/false'  # is this actually used for anything?
 
 recipe = Hostpython3Recipe()
diff --git a/pythonforandroid/recipes/hostpython3crystax/__init__.py b/pythonforandroid/recipes/hostpython3crystax/__init__.py
@@ -27,7 +27,6 @@ def build_arch(self, arch):
         Creates expected build and symlinks system Python version.
         """
         self.ctx.hostpython = '/usr/bin/false'
-        self.ctx.hostpgen = '/usr/bin/false'
         # creates the sub buildir (used by other recipes)
         # https://github.com/kivy/python-for-android/issues/1154
         sub_build_dir = join(self.get_build_dir(), 'build')
diff --git a/pythonforandroid/recipes/python2/__init__.py b/pythonforandroid/recipes/python2/__init__.py
@@ -209,8 +209,7 @@ def create_python_bundle(self, dirn, arch):
             # To quote the original distribute.sh, 'well...'
             shprint(sh.rm, '-rf', 'lib2to3')
             shprint(sh.rm, '-rf', 'idlelib')
-            for filename in glob.glob('config/libpython*.a'):
-                shprint(sh.rm, '-f', filename)
+            shprint(sh.rm, '-f', *glob.glob('config/libpython*.a'))
             shprint(sh.rm, '-rf', 'config/python.o')
 
         return site_packages_dir
