From c3a48c8fa0cf53da03be039e5841e69ca2229836 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 24 Nov 2022 07:11:58 -0800 Subject: [PATCH 1/5] Consolidate hermes-executor-debug and -release inside a single target Summary: Historically, we used to have hermes-executor debug and release as separate dynamic libraries. This makes it impossible to prefab this library, so I have to reconcile it into a single library. This will also help keep the setup consistent with the internal (BUCK) where we have a single target. Changelog: [Internal] [Changed] - Consolidate hermes-executor-debug and -release inside a single target Differential Revision: D41519119 fbshipit-source-id: 162640dd424253d8d5645602ad764397fad08127 --- ReactAndroid/build.gradle | 19 +----- .../com/facebook/hermes/reactexecutor/BUCK | 1 + .../hermes/reactexecutor/HermesExecutor.java | 9 +-- .../react/hermes/reactexecutor/CMakeLists.txt | 17 ++---- .../com/facebook/react/TaskConfiguration.kt | 3 +- .../react/utils/NdkConfiguratorUtils.kt | 19 ++---- .../react/utils/NdkConfiguratorUtilsTest.kt | 58 +++---------------- 7 files changed, 24 insertions(+), 102 deletions(-) diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index d43535934995e3..f14aa4d2352e99 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -404,6 +404,7 @@ android { targets "reactnativejni", "jscexecutor", + "hermes-executor", "jsijniprofiler", "reactnativeblob", "reactperfloggerjni", @@ -434,24 +435,6 @@ android { } } - buildTypes { - debug { - externalNativeBuild { - cmake { - targets "hermes-executor-debug" - } - } - } - - release { - externalNativeBuild { - cmake { - targets "hermes-executor-release" - } - } - } - } - externalNativeBuild { cmake { path "src/main/jni/CMakeLists.txt" diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/BUCK b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/BUCK index 0c00d480eddb44..2c954374c38dcf 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/BUCK @@ -17,6 +17,7 @@ rn_android_library( react_native_target("java/com/facebook/hermes/instrumentation:instrumentation"), react_native_target("java/com/facebook/hermes/instrumentation:hermes_samplingprofiler"), react_native_target("java/com/facebook/react/bridge:bridge"), + react_native_target("java/com/facebook/react/common:common"), react_native_target("jni/react/hermes/reactexecutor:jni"), ":runtimeconfig", ], diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java index 48eaaa4c0668ba..e72bca607c1c96 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java @@ -9,6 +9,7 @@ import com.facebook.jni.HybridData; import com.facebook.react.bridge.JavaScriptExecutor; +import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.soloader.SoLoader; import javax.annotation.Nullable; @@ -23,11 +24,11 @@ public static void loadLibrary() throws UnsatisfiedLinkError { if (mode_ == null) { // libhermes must be loaded explicitly to invoke its JNI_OnLoad. SoLoader.loadLibrary("hermes"); - try { - SoLoader.loadLibrary("hermes-executor-debug"); + SoLoader.loadLibrary("hermes-executor"); + // libhermes-executor is built differently for Debug & Release so we load the proper mode. + if (ReactBuildConfig.DEBUG == true) { mode_ = "Debug"; - } catch (UnsatisfiedLinkError e) { - SoLoader.loadLibrary("hermes-executor-release"); + } else { mode_ = "Release"; } } diff --git a/ReactAndroid/src/main/jni/react/hermes/reactexecutor/CMakeLists.txt b/ReactAndroid/src/main/jni/react/hermes/reactexecutor/CMakeLists.txt index 3145a733fe5c4d..fcf9f93e4c417d 100644 --- a/ReactAndroid/src/main/jni/react/hermes/reactexecutor/CMakeLists.txt +++ b/ReactAndroid/src/main/jni/react/hermes/reactexecutor/CMakeLists.txt @@ -8,29 +8,20 @@ set(CMAKE_VERBOSE_MAKEFILE on) file(GLOB_RECURSE hermes_executor_SRC CONFIGURE_DEPENDS *.cpp) -if(${CMAKE_BUILD_TYPE} MATCHES Debug) - set(HERMES_TARGET_SUFFIX debug) -else() - set(HERMES_TARGET_SUFFIX release) -endif() - -set(HERMES_TARGET_NAME hermes-executor-${HERMES_TARGET_SUFFIX}) - -add_library( - ${HERMES_TARGET_NAME} +add_library(hermes-executor SHARED ${hermes_executor_SRC} ) target_compile_options( - ${HERMES_TARGET_NAME} + hermes-executor PRIVATE $<$:-DHERMES_ENABLE_DEBUGGER=1> -std=c++17 -fexceptions ) -target_include_directories(${HERMES_TARGET_NAME} PRIVATE .) +target_include_directories(hermes-executor PRIVATE .) target_link_libraries( - ${HERMES_TARGET_NAME} + hermes-executor hermes-executor-common jsireact fb diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt index bb3699d58df095..a2b60f425794d8 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt @@ -46,8 +46,7 @@ internal fun Project.configureReactTasks(variant: Variant, config: ReactExtensio config.debuggableVariants.get().any { it.equals(variant.name, ignoreCase = true) } configureNewArchPackagingOptions(project, variant) - configureJsEnginePackagingOptions( - config, variant, isHermesEnabledInThisVariant, isDebuggableVariant) + configureJsEnginePackagingOptions(config, variant, isHermesEnabledInThisVariant) if (!isDebuggableVariant) { val bundleTask = diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt index 479c3f742eda99..13bbb3ab8c51dd 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt @@ -112,36 +112,25 @@ internal object NdkConfiguratorUtils { config: ReactExtension, variant: Variant, hermesEnabled: Boolean, - debuggableVariant: Boolean ) { if (config.enableSoCleanup.get()) { - val (excludes, includes) = getPackagingOptionsForVariant(hermesEnabled, debuggableVariant) + val (excludes, includes) = getPackagingOptionsForVariant(hermesEnabled) variant.packaging.jniLibs.excludes.addAll(excludes) variant.packaging.jniLibs.pickFirsts.addAll(includes) } } - fun getPackagingOptionsForVariant( - hermesEnabled: Boolean, - debuggableVariant: Boolean - ): Pair, List> { + fun getPackagingOptionsForVariant(hermesEnabled: Boolean): Pair, List> { val excludes = mutableListOf() val includes = mutableListOf() if (hermesEnabled) { excludes.add("**/libjsc.so") excludes.add("**/libjscexecutor.so") includes.add("**/libhermes.so") - if (debuggableVariant) { - excludes.add("**/libhermes-executor-release.so") - includes.add("**/libhermes-executor-debug.so") - } else { - excludes.add("**/libhermes-executor-debug.so") - includes.add("**/libhermes-executor-release.so") - } + includes.add("**/libhermes-executor.so") } else { excludes.add("**/libhermes.so") - excludes.add("**/libhermes-executor-debug.so") - excludes.add("**/libhermes-executor-release.so") + excludes.add("**/libhermes-executor.so") includes.add("**/libjsc.so") includes.add("**/libjscexecutor.so") } diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/NdkConfiguratorUtilsTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/NdkConfiguratorUtilsTest.kt index 2d0093c6a565d2..41d498b23dccec 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/NdkConfiguratorUtilsTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/NdkConfiguratorUtilsTest.kt @@ -15,70 +15,28 @@ import org.junit.Test class NdkConfiguratorUtilsTest { @Test - fun getPackagingOptionsForVariant_withHermesEnabled_andDebuggableVariant() { - val (excludes, includes) = - getPackagingOptionsForVariant(hermesEnabled = true, debuggableVariant = true) + fun getPackagingOptionsForVariant_withHermesEnabled() { + val (excludes, includes) = getPackagingOptionsForVariant(hermesEnabled = true) assertTrue("**/libjsc.so" in excludes) assertTrue("**/libjscexecutor.so" in excludes) - assertTrue("**/libhermes-executor-release.so" in excludes) assertFalse("**/libjsc.so" in includes) assertFalse("**/libjscexecutor.so" in includes) - assertFalse("**/libhermes-executor-release.so" in includes) assertTrue("**/libhermes.so" in includes) - assertTrue("**/libhermes-executor-debug.so" in includes) + assertTrue("**/libhermes-executor.so" in includes) assertFalse("**/libhermes.so" in excludes) - assertFalse("**/libhermes-executor-debug.so" in excludes) + assertFalse("**/libhermes-executor.so" in excludes) } @Test - fun getPackagingOptionsForVariant_withHermesEnabled_andNonDebuggableVariant() { - val (excludes, includes) = - getPackagingOptionsForVariant(hermesEnabled = true, debuggableVariant = false) - - assertTrue("**/libjsc.so" in excludes) - assertTrue("**/libjscexecutor.so" in excludes) - assertTrue("**/libhermes-executor-debug.so" in excludes) - assertFalse("**/libjsc.so" in includes) - assertFalse("**/libjscexecutor.so" in includes) - assertFalse("**/libhermes-executor-debug.so" in includes) - - assertTrue("**/libhermes.so" in includes) - assertTrue("**/libhermes-executor-release.so" in includes) - assertFalse("**/libhermes.so" in excludes) - assertFalse("**/libhermes-executor-release.so" in excludes) - } - - @Test - fun getPackagingOptionsForVariant_withHermesDisabled_andDebuggableVariant() { - val (excludes, includes) = - getPackagingOptionsForVariant(hermesEnabled = false, debuggableVariant = true) - - assertTrue("**/libhermes.so" in excludes) - assertTrue("**/libhermes-executor-debug.so" in excludes) - assertTrue("**/libhermes-executor-release.so" in excludes) - assertFalse("**/libhermes.so" in includes) - assertFalse("**/libhermes-executor-debug.so" in includes) - assertFalse("**/libhermes-executor-release.so" in includes) - - assertTrue("**/libjsc.so" in includes) - assertTrue("**/libjscexecutor.so" in includes) - assertFalse("**/libjsc.so" in excludes) - assertFalse("**/libjscexecutor.so" in excludes) - } - - @Test - fun getPackagingOptionsForVariant_withHermesDisabled_andNonDebuggableVariant() { - val (excludes, includes) = - getPackagingOptionsForVariant(hermesEnabled = false, debuggableVariant = false) + fun getPackagingOptionsForVariant_withHermesDisabled() { + val (excludes, includes) = getPackagingOptionsForVariant(hermesEnabled = false) assertTrue("**/libhermes.so" in excludes) - assertTrue("**/libhermes-executor-debug.so" in excludes) - assertTrue("**/libhermes-executor-release.so" in excludes) + assertTrue("**/libhermes-executor.so" in excludes) assertFalse("**/libhermes.so" in includes) - assertFalse("**/libhermes-executor-debug.so" in includes) - assertFalse("**/libhermes-executor-release.so" in includes) + assertFalse("**/libhermes-executor.so" in includes) assertTrue("**/libjsc.so" in includes) assertTrue("**/libjscexecutor.so" in includes) From e3ae131e2e400fda5a030ee73ad47e73e5a76efe Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 24 Nov 2022 07:11:58 -0800 Subject: [PATCH 2/5] Expose `hermes-executor` to be consumed via prefab Summary: This exposes `hermes-executor` to be consumed via prefab so that libraries can depend on it and use its symbols if needed (Expo and Reanimated need it). Changelog: [Internal] [Changed] - Expose `hermes-executor` to be consumed via prefab Differential Revision: D41520019 fbshipit-source-id: 79e7bea4e9da5f49396bbc64ff2f3b9d6acd8cdb --- ReactAndroid/build.gradle | 14 ++++++++++++-- ReactCommon/hermes/executor/CMakeLists.txt | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index f14aa4d2352e99..1bf06ae88265f6 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -174,6 +174,13 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa "rrc_image", new Pair("../ReactCommon/react/renderer/components/image/", "react/renderer/components/image/") ), + // This prefab target is used by Expo & Reanimated to load a new instance of Hermes + new PrefabPreprocessingEntry( + "hermes-executor", + // "hermes-executor" is statically linking agaisnt "hermes-executor-common" + // and "hermes-inspector". Here we expose only the headers that we know are needed. + new Pair("../ReactCommon/hermes/inspector/", "hermes/inspector/") + ), ] ) it.outputDir.set(prefabHeadersDir) @@ -404,7 +411,6 @@ android { targets "reactnativejni", "jscexecutor", - "hermes-executor", "jsijniprofiler", "reactnativeblob", "reactperfloggerjni", @@ -427,7 +433,8 @@ android { "yoga", "folly_runtime", "react_nativemodule_core", - "react_render_imagemanager" + "react_render_imagemanager", + "hermes-executor" } } ndk { @@ -532,6 +539,9 @@ android { react_render_imagemanager { headers(new File(prefabHeadersDir, "react_render_imagemanager").absolutePath) } + "hermes-executor" { + headers(new File(prefabHeadersDir, "hermes-executor").absolutePath) + } } publishing { diff --git a/ReactCommon/hermes/executor/CMakeLists.txt b/ReactCommon/hermes/executor/CMakeLists.txt index b6575a08e19937..4a2f54b5984191 100644 --- a/ReactCommon/hermes/executor/CMakeLists.txt +++ b/ReactCommon/hermes/executor/CMakeLists.txt @@ -19,7 +19,8 @@ target_link_libraries(hermes-executor-common jsireact hermes-engine::libhermes jsi - debug hermes-inspector + debug + hermes-inspector ) if(${CMAKE_BUILD_TYPE} MATCHES Debug) From a0c5d09b94afe8fa65e8791b1cd5cc36d073ffa7 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 24 Nov 2022 07:11:58 -0800 Subject: [PATCH 3/5] Add prefab for _uimanager _scheduler and _mounting Summary: We're adding prefab support for those modules as they're needed by Reanimated and we're exposing headers for them as well. Changelog: [Internal] [Changed] - Add prefab for _uimanager _scheduler and _mounting Differential Revision: D41520606 fbshipit-source-id: 265ccbc79c2f73a47a41e70eff8cbe0ae473bea0 --- ReactAndroid/build.gradle | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index 1bf06ae88265f6..74051746fc7208 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -181,6 +181,18 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa // and "hermes-inspector". Here we expose only the headers that we know are needed. new Pair("../ReactCommon/hermes/inspector/", "hermes/inspector/") ), + new PrefabPreprocessingEntry( + "react_render_uimanager", + new Pair("../ReactCommon/react/renderer/uimanager/", "react/renderer/uimanager/"), + ), + new PrefabPreprocessingEntry( + "react_render_scheduler", + new Pair("../ReactCommon/react/renderer/scheduler/", "react/renderer/scheduler/"), + ), + new PrefabPreprocessingEntry( + "react_render_mounting", + new Pair("../ReactCommon/react/renderer/mounting/", "react/renderer/mounting/"), + ), ] ) it.outputDir.set(prefabHeadersDir) @@ -434,6 +446,9 @@ android { "folly_runtime", "react_nativemodule_core", "react_render_imagemanager", + "react_render_uimanager", + "react_render_scheduler", + "react_render_mounting", "hermes-executor" } } @@ -539,6 +554,15 @@ android { react_render_imagemanager { headers(new File(prefabHeadersDir, "react_render_imagemanager").absolutePath) } + react_render_uimanager { + headers(new File(prefabHeadersDir, "react_render_uimanager").absolutePath) + } + react_render_scheduler { + headers(new File(prefabHeadersDir, "react_render_scheduler").absolutePath) + } + react_render_mounting { + headers(new File(prefabHeadersDir, "react_render_mounting").absolutePath) + } "hermes-executor" { headers(new File(prefabHeadersDir, "hermes-executor").absolutePath) } From acc99d5a42808ddc3d4fc2e55fd7344bc770fe2a Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 24 Nov 2022 07:11:58 -0800 Subject: [PATCH 4/5] Add missing headers to `react_nativemodule_core` prefab module Summary: Reanimated reported that `react_nativemodule_core` was missing some headers. Specifically the one from ReactAndroid::react_debug, ReactAndroid::react_render_core, ReactAndroid::glog, and ReactAndroid::react_render_debug. I'm adding them here so they get included in the shipped headers for `react_nativemodule_core` Changelog: [Internal] [Changed] - Add missing headers to `react_nativemodule_core` prefab module Differential Revision: D41520751 fbshipit-source-id: ded06ea56275a63e3009d9fd99e2c8e78181937b --- ReactAndroid/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index 74051746fc7208..e4f28debed1481 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -144,6 +144,7 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa new Pair(new File(buildDir, "third-party-ndk/boost/boost_1_76_0/").absolutePath, ""), new Pair(new File(buildDir, "third-party-ndk/double-conversion/").absolutePath, ""), new Pair(new File(buildDir, "third-party-ndk/folly/").absolutePath, ""), + new Pair(new File(buildDir, "third-party-ndk/glog/exported/").absolutePath, ""), new Pair("../ReactCommon/butter/", "butter/"), new Pair("../ReactCommon/callinvoker/", ""), new Pair("../ReactCommon/react/bridging/", "react/bridging/"), @@ -152,6 +153,8 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa new Pair("../ReactCommon/react/nativemodule/core/platform/android/", ""), new Pair("../ReactCommon/react/renderer/componentregistry/", "react/renderer/componentregistry/"), new Pair("../ReactCommon/react/renderer/components/root/", "react/renderer/components/root/"), + new Pair("../ReactCommon/react/renderer/core/", "react/renderer/core/"), + new Pair("../ReactCommon/react/renderer/debug/", "react/renderer/debug/"), new Pair("../ReactCommon/react/renderer/leakchecker/", "react/renderer/leakchecker/"), new Pair("../ReactCommon/react/renderer/mapbuffer/", "react/renderer/mapbuffer/"), new Pair("../ReactCommon/react/renderer/mounting/", "react/renderer/mounting/"), @@ -159,6 +162,7 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa new Pair("../ReactCommon/react/renderer/scheduler/", "react/renderer/scheduler/"), new Pair("../ReactCommon/react/renderer/telemetry/", "react/renderer/telemetry/"), new Pair("../ReactCommon/react/renderer/uimanager/", "react/renderer/uimanager/"), + new Pair("../ReactCommon/react/debug/", "react/debug/"), new Pair("../ReactCommon/react/utils/", "react/utils/"), new Pair("src/main/jni/react/jni", "react/jni/"), ] From ce54f81b8249517a89e420b96095f236241e730a Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 24 Nov 2022 07:12:29 -0800 Subject: [PATCH 5/5] Allow `reactnativejni` to be consumed via prefab Summary: This is another library which is adding prefab support as it's needed by Expo libraries and Reanimated. Changelog: [Internal] [Changed] - Allow `reactnativejni` to be consumed via prefab Reviewed By: cipolleschi Differential Revision: D41520801 fbshipit-source-id: a78c1ac04c2b9181422f3e7f9f6e6d9d39c972ea --- ReactAndroid/build.gradle | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index e4f28debed1481..041f546c668f6e 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -197,6 +197,10 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa "react_render_mounting", new Pair("../ReactCommon/react/renderer/mounting/", "react/renderer/mounting/"), ), + new PrefabPreprocessingEntry( + "reactnativejni", + new Pair("src/main/jni/react/jni", "react/jni/"), + ), ] ) it.outputDir.set(prefabHeadersDir) @@ -425,12 +429,12 @@ android { "-DANDROID_TOOLCHAIN=clang", "-DANDROID_PLATFORM=android-21" - targets "reactnativejni", - "jscexecutor", + targets "jscexecutor", "jsijniprofiler", "reactnativeblob", "reactperfloggerjni", // prefab targets + "reactnativejni", "react_render_debug", "turbomodulejsijni", "runtimeexecutor", @@ -567,6 +571,9 @@ android { react_render_mounting { headers(new File(prefabHeadersDir, "react_render_mounting").absolutePath) } + reactnativejni { + headers(new File(prefabHeadersDir, "reactnativejni").absolutePath) + } "hermes-executor" { headers(new File(prefabHeadersDir, "hermes-executor").absolutePath) }