Skip to content

Commit

Permalink
Consolidate hermes-executor-debug and -release inside a single target (
Browse files Browse the repository at this point in the history
…#35454)

Summary:
Pull Request resolved: #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
  • Loading branch information
cortinico authored and facebook-github-bot committed Nov 24, 2022
1 parent af6c9e2 commit fe2716b
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 102 deletions.
19 changes: 1 addition & 18 deletions ReactAndroid/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ android {

targets "reactnativejni",
"jscexecutor",
"hermes-executor",
"jsijniprofiler",
"reactnativeblob",
"reactperfloggerjni",
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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";
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
$<$<CONFIG:Debug>:-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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>, List<String>> {
fun getPackagingOptionsForVariant(hermesEnabled: Boolean): Pair<List<String>, List<String>> {
val excludes = mutableListOf<String>()
val includes = mutableListOf<String>()
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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit fe2716b

Please sign in to comment.