From fe2716b8ae3599d6a042f0f92a50e4b7ebee3fb0 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 24 Nov 2022 08:45:15 -0800 Subject: [PATCH] Consolidate hermes-executor-debug and -release inside a single target (#35454) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35454 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 Reviewed By: cipolleschi Differential Revision: D41519119 fbshipit-source-id: d9ddc30b72164daa29c735836ea433fd4d917fc8 --- 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 d435359349..f14aa4d235 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 0c00d480ed..2c954374c3 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 48eaaa4c06..e72bca607c 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 @@ package com.facebook.hermes.reactexecutor; 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 class HermesExecutor extends JavaScriptExecutor { 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 3145a733fe..fcf9f93e4c 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 bb3699d58d..a2b60f4257 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 479c3f742e..13bbb3ab8c 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 2d0093c6a5..41d498b23d 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) + 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) - - 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) - - 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) + assertFalse("**/libhermes-executor.so" in includes) assertTrue("**/libjsc.so" in includes) assertTrue("**/libjscexecutor.so" in includes)