Do not embed Hermes in the Gutenberg XCFramework. Add to distribution archive instead. #5973
Conversation
The idea being to avoid nested frameworks because of embedding and instead adding Hermes as a another vendored framework in the podspec.
mokagio
force-pushed
the
mokagio/use-hermes-from-cocoapods
branch
from
dea1737 to
a5f1d5f
Compare
|
Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job! |
Hey @mokagio 👋 , the file size issue was solved by having the Hermes framework embedded in the XCFramework Xcode project with the option "Embed & Sign". I tried other options like "Embed without Signing" but it was producing a way larger framework (around 500 MB). I'm not sure what's Xcode doing when enabling that option, but my gut feeling is that it's somehow filtering out unused parts of the framework 🤔. |
…om-cocoapods # Conflicts: # gutenberg
There was a problem hiding this comment.
@mokagio I've tested this solution in wordpress-mobile/WordPress-iOS#21021 and confirmed the app is working as expected. I think this PR is good to go, so feel free to set it ready for review anytime. In the meantime, I'll go ahead and preliminary approve it in case you want to merge it right away.
Branches used:
- Gutenberg:
rnmobile/upgrade/react-native-0.71.8 - WP-iOS:
gutenberg/tweak-pod-deps-by-rn-version
Test results using a local build with a local Gutenberg installation:
- ✅ Simulator
- ✅ Device
Test results using a local build with Gutenberg framework:
- ✅ Simulator
- ✅ Device
CI build (pr21021-1c5fb5f):
- ✅ Device
NOTE: The only problem I found is a 💥 crash when opening a post with a VideoPress video, inserting the block doesn't crash. This error seems unrelated to the Hermes framework workaround, so we can omit it for now as it will be solved in a separate PR.
mokagio
changed the title
|
Thanks @fluiddot for the detailed review and for getting this ready to merge. The Git history is dirtier than I'd like it to be. Shame on me for shipping those "WIP – ..." commits in the first place. I briefly explored rebasing or creating a new branch to cherry pick, but I got conflicts in the submodules and decided to take the hit in terms of non linear history in the interest of moving the work along. Merging now. |
Another option would be to squash and merge, this option can be helpful when merging into a feature branch like the RN upgrade to keep the Git history simpler and cleaner. In fact, we followed this approach in the Gutenberg PR because we had tons of changes there. |
|
@fluiddot yeah, good point. |
A different approach to the problem #5968 tries to solve.
Under the assumption this approach works, I believe it's simpler than the alternative because it avoids embedding in the first place.
otool -Lresult in the same way that was done here – See belowotool -Loutput:The outcome is different from the one @fluiddot reported with the version built from #5968. But it doesn't seem to impact the behavior at runtime.