Skip to content

Commit

Permalink
RNGP - Do the .so cleanup using pickFirst and exclude (#35093)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #35093

It turns out that using the Artifacts API to manipulate the APK to remove
.so has unintended side effects and is causing the `installDebug` and `installRelease`
commands to fail.

I've resorted to register a packaging option for each variant to make sure we include only
the correct artifacts we want.

This should fix the current startup crash that is experienced on main.

Changelog:
[Android] [Fixed] - RNGP - Do the .so cleanup using pickFirst and exclude

Reviewed By: cipolleschi

Differential Revision: D40722285

fbshipit-source-id: 982e1e9c474522fc4419c969ede5ee14e5404f3a
  • Loading branch information
@cortinico @facebook-github-bot
cortinico authored and facebook-github-bot committed Oct 26, 2022
1 parent 7d61b9d commit 2ff08e8
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 661 deletions.
48 changes: 12 additions & 36 deletions packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@

package com.facebook.react

import com.android.build.api.artifact.SingleArtifact
import com.android.build.api.variant.Variant
import com.facebook.react.tasks.BundleHermesCTask
import com.facebook.react.tasks.NativeLibraryAabCleanupTask
import com.facebook.react.tasks.NativeLibraryApkCleanupTask
import com.facebook.react.utils.NdkConfiguratorUtils.configureJsEnginePackagingOptions
import com.facebook.react.utils.NdkConfiguratorUtils.configureNewArchPackagingOptions
import com.facebook.react.utils.ProjectUtils.isHermesEnabled
import com.facebook.react.utils.detectedCliPath
import com.facebook.react.utils.detectedEntryFile
Expand All @@ -36,16 +35,20 @@ internal fun Project.configureReactTasks(variant: Variant, config: ReactExtensio
// Additional node and packager commandline arguments
val cliPath = detectedCliPath(project.projectDir, config)

val enableHermesInProject = project.isHermesEnabled
val enableHermesInThisVariant =
val isHermesEnabledInProject = project.isHermesEnabled
val isHermesEnabledInThisVariant =
if (config.enableHermesOnlyInVariants.get().isNotEmpty()) {
config.enableHermesOnlyInVariants.get().contains(variant.name) && enableHermesInProject
config.enableHermesOnlyInVariants.get().contains(variant.name) && isHermesEnabledInProject
} else {
enableHermesInProject
isHermesEnabledInProject
}
val isDebuggableVariant =
config.debuggableVariants.get().any { it.equals(variant.name, ignoreCase = true) }

configureNewArchPackagingOptions(project, variant)
configureJsEnginePackagingOptions(
config, variant, isHermesEnabledInThisVariant, isDebuggableVariant)

if (!isDebuggableVariant) {
val bundleTask =
tasks.register("createBundle${targetName}JsAndAssets", BundleHermesCTask::class.java) {
Expand All @@ -59,8 +62,8 @@ internal fun Project.configureReactTasks(variant: Variant, config: ReactExtensio
it.bundleAssetName.set(config.bundleAssetName)
it.jsBundleDir.set(jsBundleDir)
it.resourcesDir.set(resourcesDir)
it.hermesEnabled.set(enableHermesInThisVariant)
it.minifyEnabled.set(!enableHermesInThisVariant)
it.hermesEnabled.set(isHermesEnabledInThisVariant)
it.minifyEnabled.set(!isHermesEnabledInThisVariant)
it.devEnabled.set(false)
it.jsIntermediateSourceMapsDir.set(jsIntermediateSourceMapsDir)
it.jsSourceMapsDir.set(jsSourceMapsDir)
Expand All @@ -71,31 +74,4 @@ internal fun Project.configureReactTasks(variant: Variant, config: ReactExtensio
variant.sources.res?.addGeneratedSourceDirectory(bundleTask, BundleHermesCTask::resourcesDir)
variant.sources.assets?.addGeneratedSourceDirectory(bundleTask, BundleHermesCTask::jsBundleDir)
}

if (config.enableSoCleanup.get()) {
val nativeLibraryApkCleanupTask =
project.tasks.register(
"nativeLibrary${targetName}ApkCleanup", NativeLibraryApkCleanupTask::class.java) {
it.debuggableVariant.set(isDebuggableVariant)
it.enableHermes.set(enableHermesInThisVariant)
}
val nativeLibraryBundleCleanupTask =
project.tasks.register(
"nativeLibrary${targetName}BundleCleanup", NativeLibraryAabCleanupTask::class.java) {
it.debuggableVariant.set(isDebuggableVariant)
it.enableHermes.set(enableHermesInThisVariant)
}

variant.artifacts
.use(nativeLibraryApkCleanupTask)
.wiredWithDirectories(
NativeLibraryApkCleanupTask::inputApkDirectory,
NativeLibraryApkCleanupTask::outputApkDirectory)
.toTransform(SingleArtifact.APK)
variant.artifacts
.use(nativeLibraryBundleCleanupTask)
.wiredWithFiles(
NativeLibraryAabCleanupTask::inputBundle, NativeLibraryAabCleanupTask::outputBundle)
.toTransform(SingleArtifact.BUNDLE)
}
}
40 changes: 0 additions & 40 deletions ...ive-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/NativeLibraryAabCleanupTask.kt
View file

This file was deleted.

41 changes: 0 additions & 41 deletions ...ive-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/NativeLibraryApkCleanupTask.kt
View file

This file was deleted.

204 changes: 131 additions & 73 deletions ...act-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package com.facebook.react.utils

import com.android.build.api.variant.AndroidComponentsExtension
import com.android.build.api.variant.Variant
import com.facebook.react.ReactExtension
import com.facebook.react.utils.ProjectUtils.isNewArchEnabled
import java.io.File
Expand All @@ -19,83 +20,140 @@ internal object NdkConfiguratorUtils {
project.pluginManager.withPlugin("com.android.application") {
project.extensions.getByType(AndroidComponentsExtension::class.java).finalizeDsl { ext ->
if (!project.isNewArchEnabled) {
// For Old Arch, we set a pickFirst only on libraries that we know are
// clashing with our direct dependencies (FBJNI, Flipper and Hermes).
ext.packagingOptions.jniLibs.pickFirsts.addAll(
listOf(
"**/libfbjni.so",
"**/libc++_shared.so",
))
} else {
// We enable prefab so users can consume .so/headers from ReactAndroid and hermes-engine
// .aar
ext.buildFeatures.prefab = true

// We set some packagingOptions { pickFirst ... } for our users for libraries we own.
ext.packagingOptions.jniLibs.pickFirsts.addAll(
listOf(
// Hermes & JSC are provided by AAR dependencies we pre-bundle.
"**/libhermes.so",
"**/libjsc.so",
// This is the .so provided by FBJNI via prefab
"**/libfbjni.so",
// Those are prefab libraries we distribute via ReactAndroid
// Due to a bug in AGP, they fire a warning on console as both the JNI
// and the prefab .so files gets considered. See more on:
"**/libfabricjni.so",
"**/libfolly_runtime.so",
"**/libglog.so",
"**/libjsi.so",
"**/libreact_codegen_rncore.so",
"**/libreact_debug.so",
"**/libreact_nativemodule_core.so",
"**/libreact_newarchdefaults.so",
"**/libreact_render_componentregistry.so",
"**/libreact_render_core.so",
"**/libreact_render_debug.so",
"**/libreact_render_graphics.so",
"**/libreact_render_imagemanager.so",
"**/libreact_render_mapbuffer.so",
"**/librrc_image.so",
"**/librrc_view.so",
"**/libruntimeexecutor.so",
"**/libturbomodulejsijni.so",
"**/libyoga.so",
// AGP will give priority of libc++_shared coming from App modules.
"**/libc++_shared.so",
))
// For Old Arch, we don't need to setup the NDK
return@finalizeDsl
}
// We enable prefab so users can consume .so/headers from ReactAndroid and hermes-engine
// .aar
ext.buildFeatures.prefab = true

// If the user has not provided a CmakeLists.txt path, let's provide
// the default one from the framework
if (ext.externalNativeBuild.cmake.path == null) {
ext.externalNativeBuild.cmake.path =
File(
extension.reactNativeDir.get().asFile,
"ReactAndroid/cmake-utils/default-app-setup/CMakeLists.txt")
}
// If the user has not provided a CmakeLists.txt path, let's provide
// the default one from the framework
if (ext.externalNativeBuild.cmake.path == null) {
ext.externalNativeBuild.cmake.path =
File(
extension.reactNativeDir.get().asFile,
"ReactAndroid/cmake-utils/default-app-setup/CMakeLists.txt")
}

// 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 ("-DGENERATED_SRC_DIR" !in cmakeArgs) {
cmakeArgs.add("-DGENERATED_SRC_DIR=${File(project.buildDir, "generated/source")}")
}
if ("-DPROJECT_BUILD_DIR" !in cmakeArgs) {
cmakeArgs.add("-DPROJECT_BUILD_DIR=${project.buildDir}")
}
if ("-DREACT_ANDROID_DIR" !in cmakeArgs) {
cmakeArgs.add(
"-DREACT_ANDROID_DIR=${extension.reactNativeDir.file("ReactAndroid").get().asFile}")
}
if ("-DREACT_ANDROID_BUILD_DIR" !in cmakeArgs) {
cmakeArgs.add(
"-DREACT_ANDROID_BUILD_DIR=${extension.reactNativeDir.file("ReactAndroid/build").get().asFile}")
}
if ("-DANDROID_STL" !in cmakeArgs) {
cmakeArgs.add("-DANDROID_STL=c++_shared")
}
// 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 ("-DGENERATED_SRC_DIR" !in cmakeArgs) {
cmakeArgs.add("-DGENERATED_SRC_DIR=${File(project.buildDir, "generated/source")}")
}
if ("-DPROJECT_BUILD_DIR" !in cmakeArgs) {
cmakeArgs.add("-DPROJECT_BUILD_DIR=${project.buildDir}")
}
if ("-DREACT_ANDROID_DIR" !in cmakeArgs) {
cmakeArgs.add(
"-DREACT_ANDROID_DIR=${extension.reactNativeDir.file("ReactAndroid").get().asFile}")
}
if ("-DREACT_ANDROID_BUILD_DIR" !in cmakeArgs) {
cmakeArgs.add(
"-DREACT_ANDROID_BUILD_DIR=${
extension.reactNativeDir.file("ReactAndroid/build").get().asFile
}")
}
if ("-DANDROID_STL" !in cmakeArgs) {
cmakeArgs.add("-DANDROID_STL=c++_shared")
}
}
}
}

/**
* This method is used to configure the .so Packaging Options for the given variant. It will make
* sure we specify the correct .pickFirsts for all the .so files we are producing or that we're
* aware of as some of our dependencies are pulling them in.
*/
fun configureNewArchPackagingOptions(
project: Project,
variant: Variant,
) {
if (!project.isNewArchEnabled) {
// For Old Arch, we set a pickFirst only on libraries that we know are
// clashing with our direct dependencies (FBJNI, Flipper and Hermes).
variant.packaging.jniLibs.pickFirsts.addAll(
listOf(
"**/libfbjni.so",
"**/libc++_shared.so",
))
} else {
// We set some packagingOptions { pickFirst ... } for our users for libraries we own.
variant.packaging.jniLibs.pickFirsts.addAll(
listOf(
// This is the .so provided by FBJNI via prefab
"**/libfbjni.so",
// Those are prefab libraries we distribute via ReactAndroid
// Due to a bug in AGP, they fire a warning on console as both the JNI
// and the prefab .so files gets considered. See more on:
"**/libfabricjni.so",
"**/libfolly_runtime.so",
"**/libglog.so",
"**/libjsi.so",
"**/libreact_codegen_rncore.so",
"**/libreact_debug.so",
"**/libreact_nativemodule_core.so",
"**/libreact_newarchdefaults.so",
"**/libreact_render_componentregistry.so",
"**/libreact_render_core.so",
"**/libreact_render_debug.so",
"**/libreact_render_graphics.so",
"**/libreact_render_imagemanager.so",
"**/libreact_render_mapbuffer.so",
"**/librrc_image.so",
"**/librrc_view.so",
"**/libruntimeexecutor.so",
"**/libturbomodulejsijni.so",
"**/libyoga.so",
// AGP will give priority of libc++_shared coming from App modules.
"**/libc++_shared.so",
))
}
}

/**
* This method is used to configure the .so Cleanup for the given variant. It takes care of
* cleaning up the .so files that are not needed for Hermes or JSC, given a specific variant.
*/
fun configureJsEnginePackagingOptions(
config: ReactExtension,
variant: Variant,
hermesEnabled: Boolean,
debuggableVariant: Boolean
) {
if (config.enableSoCleanup.get()) {
val (excludes, includes) = getPackagingOptionsForVariant(hermesEnabled, debuggableVariant)
variant.packaging.jniLibs.excludes.addAll(excludes)
variant.packaging.jniLibs.pickFirsts.addAll(includes)
}
}

fun getPackagingOptionsForVariant(
hermesEnabled: Boolean,
debuggableVariant: 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")
}
} else {
excludes.add("**/libhermes.so")
excludes.add("**/libhermes-executor-debug.so")
excludes.add("**/libhermes-executor-release.so")
includes.add("**/libjsc.so")
includes.add("**/libjscexecutor.so")
}
return excludes to includes
}
}

0 comments on commit 2ff08e8

Please sign in to comment.