Skip to content

Commit

Permalink
Add hermesFlagsForVariant and deleteDebugFilesForVariant (#32281)
Browse files Browse the repository at this point in the history
Summary:
Ref #25601 (comment).

From #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

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Fixed] - Fix hermesFlags not working with multiple variants

Pull Request resolved: #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
  • Loading branch information
geraintwhite authored and facebook-github-bot committed Oct 13, 2021
1 parent 8ba4a2f commit 91adb76
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> = {
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<String> =
objects.listProperty(String::class.java).convention(emptyList())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -166,23 +166,23 @@ 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) }
}
}

stripDebugSymbolsTask?.configure {
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) }
}
}

mergeNativeLibsTask?.configure {
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) }
}
}

Expand Down Expand Up @@ -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'
Expand All @@ -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")
Expand All @@ -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")
}
}
Expand All @@ -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")
39 changes: 26 additions & 13 deletions react.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -328,15 +341,15 @@ 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 {
if (enableHermes) {
// 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'
Expand All @@ -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"
}
}
Expand Down

0 comments on commit 91adb76

Please sign in to comment.