Do not include JSC and Hermes debug libraries when building in release mode#30303
Do not include JSC and Hermes debug libraries when building in release mode#30303maxammann wants to merge 7 commits into
Conversation
Summary:
* Execute vmSelectionAction before strip${targetName}DebugSymbols instead of package${targetName}
* Remove .so files from merged_native_libs such that they don't get copied to stripped_native_libs
|
Hi @maxammann! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Fixes #29064 and facebook/hermes#96 |
|
|
A workaround which works right now is: thanks to @prempalsingh |
Base commit: 00d9dea |
Base commit: 00d9dea |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
cc @fkgozali |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@fkgozali has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
There's a failure in FB internal CI for some reason, I'll need some time to look into that. CircleCI test_android seems green with this PR though. |
|
@maxammann: it looks like something in this change doesn't play well with multiple flavors of rn-tester build. I think gradle got confused with the cached NDK build artifacts from the previous runs. To repro, we can try building with and without Fabric, and the 2nd build always failed, until you try again. For example: The failure: This means, the removal logic in this PR incorrectly notifies gradle of the changes, causing incorrect caching of task outputs. Can you look into how to make each run consistently succeed without having to run the same command twice? |
|
@fkgozali thanks for checking the changes! I replaced the hacky way with a more stable way. I think this should solve problems with compiling multiple times |
|
Updating branch to rerun failed ios tests |
|
The iOS failures seem unrelated, but test_android had: |
|
@fkgozali The api I need is only available with the gradle plugin 4.2 which is still alpha: https://developer.android.com/reference/tools/gradle-api/4.2/com/android/build/api/variant/ApplicationVariant#packagingOptions(kotlin.Function1) I think it makes sense to wait with this PR until we are at version 4.2. What do you think? |
|
Alternatively there is this solution: https://github.com/Jween/android-soexcluder/blob/master/app/src/main/groovy/com/jween/gradle/soexcluder/AppPlugin.groovy#L66 |
I think this is probably best next step. We tend to upgrade AGP regularly, and if this is a good reason to upgrade, then it will make it easier for us to move forward during upgrade.
I'm hesitant to add a custom logic that's supposed to be supported by AGP directly, so unless there's a super urgent reason for excluding the .so right now, let's wait. What do you think? |
|
Jup lets wait, I can close this and reopen it in the future. What do you think? |
Sure, thanks! |
|
Gradle 4.2 is released now and this could be fixed. I'll also check whether this still happens. Does RN already use 4.2? |
|
Still happening here. Using Gradle 6.4 and React Native 0.66.4. My workaround is to put this in build.gradle: android {
// ...
buildTypes {
release {
packagingOptions {
exclude "**/libhermes-inspector.so"
exclude "**/libhermes*debug.so"
}
// ...
}
}
} |
|
I'm closing this as we now have a different mechanism to exclude unnecessary libs from being bundled in the final .apk/.aab |
|
@cortinico Could you place a link to where that new mechanism is explained? Thanks in advance. |
Summary
Right now the
libhermes-executor-debug.soandlibjscexecutor.soget included when doing a release build with Hermes enabled. This can be fixed by deleting the .so files from the correct directories in the gradle build directory. They have to be deleted because they are packaged in the./node_modules/react-native/android/com/facebook/react/react-native/0.63.2/react-native-0.63.2.aaraar. This actually should be fixed somewhere else properly.This PR fixes the workaround already implemented:
Before this PR multiple stack traces are thrown when starting an app:
Afterwards they are gone:
Changelog
Test Plan
adb logcatfor the stacktraces. If they are gone after implementing this fix then it works. Run./gradlew cleanevery time before building.