Update NDK 25 LTS (r25b) #7240

AGulev · 2022-12-23T15:10:31Z

Defold updated from NDK r20 to NDK r25b - the latesNDK version at the moment.
Now minimum Android version API is 19 (Android 4.4)

Fix #7160

…Unwind_GetIP`

… unwinding

…` and `_Unwind_GetIP`" This reverts commit e790b25.

AGulev · 2022-12-23T15:10:43Z

PR checklist (remove before merging)

Prepared the PR for release notes
- Proper description of feature or fix (see example x?)
- Added a #fixes ### if it fixes an issue
- Assigned issue to a project
Added documentation for public API changes
Is this a new feature?
- Added engine and/or editor unit tests
- Added or updated manual entry

AGulev · 2022-12-23T15:10:55Z

Useful notes https://docs.google.com/document/d/1uguu07VuUvBRQvJ4T_NnrfNl7q8JHZD1LF7IuePkhy4/edit#

AGulev · 2022-12-23T15:15:03Z

build_tools/waf_package.py

@@ -41,51 +36,6 @@ def glob_files(patterns, cwd=None):
        os.chdir(oldcwd)
    return out

-# Original from  wafadmin/Build.py install_files:
-# I couldn't get the original code to work as I needed, so I made a small change to it.
-def package_install_files(self, path, files, env=None, chmod=Constants.O644, relative_trick=False, cwd=None):


new waf doesn't have this function
it is possible to implement it using default waf functionality

AGulev · 2022-12-23T15:15:33Z

editor/resources/meta.edn

@@ -642,7 +642,7 @@
   :path ["android" "version_code"]}
  {:type :integer,
   :help "minimum API Level required for the application to run (android:minSdkVersion)",
-   :default 16,


Minimum android version increased to 19

AGulev · 2022-12-23T15:16:32Z

engine/crash/src/wscript

@@ -20,7 +20,7 @@ def build(bld):
                          target = 'crashext_null')

    if 'android' in bld.env['PLATFORM']:
-        sig_handler = 'backtrace_libunwind.cpp'


New backtrace unwinder implemented for Android. It uses clang libunwind

ANd, now we have two files using the libunwind library.
I think you need to rename one.

Note that "lib" is implicit for library names. I.e. "libunwind" == "the unwind library"

we use libunwind from NDK on Android and modified libunwind from Google for macos (because it's a bit better and can get symbols from DWARF).
What do you think is the best name backtrace_libunwind_ndk.cpp?

AGulev · 2022-12-23T15:17:54Z

external/bullet3d/wscript

+    bld.install_files('${PREFIX}/%s-%s/lib/%s/' % (APPNAME, VERSION, bld.env.PLATFORM), bld.env.cstlib_PATTERN % 'BulletCollision')
+    bld.install_files('${PREFIX}/%s-%s/lib/%s/' % (APPNAME, VERSION, bld.env.PLATFORM), bld.env.cstlib_PATTERN % 'BulletDynamics')
+    bld.install_files('${PREFIX}/%s-%s/lib/%s/' % (APPNAME, VERSION, bld.env.PLATFORM), bld.env.cstlib_PATTERN % 'LinearMath')
+    bld.install_files('${PREFIX}/%s-%s/include' % (APPNAME, VERSION), bld.path.ant_glob(['**/*.h']), cwd=bld.path.find_dir(packagedir), relative_trick=True)


it works with folders hierarhy this way

AGulev · 2022-12-23T15:19:10Z

scripts/build.py

@@ -605,7 +605,7 @@ def install_sdk(self):
                host = 'darwin' # our packages are still called darwin

            # Android NDK
-            download_sdk(self, '%s/%s-%s-x86_64.tar.gz' % (self.package_path, PACKAGES_ANDROID_NDK, host), join(sdkfolder, PACKAGES_ANDROID_NDK))


the original file on google's side doesn't use-x86_64 postfix, I removed it on our side as well

AGulev · 2022-12-23T15:20:50Z

scripts/package/package_android_ndk.sh

@@ -63,7 +62,7 @@ if [ ! -e "${TARGET_PATH}/${ANDROID_NDK_BASENAME}.tar.gz" ]; then
 	# Cleanup the package, shrinking 2.9GB down to 1.6GB (extracted)
 	echo "Cleaning NDK" ${TMP}/${ANDROID_NDK}

-	(cd ${TMP} && rm -rf ${ANDROID_NDK}/prebuilt)
+	# keep: (cd ${TMP} && rm -rf ${ANDROID_NDK}/prebuilt) -- it's easier to rebuild packages using NDKs cmake


it is a pretty small folder (~7mb), but having it helps to rebuild packages easier

But for those things, we could as easily require the developer to have the NDK installed, and ANDROID_HOME etc setup?

We'll support that soon anyways so that they don't have to create/install this package. So I think that it's still something to think about, since the Docker disc space is still an issue.

I can do removing of that folder on extender side in Dockerfile then in my PR

AGulev · 2022-12-23T15:21:29Z

share/ext/common.sh

 ANDROID_NDK_ROOT=${DYNAMO_HOME}/ext/SDKs/android-ndk-r${ANDROID_NDK_VERSION}

-ANDROID_VERSION=16 # Android 4.1
-ANDROID_GCC_VERSION='4.9'


no more gcc tools in ndk25

AGulev · 2022-12-23T15:23:10Z

share/ext/protobuf/build_protobuf.sh

+        ;;
+
+        arm64-android)
+        export LDFLAGS="$LDFLAGS -llog"


this flag is needed for rebuilding protobuf for android

AGulev · 2022-12-23T15:25:20Z

share/extender/build_input.yml

        MANIFEST_MERGE_TOOL:    "{{env.MANIFEST_MERGE_TOOL}}"
+        PATH:                   "{{env.ANDROID_NDK25_BIN_PATH}}:{{env.PATH}}"
+        NDK_LD:                 "{{env.ANDROID_NDK25_BIN_PATH}}/ld.lld"
+        NDK_CXX:                "{{env.ANDROID_NDK25_BIN_PATH}}/armv7a-linux-androideabi{{env.ANDROID_NDK25_API_VERSION}}-clang++"


Taking into account specific aspects of how ProcessBuilder works, it’s important to use absolute path for the compiler. If a relative path is used, then subprocesses of the compiler are looking for the InstalledDir in PATH variable and may find a wrong one (from another SDK). For the absolute path InstalledDir detects from it.

AGulev · 2022-12-23T15:26:06Z

share/extender/build_input.yml

        MANIFEST_MERGE_TOOL:    "{{env.MANIFEST_MERGE_TOOL}}"
+        PATH:                   "{{env.ANDROID_NDK25_BIN_PATH}}:{{env.PATH}}"
+        NDK_LD:                 "{{env.ANDROID_NDK25_BIN_PATH}}/ld.lld"


To make sure the compiler uses the linker I expected I use absolute path for -fuse-ld.
It’s not obvious from the official clang documentation that it is possible to use absolute path, but here is a mention about this possibility.

JCash

LGTM, only issue is with the name of the new crash handler file.

JCash · 2023-01-09T08:40:39Z

build_tools/waf_dynamo.py

@@ -872,8 +868,7 @@ def _strip_executable(bld, platform, target_arch, path):
    if 'android' in platform:
        HOME = os.environ['USERPROFILE' if sys.platform == 'win32' else 'HOME']
        ANDROID_HOST = 'linux' if sys.platform == 'linux' else 'darwin'
-        build_tool = getAndroidBuildtoolName(target_arch)
-        strip = "%s/toolchains/%s-%s/prebuilt/%s-x86_64/bin/%s-strip" % (ANDROID_NDK_ROOT, build_tool, ANDROID_GCC_VERSION, ANDROID_HOST, build_tool)
+        strip = "%s/toolchains/llvm/prebuilt/%s-x86_64/bin/llvm-strip" % (ANDROID_NDK_ROOT, ANDROID_HOST)


Does the 64 bit stripnowwork on armv7` executables?

I can't find docs for it (it's always isn't so easy) but I tested it and it works

JCash · 2023-01-09T08:43:56Z

engine/crash/src/backtrace_unwind.cpp

@@ -0,0 +1,172 @@
+// Copyright 2020-2022 The Defold Foundation


Now it has the same name as the macos version, "libunwind" which is very confusing.

JCash · 2023-01-09T08:45:16Z

engine/crash/src/wscript

@@ -20,7 +20,7 @@ def build(bld):
                          target = 'crashext_null')

    if 'android' in bld.env['PLATFORM']:
-        sig_handler = 'backtrace_libunwind.cpp'


ANd, now we have two files using the libunwind library.
I think you need to rename one.

Note that "lib" is implicit for library names. I.e. "libunwind" == "the unwind library"

JCash · 2023-01-09T08:50:30Z

scripts/package/package_android_ndk.sh

@@ -63,7 +62,7 @@ if [ ! -e "${TARGET_PATH}/${ANDROID_NDK_BASENAME}.tar.gz" ]; then
 	# Cleanup the package, shrinking 2.9GB down to 1.6GB (extracted)
 	echo "Cleaning NDK" ${TMP}/${ANDROID_NDK}

-	(cd ${TMP} && rm -rf ${ANDROID_NDK}/prebuilt)
+	# keep: (cd ${TMP} && rm -rf ${ANDROID_NDK}/prebuilt) -- it's easier to rebuild packages using NDKs cmake


But for those things, we could as easily require the developer to have the NDK installed, and ANDROID_HOME etc setup?

We'll support that soon anyways so that they don't have to create/install this package. So I think that it's still something to think about, since the Docker disc space is still an issue.

JCash · 2023-01-09T08:52:09Z

share/ext/libunwind/jni/Application.mk

+APP_ABI := arm64-v8a #armeabi-v7a
 APP_CXXFLAGS := -std=c++11 -frtti -D__STDC_LIMIT_MACROS -D_THREAD_SAFE
-APP_PLATFORM := android-9
+APP_PLATFORM := android-19


Is this file even used now that we use the libunwind from the NDK?

no, but I would like to keep it for now, because it will be useful for libunwind**stack** building
see Libunwind
in https://docs.google.com/document/d/1uguu07VuUvBRQvJ4T_NnrfNl7q8JHZD1LF7IuePkhy4/edit#heading=h.mfeghlxitqk0

AGulev · 2023-01-11T08:30:46Z

@JCash thank you for review!
This PR depends on extender's PR, pls take a look when you have time defold/extender#253

# Conflicts: # share/extender/build_input.yml

AGulev added 18 commits November 23, 2022 16:20

fix script

fc9b93c

initial paths fixes

e807c5c

use lld ref

9c5dfe3

keep NDKs make file

b2d2095

mk for libunwind fixes

5ae8c24

draft implementation of the unwinder using _Unwind_Backtrace and `_…

e790b25

…Unwind_GetIP`

add _Unwind_Backtrace + _Unwind_GetIP implementation of the stack…

3573dbe

… unwinding

build luajit with ndk 25

6202862

rebuild bullet for android

8310ba7

rebuild protobuf for android

ecd3d20

Revert "draft implementation of the unwinder using `_Unwind_Backtrace…

0c82dce

…` and `_Unwind_GetIP`" This reverts commit e790b25.

fix LD arguments for protobuf on Android

78346b9

fix extender constants

bf486a5

make sure this dmSDK uses NDK25b first

751a15e

don't need to link STL manually anymore

74f9dd7

use llvm-ar tools instead of GCC ar

a9ddabb

fix build_input for ndk25

dfe6d9c

use NDK25 instead of NDK25b for enviroment variables

c068d8c

AGulev requested a review from JCash December 23, 2022 15:10

AGulev commented Dec 23, 2022

View reviewed changes

AGulev added breaking change Issue introducing a breaking change android Issue related to the Android platform labels Dec 23, 2022

JCash requested changes Jan 9, 2023

View reviewed changes

use libunwind_ndk name

4ceef84

AGulev requested a review from JCash January 9, 2023 12:29

JCash previously approved these changes Jan 10, 2023

View reviewed changes

Merge branch 'dev' into update-to-ndk-r25-lts

6b1c49c

AGulev self-assigned this Jan 13, 2023

AGulev added 2 commits January 16, 2023 16:31

Merge branch 'dev' into update-to-ndk-r25-lts

467a6fc

Merge branch 'dev' into update-to-ndk-r25-lts

9ab11f9

# Conflicts: # share/extender/build_input.yml

AGulev dismissed JCash’s stale review via 9ab11f9 January 16, 2023 15:35

AGulev merged commit a9d41b8 into dev Jan 16, 2023

AGulev deleted the update-to-ndk-r25-lts branch January 16, 2023 16:29

AGulev removed the breaking change Issue introducing a breaking change label Jan 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update NDK 25 LTS (r25b) #7240

Update NDK 25 LTS (r25b) #7240

AGulev commented Dec 23, 2022 •

edited

AGulev commented Dec 23, 2022 •

edited

AGulev commented Dec 23, 2022

AGulev Dec 23, 2022

AGulev Dec 23, 2022

AGulev Dec 23, 2022

JCash Jan 9, 2023

AGulev Jan 9, 2023

AGulev Dec 23, 2022

AGulev Dec 23, 2022

AGulev Dec 23, 2022

JCash Jan 9, 2023

AGulev Jan 9, 2023

AGulev Dec 23, 2022

AGulev Dec 23, 2022

AGulev Dec 23, 2022

AGulev Dec 23, 2022

JCash left a comment

JCash Jan 9, 2023

AGulev Jan 9, 2023

JCash Jan 9, 2023

JCash Jan 9, 2023

JCash Jan 9, 2023

JCash Jan 9, 2023

AGulev Jan 9, 2023

AGulev commented Jan 11, 2023

		@@ -0,0 +1,172 @@
		// Copyright 2020-2022 The Defold Foundation

Update NDK 25 LTS (r25b) #7240

Update NDK 25 LTS (r25b) #7240

Conversation

AGulev commented Dec 23, 2022 • edited

AGulev commented Dec 23, 2022 • edited

PR checklist (remove before merging)

AGulev commented Dec 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JCash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AGulev commented Jan 11, 2023

AGulev commented Dec 23, 2022 •

edited

AGulev commented Dec 23, 2022 •

edited