Remove libc++_shared.so from AAR and add hermes-cppruntime.aar #74
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary:
1. Support multiple projects architecture and move original Hermes build
into hermes/ subproject.
2. Add packagingOptions.exclude "**/libc++_shared.so" for Hermes
subproject.
Summary:
1. Add cppruntime subproject which will generate
hermes-cppruntime-release.aar included libc++_shared.so.
2. To get libc++_shared.so, although we could have a gradle
custom task to copy the file from NDK, the drawback is that the
path might change between different NDK versions.
I decided to have a stub cpp and use formal cmake build to
generate libc++_shared.so as usual.
During the time to generate AAR,
using packagingOptions.exclude "**/libstub.so"
to keep only libc++_shared.so.
facebook-github-bot
added
the
CLA Signed
|
Looks good to me. Am I understanding it correctly that this does not prevent ABI compatibility bugs, but instead makes such bugs easier to reproduce and fix because the Will we need to e.g. update the react-native app template? |
There was a problem hiding this comment.
@willholen has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Yes, to truly prevent ABI incompatibility, we should align the NDK version for Hermes and RN.
Yes, if the PR landed and new version released. I will have a PR to remove the pickFirst from RN template. |
|
@willholen merged this pull request in 7089054. |
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 12, 2019
Summary: The 'aar/' subdirectory was introduced in PR #74. This changes it back, to retain compatibility with existing versions. Reviewed By: jbower-fb Differential Revision: D17330056 fbshipit-source-id: 28722e874024ad16b2f5fbba4bbc3e7d4669610f
mganandraj
pushed a commit
to mganandraj/hermes
that referenced
this pull request
Oct 15, 2019
…ook#74) Summary: Currently the Hermes AAR includes libc++_shared.so, so that React Native needs to add [pickFirst](https://github.com/facebook/react-native/blob/0738fe5738c21463da93e3551f8f78ddcb6545e3/template/android/app/build.gradle#L181-L184) to workaround gradle conflict issue. However, pickFirst is not a reliable way to select expected libc++_shared.so because we cannot ensure which gradle dependency is the first one for gradle to pick. If user have other 3rd party gradle dependencies, the situation is even worst and this kind of bug is hard to troubleshoot. Even though NDK maintains good ABI quality between different versions, I would still suggest if we could remove the pickFirst from RN project template. This PR removes libc++_shared.so from Hermes AAR and adds another hermes-cppruntime.aar depedency if user need it for other purposes. The approach is pretty much like as what we did in [jsc-android](https://github.com/react-native-community/jsc-android-buildscripts#for-react-native-version-058-below). Pull Request resolved: facebook#74 Reviewed By: dulinriley Differential Revision: D16586875 Pulled By: willholen fbshipit-source-id: add72a429d320c4ac02f443ddc1b912b2c52506f
mganandraj
pushed a commit
to mganandraj/hermes
that referenced
this pull request
Oct 15, 2019
Summary: The 'aar/' subdirectory was introduced in PR facebook#74. This changes it back, to retain compatibility with existing versions. Reviewed By: jbower-fb Differential Revision: D17330056 fbshipit-source-id: 28722e874024ad16b2f5fbba4bbc3e7d4669610f
2 tasks
Closed
mganandraj
pushed a commit
to mganandraj/hermes
that referenced
this pull request
Jun 22, 2022
Split PR and CI in two yamls so we can run publish on internal ADO instance
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the Hermes AAR includes libc++_shared.so, so that React Native needs to add pickFirst to workaround gradle conflict issue.
However, pickFirst is not a reliable way to select expected libc++_shared.so because we cannot ensure which gradle dependency is the first one for gradle to pick.
If user have other 3rd party gradle dependencies, the situation is even worst and this kind of bug is hard to troubleshoot.
Even though NDK maintains good ABI quality between different versions, I would still suggest if we could remove the pickFirst from RN project template.
This PR removes libc++_shared.so from Hermes AAR and adds another hermes-cppruntime.aar depedency if user need it for other purposes.
The approach is pretty much like as what we did in jsc-android.