Skip to content

Commit

Permalink
Make sure the -DANDROID compilation flag is always included. (#37451)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #37451

It seems like starting from AGP 7.4, Android is not including the `-DANDROID` flag
anymore from the NDK toolchain.

As we do have several `#ifdef` logic that takes care of having this macro set,
I'm going to make sure it's always set for both ReactAndroid, hermes-engine
and the template.

Changelog:
[Android] [Fixed] - Make sure the -DANDROID compilation flag is always included

Reviewed By: javache

Differential Revision: D45908787

fbshipit-source-id: 07284712d7bcce73dc8ea0dffd4a9d00af4de1d2
  • Loading branch information
cortinico authored and facebook-github-bot committed May 17, 2023
1 parent f7dcc65 commit 3a321ae
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,22 @@ internal object NdkConfiguratorUtils {
// Parameters should be provided in an additive manner (do not override what
// the user provided, but allow for sensible defaults).
val cmakeArgs = ext.defaultConfig.externalNativeBuild.cmake.arguments
if ("-DPROJECT_BUILD_DIR" !in cmakeArgs) {
if (cmakeArgs.none { it.startsWith("-DPROJECT_BUILD_DIR") }) {
cmakeArgs.add("-DPROJECT_BUILD_DIR=${project.buildDir}")
}
if ("-DREACT_ANDROID_DIR" !in cmakeArgs) {
if (cmakeArgs.none { it.startsWith("-DREACT_ANDROID_DIR") }) {
cmakeArgs.add(
"-DREACT_ANDROID_DIR=${extension.reactNativeDir.file("ReactAndroid").get().asFile}")
}
if ("-DANDROID_STL" !in cmakeArgs) {
if (cmakeArgs.none { it.startsWith("-DANDROID_STL") }) {
cmakeArgs.add("-DANDROID_STL=c++_shared")
}
// Due to the new NDK toolchain file, the C++ flags gets overridden between compilation
// units. This is causing some libraries to don't be compiled with -DANDROID and other
// crucial flags. This can be revisited once we bump to NDK 25/26
if (cmakeArgs.none { it.startsWith("-DANDROID_USE_LEGACY_TOOLCHAIN_FILE") }) {
cmakeArgs.add("-DANDROID_USE_LEGACY_TOOLCHAIN_FILE=ON")
}

val architectures = project.getReactNativeArchitectures()
// abiFilters are split ABI are not compatible each other, so we set the abiFilters
Expand Down
5 changes: 4 additions & 1 deletion packages/react-native/ReactAndroid/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,10 @@ android {
"-DREACT_BUILD_DIR=$buildDir",
"-DANDROID_STL=c++_shared",
"-DANDROID_TOOLCHAIN=clang",
"-DANDROID_PLATFORM=android-21"
"-DANDROID_PLATFORM=android-21",
// Due to https://github.com/android/ndk/issues/1693 we're losing Android
// specific compilation flags. This can be removed once we moved to NDK 25/26
"-DANDROID_USE_LEGACY_TOOLCHAIN_FILE=ON"

targets "jsijniprofiler",
"reactnativeblob",
Expand Down
3 changes: 3 additions & 0 deletions packages/react-native/ReactAndroid/hermes-engine/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ android {
// We intentionally build Hermes with Intl support only. This is to simplify
// the build setup and to avoid overcomplicating the build-type matrix.
arguments "-DHERMES_ENABLE_INTL=True"
// Due to https://github.com/android/ndk/issues/1693 we're losing Android
// specific compilation flags. This can be removed once we moved to NDK 25/26
arguments "-DANDROID_USE_LEGACY_TOOLCHAIN_FILE=ON"
targets "libhermes"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ add_executable(reactnative_unittest
-fexceptions
-frtti
-std=c++17
-DANDROID
-DHERMES_ENABLE_DEBUGGER)

target_link_libraries(reactnative_unittest
Expand Down

0 comments on commit 3a321ae

Please sign in to comment.