From 34a98374b1878cb35f30b186604115465c5fddd2 Mon Sep 17 00:00:00 2001
From: Jonas Thiem <jonas@thiem.email>
Date: Sun, 16 Jun 2019 05:32:22 +0200
Subject: [PATCH] Fix debug build missing symbols and other related issues

- fixes that a debug (non-`--release`) `.apk` build won't actually enable gdb debugging in the manifest
- renames `build.py`'s `parse_args` to `parse_args_and_make_package` because that is what it appears to be actually doing :smile:
- makes SDL2 and any standard `NDKRecipe` build with `NDK_DEBUG=1` when `--release` is not specified to add debugging symbols
- fixes debugging symbols being stripped even when not using `--releaase`
  which makes using gdb a hassle
---
 .../bootstraps/common/build/build.py          | 14 ++++++---
 .../common/build/templates/build.tmpl.gradle  |  6 ++++
 pythonforandroid/bootstraps/sdl2/__init__.py  |  3 +-
 .../build/templates/AndroidManifest.tmpl.xml  |  1 +
 .../bootstraps/service_only/__init__.py       |  3 +-
 .../bootstraps/webview/__init__.py            |  3 +-
 pythonforandroid/build.py                     | 11 ++++---
 pythonforandroid/recipe.py                    |  4 ++-
 pythonforandroid/recipes/sdl2/__init__.py     |  7 ++++-
 pythonforandroid/toolchain.py                 | 20 +++++++++++--
 tests/test_build.py                           | 29 +++++++++++++++++++
 11 files changed, 85 insertions(+), 16 deletions(-)

diff --git a/pythonforandroid/bootstraps/common/build/build.py b/pythonforandroid/bootstraps/common/build/build.py
index ed5e708892..f32047d417 100644
--- a/pythonforandroid/bootstraps/common/build/build.py
+++ b/pythonforandroid/bootstraps/common/build/build.py
@@ -486,7 +486,8 @@ def make_package(args):
         "args": args,
         "service": service,
         "service_names": service_names,
-        "android_api": android_api
+        "android_api": android_api,
+        "debug": "debug" in args.build_mode,
     }
     if get_bootstrap_name() == "sdl2":
         render_args["url_scheme"] = url_scheme
@@ -509,7 +510,8 @@ def make_package(args):
         aars=aars,
         jars=jars,
         android_api=android_api,
-        build_tools_version=build_tools_version
+        build_tools_version=build_tools_version,
+        debug_build="debug" in args.build_mode,
         )
 
     # ant build templates
@@ -574,7 +576,7 @@ def make_package(args):
                     raise e
 
 
-def parse_args(args=None):
+def parse_args_and_make_package(args=None):
     global BLACKLIST_PATTERNS, WHITELIST_PATTERNS, PYTHON
 
     # Get the default minsdk, equal to the NDK API that this dist is built against
@@ -674,6 +676,10 @@ def parse_args(args=None):
                     default=join(curdir, 'whitelist.txt'),
                     help=('Use a whitelist file to prevent blacklisting of '
                           'file in the final APK'))
+    ap.add_argument('--release', dest='build_mode', action='store_const',
+                    const='release', default='debug',
+                    help='Build your app as a non-debug release build. '
+                         '(Disables gdb debugging among other things)')
     ap.add_argument('--add-jar', dest='add_jar', action='append',
                     help=('Add a Java .jar to the libs, so you can access its '
                           'classes with pyjnius. You can specify this '
@@ -817,4 +823,4 @@ def _read_configuration():
 
 
 if __name__ == "__main__":
-    parse_args()
+    parse_args_and_make_package()
diff --git a/pythonforandroid/bootstraps/common/build/templates/build.tmpl.gradle b/pythonforandroid/bootstraps/common/build/templates/build.tmpl.gradle
index 32bd091b72..a77f4b22b9 100644
--- a/pythonforandroid/bootstraps/common/build/templates/build.tmpl.gradle
+++ b/pythonforandroid/bootstraps/common/build/templates/build.tmpl.gradle
@@ -31,6 +31,12 @@ android {
 		versionName '{{ args.version }}'
 	}
 
+	{% if debug_build -%}
+	packagingOptions {
+		doNotStrip '**/*.so'
+	}
+	{%- endif %}
+
 	{% if args.sign -%}
 	signingConfigs {
 		release {
diff --git a/pythonforandroid/bootstraps/sdl2/__init__.py b/pythonforandroid/bootstraps/sdl2/__init__.py
index dde5f7f79b..99b8779ffa 100644
--- a/pythonforandroid/bootstraps/sdl2/__init__.py
+++ b/pythonforandroid/bootstraps/sdl2/__init__.py
@@ -47,7 +47,8 @@ def run_distribute(self):
                 with open('blacklist.txt', 'a') as fileh:
                     fileh.write('\nsqlite3/*\nlib-dynload/_sqlite3.so\n')
 
-        self.strip_libraries(arch)
+        if not self.ctx.build_as_debuggable:
+            self.strip_libraries(arch)
         self.fry_eggs(site_packages_dir)
         super(SDL2GradleBootstrap, self).run_distribute()
 
diff --git a/pythonforandroid/bootstraps/sdl2/build/templates/AndroidManifest.tmpl.xml b/pythonforandroid/bootstraps/sdl2/build/templates/AndroidManifest.tmpl.xml
index 119a4daefd..95450fe1b8 100644
--- a/pythonforandroid/bootstraps/sdl2/build/templates/AndroidManifest.tmpl.xml
+++ b/pythonforandroid/bootstraps/sdl2/build/templates/AndroidManifest.tmpl.xml
@@ -52,6 +52,7 @@
          An example Java class can be found in README-android.txt
     -->
     <application android:label="@string/app_name"
+                 {% if debug %}android:debuggable="true"{% endif %}
                  android:icon="@drawable/icon"
                  android:allowBackup="{{ args.allow_backup }}"
                  android:theme="@android:style/Theme.NoTitleBar{% if not args.window %}.Fullscreen{% endif %}"
diff --git a/pythonforandroid/bootstraps/service_only/__init__.py b/pythonforandroid/bootstraps/service_only/__init__.py
index 2175f8b61f..f3388100a1 100644
--- a/pythonforandroid/bootstraps/service_only/__init__.py
+++ b/pythonforandroid/bootstraps/service_only/__init__.py
@@ -45,7 +45,8 @@ def run_distribute(self):
                 with open('blacklist.txt', 'a') as fileh:
                     fileh.write('\nsqlite3/*\nlib-dynload/_sqlite3.so\n')
 
-        self.strip_libraries(arch)
+        if not self.ctx.build_as_debuggable:
+            self.strip_libraries(arch)
         self.fry_eggs(site_packages_dir)
         super(ServiceOnlyBootstrap, self).run_distribute()
 
diff --git a/pythonforandroid/bootstraps/webview/__init__.py b/pythonforandroid/bootstraps/webview/__init__.py
index c59c158356..a3c61bb6ef 100644
--- a/pythonforandroid/bootstraps/webview/__init__.py
+++ b/pythonforandroid/bootstraps/webview/__init__.py
@@ -42,7 +42,8 @@ def run_distribute(self):
                 with open('blacklist.txt', 'a') as fileh:
                     fileh.write('\nsqlite3/*\nlib-dynload/_sqlite3.so\n')
 
-        self.strip_libraries(arch)
+        if not self.ctx.build_as_debuggable:
+            self.strip_libraries(arch)
         self.fry_eggs(site_packages_dir)
         super(WebViewBootstrap, self).run_distribute()
 
diff --git a/pythonforandroid/build.py b/pythonforandroid/build.py
index 16cc459b4c..7b4461ea03 100644
--- a/pythonforandroid/build.py
+++ b/pythonforandroid/build.py
@@ -83,6 +83,9 @@ class Context(object):
     '''A build context. If anything will be built, an instance this class
     will be instantiated and used to hold all the build state.'''
 
+    # Whether to build with debugging symbols
+    build_as_debuggable = False
+
     env = environ.copy()
     # the filepath of toolchain.py
     root_dir = None
@@ -601,8 +604,6 @@ def build_recipes(build_order, python_modules, ctx, project_dir,
         ignore_setup_py=ignore_project_setup_py
     )
 
-    return
-
 
 def project_has_setup_py(project_dir):
     if project_dir is not None and \
@@ -847,8 +848,10 @@ def run_pymodules_install(ctx, modules, project_dir=None,
                 )
 
         # Strip object files after potential Cython or native code builds:
-        standard_recipe.strip_object_files(ctx.archs[0], env,
-                                           build_dir=ctx.build_dir)
+        if not ctx.build_as_debuggable:
+            standard_recipe.strip_object_files(
+                ctx.archs[0], env, build_dir=ctx.build_dir
+            )
 
 
 def biglink(ctx, arch):
diff --git a/pythonforandroid/recipe.py b/pythonforandroid/recipe.py
index 7d2c4ac4e8..822ebce1ae 100644
--- a/pythonforandroid/recipe.py
+++ b/pythonforandroid/recipe.py
@@ -695,6 +695,7 @@ def build_arch(self, arch, *extra_args):
             shprint(
                 sh.ndk_build,
                 'V=1',
+                'NDK_DEBUG=' + ("1" if self.ctx.build_as_debuggable else "0"),
                 'APP_PLATFORM=android-' + str(self.ctx.ndk_api),
                 'APP_ABI=' + arch.arch,
                 *extra_args, _env=env
@@ -998,7 +999,8 @@ def build_cython_components(self, arch):
                 info('First build appeared to complete correctly, skipping manual'
                      'cythonising.')
 
-            self.strip_object_files(arch, env)
+            if not self.ctx.build_as_debuggable:
+                self.strip_object_files(arch, env)
 
     def strip_object_files(self, arch, env, build_dir=None):
         if build_dir is None:
diff --git a/pythonforandroid/recipes/sdl2/__init__.py b/pythonforandroid/recipes/sdl2/__init__.py
index 00345e89a5..caef78a80e 100644
--- a/pythonforandroid/recipes/sdl2/__init__.py
+++ b/pythonforandroid/recipes/sdl2/__init__.py
@@ -22,7 +22,12 @@ def build_arch(self, arch):
         env = self.get_recipe_env(arch)
 
         with current_directory(self.get_jni_dir()):
-            shprint(sh.ndk_build, "V=1", _env=env)
+            shprint(
+                sh.ndk_build,
+                "V=1",
+                "NDK_DEBUG=" + ("1" if self.ctx.build_as_debuggable else "0"),
+                _env=env
+            )
 
 
 recipe = LibSDL2Recipe()
diff --git a/pythonforandroid/toolchain.py b/pythonforandroid/toolchain.py
index d7883a2690..42048fc176 100644
--- a/pythonforandroid/toolchain.py
+++ b/pythonforandroid/toolchain.py
@@ -193,6 +193,10 @@ def build_dist_from_args(ctx, dist, args):
         ctx.recipe_build_order))
     info('Dist will also contain modules ({}) installed from pip'.format(
         ', '.join(ctx.python_modules)))
+    if hasattr(args, "build_mode") and args.build_mode == "debug":
+        info('Building WITH debugging symbols (no --release option used)')
+    else:
+        info('Building WITHOUT debugging symbols (--release option used)')
 
     ctx.dist_name = bs.distribution.name
     ctx.prepare_bootstrap(bs)
@@ -495,7 +499,8 @@ def add_parser(subparsers, *args, **kwargs):
         parser_apk.add_argument(
             '--release', dest='build_mode', action='store_const',
             const='release', default='debug',
-            help='Build the PARSER_APK. in Release mode')
+            help='Build your app as a non-debug release build. '
+                 '(Disables gdb debugging among other things)')
         parser_apk.add_argument(
             '--use-setup-py', dest="use_setup_py",
             action='store_true', default=False,
@@ -571,6 +576,8 @@ def add_parser(subparsers, *args, **kwargs):
         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]
+        if hasattr(args, "build_mode") and args.build_mode == "release":
+            args.unknown_args += ["--release"]
         if hasattr(args, "ignore_setup_py") and args.ignore_setup_py:
             args.use_setup_py = False
 
@@ -587,6 +594,9 @@ def add_parser(subparsers, *args, **kwargs):
 
         self.ctx = Context()
         self.ctx.use_setup_py = getattr(args, "use_setup_py", True)
+        self.ctx.build_as_debuggable = getattr(
+            args, "build_mode", "debug"
+        ) == "debug"
 
         have_setup_py_or_similar = False
         if getattr(args, "private", None) is not None:
@@ -953,7 +963,9 @@ def apk(self, args):
         with current_directory(dist.dist_dir):
             self.hook("before_apk_build")
             os.environ["ANDROID_API"] = str(self.ctx.android_api)
-            build_args = build.parse_args(args.unknown_args)
+            build_args = build.parse_args_and_make_package(
+                args.unknown_args
+            )
             self.hook("after_apk_build")
             self.hook("before_apk_assemble")
 
@@ -1003,7 +1015,9 @@ def apk(self, args):
                     gradle_task = "assembleRelease"
                 else:
                     raise BuildInterruptingException(
-                        "Unknown build mode {} for apk()".format(args.build_mode))
+                        "Unknown build mode {} for apk()".
+                        format(args.build_mode)
+                    )
                 output = shprint(gradlew, gradle_task, _tail=20,
                                  _critical=True, _env=env)
 
diff --git a/tests/test_build.py b/tests/test_build.py
index 784e24c23f..2e93ad6f8a 100644
--- a/tests/test_build.py
+++ b/tests/test_build.py
@@ -23,3 +23,32 @@ def test_run_pymodules_install_optional_project_dir(self):
             assert run_pymodules_install(ctx, modules, project_dir) is None
         assert m_info.call_args_list[-1] == mock.call(
             'No Python modules and no setup.py to process, skipping')
+
+    def test_strip_if_debuggable(self):
+        ctx = mock.Mock()
+        ctx.python_recipe.major_minor_version_string = "python3.6"
+        ctx.get_site_packages_dir.return_value = "test-doesntexist"
+        ctx.build_dir = "nonexistant_directory"
+        ctx.archs = ["arm64"]
+
+        modules = ["mymodule"]
+        project_dir = None
+        with mock.patch('pythonforandroid.build.info'), \
+                mock.patch('sh.Command'),\
+                mock.patch('pythonforandroid.build.open'),\
+                mock.patch('pythonforandroid.build.shprint'),\
+                mock.patch('pythonforandroid.build.current_directory'),\
+                mock.patch('pythonforandroid.build.CythonRecipe') as m_CythonRecipe, \
+                mock.patch('pythonforandroid.build.project_has_setup_py') as m_project_has_setup_py, \
+                mock.patch('pythonforandroid.build.run_setuppy_install'):
+            m_project_has_setup_py.return_value = False
+
+            # Make sure it is NOT called when debug build:
+            ctx.build_as_debuggable = True
+            assert run_pymodules_install(ctx, modules, project_dir) is None
+            assert m_CythonRecipe().strip_object_files.called is False
+
+            # Make sure strip object files IS called when release build:
+            ctx.build_as_debuggable = False
+            assert run_pymodules_install(ctx, modules, project_dir) is None
+            assert m_CythonRecipe().strip_object_files.called is True