Skip to content

Commit

Permalink
gradle vm cleanup fix (#32257)
Browse files Browse the repository at this point in the history
Summary:
This patch remove unused .so files for reduce android .apk and .aab

## Changelog

- [Android] [Fixed] - Exclude unused .so files for reduce android .apk and .aab

Pull Request resolved: #32257

Test Plan:
I have created a repository for testing:
https://github.com/enniel/react-native-invalid-build-example

That lines includes patch for fix bug:
https://github.com/enniel/react-native-invalid-build-example/blob/f195ecdbeaca88ffb57c344615ede45ea0f6ef57/android/app/react.gradle#L331-L385

```sh
git clone https://github.com/enniel/react-native-invalid-build-example
cd react-native-invalid-build-example
npm i
```

debug, hermes disabled, without patch
```sh
git switch hermes-disabled-without-patch
npm run build:android:debug:apk
```
open android/app/build/outputs/apk/debug/app-debug.apk in android studio

![debug-hermes-disabled-without-patch](https://user-images.githubusercontent.com/19760944/134640636-1505cfa4-0be9-4bb8-8130-74af01ebfd94.png)

debug, hermes enabled, without patch
```sh
git switch hermes-enabled-without-patch
npm run build:android:debug:apk
```
open android/app/build/outputs/apk/debug/app-debug.apk in android studio

![debug-hermes-enabled-without-patch](https://user-images.githubusercontent.com/19760944/134640743-f5730fbe-433f-45eb-a87c-e0e2eadb056b.png)

debug, hermes disabled, with patch
```sh
git switch hermes-disabled-with-patch
npm run build:android:debug:apk
```
open android/app/build/outputs/apk/debug/app-debug.apk in android studio

![debug-hermes-disabled-with-patch](https://user-images.githubusercontent.com/19760944/134640895-36f294ae-36fb-40b7-a767-f6b421fdbabd.png)

debug, hermes enabled, with patch
```sh
git switch hermes-enabled-with-patch
npm run build:android:debug:apk
```
open android/app/build/outputs/apk/debug/app-debug.apk in android studio

![debug-hermes-enabled-with-patch](https://user-images.githubusercontent.com/19760944/134641040-f4f08755-5234-4567-849d-0b815d58bc15.png)

release, hermes disabled, without patch
```sh
git switch hermes-disabled-without-patch
npm run build:android:release:apk
```
open android/app/build/outputs/apk/release/app-release.apk in android studio

![release-hermes-disabled-without-patch](https://user-images.githubusercontent.com/19760944/134641255-da3ccc5f-91e2-4b12-b0b2-1590fa13e5ce.png)

release, hermes enabled, without patch
```sh
git switch hermes-enabled-without-patch
npm run build:android:release:apk
```
open android/app/build/outputs/apk/release/app-release.apk in android studio

![release-hermes-enabled-without-patch](https://user-images.githubusercontent.com/19760944/134641396-f2ca3178-4225-4de7-b1f7-b741edba1877.png)

release, hermes disabled, with patch
```sh
git switch hermes-disabled-with-patch
npm run build:android:release:apk
```
open android/app/build/outputs/apk/release/app-release.apk in android studio

![release-hermes-disabled-with-patch](https://user-images.githubusercontent.com/19760944/134641675-b967df44-af1b-4070-9f84-a2f029381a39.png)

release, hermes enabled, with patch
```sh
git switch hermes-enabled-with-patch
npm run build:android:release:apk
```
open android/app/build/outputs/apk/release/app-release.apk in android studio

![release-hermes-enabled-with-patch](https://user-images.githubusercontent.com/19760944/134641737-aa4a435a-1fe2-4107-8bbd-8dadda24aebd.png)

Reviewed By: cortinico

Differential Revision: D31167423

Pulled By: ShikaSD

fbshipit-source-id: 4026df818c57fa699526ca1c31c1d1a68d58baef
  • Loading branch information
enniel authored and facebook-github-bot committed Sep 27, 2021
1 parent 8d0a2e7 commit 6f12674
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import com.facebook.react.utils.detectedHermesCommand
import java.io.File
import java.util.*
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.tasks.Copy
import org.gradle.api.tasks.TaskProvider

private const val REACT_GROUP = "react"

Expand Down Expand Up @@ -134,6 +136,8 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte
is LibraryVariant -> variant.packageLibraryProvider
else -> tasks.named("package$targetName")
}
val stripDebugSymbolsTask: TaskProvider<Task>? = tasks.named("strip${targetName}DebugSymbols")
val mergeNativeLibsTask: TaskProvider<Task>? = tasks.named("merge${targetName}NativeLibs")

val mergeResourcesTask = variant.mergeResourcesProvider
val mergeAssetsTask = variant.mergeAssetsProvider
Expand All @@ -160,7 +164,25 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte

packageTask.configure {
if (config.enableVmCleanup.get()) {
it.doFirst { cleanupVMFiles(enableHermes, isRelease, targetPath) }
val libDir = "$buildDir/intermediates/transforms/"
val targetVariant = ".*/transforms/[^/]*/$targetPath/.*".toRegex()
it.doFirst { cleanupVMFiles(libDir, targetVariant, enableHermes, isRelease) }
}
}

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) }
}
}

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) }
}
}

Expand Down Expand Up @@ -191,14 +213,13 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte
preBundleTask.dependsOn(currentAssetsCopyTask)
}

private fun Project.cleanupVMFiles(enableHermes: Boolean, isRelease: Boolean, targetPath: String) {
private fun Project.cleanupVMFiles(libDir: String, targetVariant: Regex, enableHermes: Boolean, isRelease: Boolean) {
// Delete the VM related libraries that this build doesn't need.
// The application can manage this manually by setting 'enableVmCleanup: false'
//
// 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.
val libDir = "$buildDir/intermediates/transforms/"
fileTree(libDir) {
if (enableHermes) {
// For Hermes, delete all the libjsc* files
Expand All @@ -208,18 +229,23 @@ private fun Project.cleanupVMFiles(enableHermes: Boolean, isRelease: Boolean, ta
// Reduce size by deleting the debugger/inspector
it.include("**/libhermes-inspector.so")
it.include("**/libhermes-executor-debug.so")
it.include("**/libhermes-executor-common-debug.so")
} else {
// Release libs take precedence and must be removed
// to allow debugging
it.include("**/libhermes-executor-release.so")
it.include("**/libhermes-executor-common-release.so")
}
} else {
// For JSC, delete all the libhermes* files
it.include("**/libhermes*.so")
// Delete the libjscexecutor from release build
if (isRelease) {
it.include("**/libjscexecutor.so")
}
}
}
.visit { visit ->
val targetVariant = ".*/transforms/[^/]*/$targetPath/.*".toRegex()
val path = visit.file.absolutePath.replace(File.separatorChar, '/')
if (path.matches(targetVariant) && visit.file.isFile) {
visit.file.delete()
Expand Down
32 changes: 27 additions & 5 deletions react.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,8 @@ afterEvaluate {
// 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 libDir = "$buildDir/intermediates/transforms/"
def vmSelectionAction = {

def vmSelectionAction = { libDir ->
fileTree(libDir).matching {
if (enableHermes) {
// For Hermes, delete all the libjsc* files
Expand All @@ -340,26 +340,48 @@ afterEvaluate {
// Reduce size by deleting the debugger/inspector
include '**/libhermes-inspector.so'
include '**/libhermes-executor-debug.so'
include '**/libhermes-executor-common-debug.so'
} else {
// Release libs take precedence and must be removed
// to allow debugging
include '**/libhermes-executor-release.so'
include '**/libhermes-executor-common-release.so'
}
} else {
// For JSC, delete all the libhermes* files
include "**/libhermes*.so"
// Delete the libjscexecutor from release build
if (isRelease) {
include "**/libjscexecutor.so"
}
}
}.visit { details ->
def targetVariant = ".*/transforms/[^/]*/${targetPath}/.*"
def targetVariant1 = ".*/transforms/[^/]*/${targetPath}/.*"
def targetVariant2 = ".*/merged_native_libs/${targetPath}/out/lib/.*"
def targetVariant3 = ".*/stripped_native_libs/${targetPath}/out/lib/.*"
def path = details.file.getAbsolutePath().replace(File.separatorChar, '/' as char)
if (path.matches(targetVariant) && details.file.isFile()) {
if ((path.matches(targetVariant1) || path.matches(targetVariant2) || path.matches(targetVariant3)) && details.file.isFile()) {
details.file.delete()
}
}
}

if (enableVmCleanup) {
packageTask.doFirst(vmSelectionAction)
def task = tasks.findByName("package${targetName}")
def transformsLibDir = "$buildDir/intermediates/transforms/"
task.doFirst { vmSelectionAction(transformsLibDir) }

def sTask = tasks.findByName("strip${targetName}DebugSymbols")
if (sTask != null) {
def strippedLibDir = "$buildDir/intermediates/stripped_native_libs/${targetPath}/out/lib/"
sTask.doLast { vmSelectionAction(strippedLibDir) }
}

def mTask = tasks.findByName("merge${targetName}NativeLibs")
if (mTask != null) {
def mergedLibDir = "$buildDir/intermediates/merged_native_libs/${targetPath}/out/lib/"
mTask.doLast { vmSelectionAction(mergedLibDir) }
}
}
}
}

2 comments on commit 6f12674

@coolguy001tv
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libjscexecutor should not be deleted! In android release, current version (0.67.0) will crash without libjscexecutor.so.
I comment the code 354-356 lines in react.gradle , then everything is ok.

//if (isRelease) {
//                        include "**/libjscexecutor.so"
//                    }

@cortinico
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libjscexecutor should not be deleted! In android release, current version (0.67.0) will crash without libjscexecutor.so.

You should be good without libjscexecutor.so as libjsc.so should be sufficient for Release only builds. If that is not the case, please open an issue + provide a reproducer

Please sign in to comment.