Skip to content

Allow configuring ndk build architectures #31232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Allow configuring ndk build architectures #31232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81a93ad
Allow configuring ndk build architectures
janicduplessis Mar 25, 2021
0f0cd66
Update template
janicduplessis Jul 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ReactAndroid/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,14 @@ def reactNativeInspectorProxyPort() {
return value != null ? value : reactNativeDevServerPort()
}

def reactNativeArchitectures() {
def isDebug = gradle.startParameter.taskRequests.any {
it.args.any { it.endsWith("Debug") }
}
def value = project.getProperties().get("reactNativeDebugArchitectures")
return value != null && isDebug ? value : "all"
Copy link
Contributor

@ShikaSD ShikaSD Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "all" value valid for the abiFilters block? From the documentation, I only see it used for ndk-build.

Copy link
Contributor Author

@janicduplessis janicduplessis Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is only used with ndk-build so the all value is fine.

}

def getNdkBuildFullPath() {
def ndkBuildFullPath = findNdkBuildFullPath()
if (ndkBuildFullPath == null) {
Expand Down Expand Up @@ -355,6 +363,7 @@ def buildReactNdkLib = tasks.register("buildReactNdkLib", Exec) {
inputs.dir("src/main/java/com/facebook/react/modules/blob")
outputs.dir("$buildDir/react-ndk/all")
commandLine(getNdkBuildFullPath(),
"APP_ABI=${reactNativeArchitectures()}",
Copy link
Contributor

@ShikaSD ShikaSD Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe safeguard the debug version here as well? 🙂

Copy link
Contributor Author

@janicduplessis janicduplessis Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed since reactNativeArchitectures does the debug check already and is only used here. It will always return 'all' for non-debug builds.

Copy link
Contributor

@ShikaSD ShikaSD Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, my intention is to avoid hacking the debug check based on the task name and use flags from the build task itself, that's why I am asking about it :)

Copy link
Contributor Author

@janicduplessis janicduplessis Jul 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I tried finding a better way to detect wether it was a debug or release build but could not find it. The task is the same for both build types so at that point from what I could see it is not possible to know without the gradle invocation args.

One more involved solution that I could see is create a different task per variant for buildReactNdkLib, but not sure if that would be worth changing for this.

Copy link
Contributor

@ShikaSD ShikaSD Jul 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see it now, didn't realize we always compile the build with NDK_DEBUG set to 0. I guess it doesn't matter much for this module now, as we build everything on the CI anyways and distribute binaries only.

"NDK_DEBUG=" + (nativeBuildType.equalsIgnoreCase("debug") ? "1" : "0"),
"NDK_PROJECT_PATH=null",
"NDK_APPLICATION_MK=$projectDir/src/main/jni/Application.mk",
Expand Down
11 changes: 10 additions & 1 deletion packages/rn-tester/android/app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ def enableFabric = project.ext.react.enableFabric
*/
def useIntlJsc = false

/**
* Architectures to build native code for in debug.
*/
def nativeArchitectures = project.getProperties().get("reactNativeDebugArchitectures")

android {
compileSdkVersion 29
ndkVersion ANDROID_NDK_VERSION
Expand Down Expand Up @@ -176,6 +181,11 @@ android {
debug {
debuggable true
signingConfig signingConfigs.release
if (nativeArchitectures) {
ndk {
abiFilters nativeArchitectures.split(',')
}
}
}
release {
debuggable false
Expand Down Expand Up @@ -251,7 +261,6 @@ if (enableCodegen) {
defaultConfig {
externalNativeBuild {
ndkBuild {
abiFilters "armeabi-v7a", "x86", "x86_64", "arm64-v8a"
Copy link
Contributor Author

@janicduplessis janicduplessis Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default already according to https://developer.android.com/ndk/guides/abis. This will use the project wide config added a few lines up.

arguments "APP_PLATFORM=android-21",
"APP_STL=c++_shared",
"NDK_TOOLCHAIN_VERSION=clang",
Expand Down
10 changes: 10 additions & 0 deletions template/android/app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ def jscFlavor = 'org.webkit:android-jsc:+'
*/
def enableHermes = project.ext.react.get("enableHermes", false);

/**
* Architectures to build native code for in debug.
*/
def nativeArchitectures = project.getProperties().get("reactNativeDebugArchitectures")

android {
ndkVersion rootProject.ext.ndkVersion

Expand Down Expand Up @@ -151,6 +156,11 @@ android {
buildTypes {
debug {
signingConfig signingConfigs.debug
if (nativeArchitectures) {
ndk {
abiFilters nativeArchitectures.split(',')
}
}
}
release {
// Caution! In production, you need to generate your own keystore file.
Expand Down