From 91adb761cf1583598d4d63ce879fd7e0f4ae793c Mon Sep 17 00:00:00 2001 From: Geraint White Date: Wed, 13 Oct 2021 12:49:02 -0700 Subject: [PATCH] Add hermesFlagsForVariant and deleteDebugFilesForVariant (#32281) Summary: Ref https://github.com/facebook/react-native/issues/25601#issuecomment-510856047. From https://github.com/facebook/react-native/pull/31040. The `hermesFlagsRelease` option only works with the release build type, but not with other build types. This PR allows hermes flags on a per variant basis to be specified using the `hermesFlagsForVariant` lambda. It also allows the hermes debugger cleanup to be run on a per variant basis using the `deleteDebugFilesForVariant` lambda. ## Changelog [Android] [Fixed] - Fix hermesFlags not working with multiple variants Pull Request resolved: https://github.com/facebook/react-native/pull/32281 Test Plan: Set the following options in `android/app/build.gradle` and ensure warnings are hidden when running `./gradlew assembleRelease` and `./gradlew assembleLive`. ``` hermesFlagsForVariant: { def v -> v.name.toLowerCase().contains('release') || v.name.toLowerCase().contains('live') ? ['-w'] : [] }, deleteDebugFilesForVariant: { def v -> v.name.toLowerCase().contains('release') || v.name.toLowerCase().contains('live') }, ``` Reviewed By: cortinico Differential Revision: D31234241 Pulled By: ShikaSD fbshipit-source-id: 2cb3dd63adbcd023061076b5a3b262a87b470518 --- .../com/facebook/react/ReactExtension.kt | 16 ++++++++ .../com/facebook/react/TaskConfiguration.kt | 18 ++++----- react.gradle | 39 ++++++++++++------- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt index ad432829ace3f9..9344c51faf8493 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt @@ -137,6 +137,22 @@ abstract class ReactExtension @Inject constructor(project: Project) { */ var enableHermesForVariant: (BaseVariant) -> Boolean = { enableHermes.get() } + /** + * Functional interface specify flags for Hermes on specific [BaseVariant] Default: will + * return [hermesFlagsRelease] for Release variants and [hermesFlagsDebug] for Debug variants. + */ + var hermesFlagsForVariant: (BaseVariant) -> List = { + variant -> if (variant.isRelease) hermesFlagsRelease.get() else hermesFlagsDebug.get() + } + + /** + * Functional interface to delete debug files only on specific [BaseVariant] Default: will + * return True for Release variants and False for Debug variants. + */ + var deleteDebugFilesForVariant: (BaseVariant) -> Boolean = { + variant -> variant.isRelease + } + /** Flags to pass to Hermes for Debug variants. Default: [] */ val hermesFlagsDebug: ListProperty = objects.listProperty(String::class.java).convention(emptyList()) 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 ad0f73c5ce4b36..eefaef68a8735c 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 @@ -49,6 +49,7 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte val execCommand = nodeExecutableAndArgs + cliPath val enableHermes = config.enableHermesForVariant(variant) + val cleanup = config.deleteDebugFilesForVariant(variant) val bundleEnabled = variant.checkBundleEnabled(config) val bundleTask = @@ -100,8 +101,7 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte it.reactRoot = config.reactRoot.get().asFile it.hermesCommand = detectedHermesCommand(config) - it.hermesFlags = - if (isRelease) config.hermesFlagsRelease.get() else config.hermesFlagsDebug.get() + it.hermesFlags = config.hermesFlagsForVariant(variant) it.jsBundleFile = jsBundleFile it.composeSourceMapsCommand = nodeExecutableAndArgs + config.composeSourceMapsPath.get() it.jsPackagerSourceMapFile = jsPackagerSourceMapFile @@ -166,7 +166,7 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte if (config.enableVmCleanup.get()) { val libDir = "$buildDir/intermediates/transforms/" val targetVariant = ".*/transforms/[^/]*/$targetPath/.*".toRegex() - it.doFirst { cleanupVMFiles(libDir, targetVariant, enableHermes, isRelease) } + it.doFirst { cleanupVMFiles(libDir, targetVariant, enableHermes, cleanup) } } } @@ -174,7 +174,7 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte if (config.enableVmCleanup.get()) { val libDir = "$buildDir/intermediates/stripped_native_libs/${targetPath}/out/lib/" val targetVariant = ".*/stripped_native_libs/$targetPath/out/lib/.*".toRegex() - it.doLast { cleanupVMFiles(libDir, targetVariant, enableHermes, isRelease) } + it.doLast { cleanupVMFiles(libDir, targetVariant, enableHermes, cleanup) } } } @@ -182,7 +182,7 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte if (config.enableVmCleanup.get()) { val libDir = "$buildDir/intermediates/merged_native_libs/${targetPath}/out/lib/" val targetVariant = ".*/merged_native_libs/$targetPath/out/lib/.*".toRegex() - it.doLast { cleanupVMFiles(libDir, targetVariant, enableHermes, isRelease) } + it.doLast { cleanupVMFiles(libDir, targetVariant, enableHermes, cleanup) } } } @@ -217,7 +217,7 @@ private fun Project.cleanupVMFiles( libDir: String, targetVariant: Regex, enableHermes: Boolean, - isRelease: Boolean + cleanup: Boolean ) { // Delete the VM related libraries that this build doesn't need. // The application can manage this manually by setting 'enableVmCleanup: false' @@ -230,7 +230,7 @@ private fun Project.cleanupVMFiles( // For Hermes, delete all the libjsc* files it.include("**/libjsc*.so") - if (isRelease) { + if (cleanup) { // Reduce size by deleting the debugger/inspector it.include("**/libhermes-inspector.so") it.include("**/libhermes-executor-debug.so") @@ -245,7 +245,7 @@ private fun Project.cleanupVMFiles( // For JSC, delete all the libhermes* files it.include("**/libhermes*.so") // Delete the libjscexecutor from release build - if (isRelease) { + if (cleanup) { it.include("**/libjscexecutor.so") } } @@ -270,5 +270,5 @@ private fun BaseVariant.checkBundleEnabled(config: ReactExtension): Boolean { return isRelease } -private val BaseVariant.isRelease: Boolean +internal val BaseVariant.isRelease: Boolean get() = name.toLowerCase(Locale.ROOT).contains("release") diff --git a/react.gradle b/react.gradle index 812d11e43e02de..d9e27140aeaf94 100644 --- a/react.gradle +++ b/react.gradle @@ -83,6 +83,28 @@ def enableHermesForVariant = config.enableHermesForVariant ?: { def variant -> config.enableHermes ?: false } +// Set hermesFlagsForVariant to a function to configure per variant, +// or set `hermesFlagsRelease` and `hermesFlagsDebug` to an array +def hermesFlagsForVariant = config.hermesFlagsForVariant ?: { + def variant -> + def hermesFlags; + if (variant.name.toLowerCase().contains("release")) { + // Can't use ?: since that will also substitute valid empty lists + hermesFlags = config.hermesFlagsRelease + if (hermesFlags == null) hermesFlags = ["-O", "-output-source-map"] + } else { + hermesFlags = config.hermesFlagsDebug + if (hermesFlags == null) hermesFlags = [] + } + return hermesFlags +} + +// Set deleteDebugFilesForVariant to a function to configure per variant, +// defaults to True for Release variants and False for debug variants +def deleteDebugFilesForVariant = config.deleteDebugFilesForVariant ?: { + def variant -> variant.name.toLowerCase().contains("release") +} + android { buildTypes.all { resValue "integer", "react_native_dev_server_port", reactNativeDevServerPort() @@ -177,18 +199,9 @@ afterEvaluate { if (enableHermes) { doLast { - def hermesFlags; + def hermesFlags = hermesFlagsForVariant(variant) def hbcTempFile = file("${jsBundleFile}.hbc") exec { - if (targetName.toLowerCase().contains("release")) { - // Can't use ?: since that will also substitute valid empty lists - hermesFlags = config.hermesFlagsRelease - if (hermesFlags == null) hermesFlags = ["-O", "-output-source-map"] - } else { - hermesFlags = config.hermesFlagsDebug - if (hermesFlags == null) hermesFlags = [] - } - if (Os.isFamily(Os.FAMILY_WINDOWS)) { commandLine("cmd", "/c", getHermesCommand(), "-emit-binary", "-out", hbcTempFile, jsBundleFile, *hermesFlags) } else { @@ -328,7 +341,7 @@ afterEvaluate { // This should really be done by packaging all Hermes related libs into // two separate HermesDebug and HermesRelease AARs, but until then we'll // kludge it by deleting the .so files out of the /transforms/ directory. - def isRelease = targetName.toLowerCase().contains("release") + def cleanup = deleteDebugFilesForVariant(variant) def vmSelectionAction = { libDir -> fileTree(libDir).matching { @@ -336,7 +349,7 @@ afterEvaluate { // For Hermes, delete all the libjsc* files include "**/libjsc*.so" - if (isRelease) { + if (cleanup) { // Reduce size by deleting the debugger/inspector include '**/libhermes-inspector.so' include '**/libhermes-executor-debug.so' @@ -351,7 +364,7 @@ afterEvaluate { // For JSC, delete all the libhermes* files include "**/libhermes*.so" // Delete the libjscexecutor from release build - if (isRelease) { + if (cleanup) { include "**/libjscexecutor.so" } }