[Gutenberg] Tweak Pod dependencies based on React Native version used by Gutenberg #21021

fluiddot · 2023-07-06T16:35:35Z

Description

Fixes CI failures in the upstream branch.
Uses workaround for Hermes framework (Do not embed Hermes in the Gutenberg XCFramework. Add to distribution archive instead. gutenberg-mobile#5973)

To test

Check all CI jobs.
Check that app can be built using a local Gutenberg installation:
- Checkout RN upgrade branch in the Gutenberg Mobile project (reference).
- Install pods by running: bundle install && LOCAL_GUTENBERG=1 bundle exec pod install.
- Build the app using XCode.
  NOTE: If you experience a build failure related to 'butter/map.h', try to execute: rm -rf build && rm -rf Pods and install pods again.
Check that app can be built using Gutenberg 1.99.0 via XCFramework:
- Install pods by running: bundle install && bundle exec pod install.
- Build the app using XCode.

NOTE: It's recommended to use Xcode 14.3.1, as I encountered build failures when using Xcode 14.0.

Regression Notes

Potential unintended areas of impact
N/A
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

Portrait and landscape orientations.
Light and dark modes.
Fonts: Larger, smaller and bold text.
High contrast.
VoiceOver.
Languages with large words or with letters/accents not frequently used in English.
Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
iPhone and iPad.
Multi-tasking: Split view and Slide over. (iPad)

fluiddot · 2023-07-06T17:30:58Z

Builds are failing due to the XCFramework:

Gutenberg:

failed to build module 'Gutenberg'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.8 (swiftlang-5.8.0.124.2 clang-1403.0.22.11.100)', while this compiler is 'Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)'). Please select a toolchain which matches the SDK.

Aztec:

compiling for iOS 14.0, but module 'Aztec' has a minimum deployment target of iOS 15.0
[...]

I'm wondering why they failed since the minimum deployment target is already set to 15.0 🤔:

WordPress-iOS/config/Common.xcconfig

Line 3 in d7cdd21

IPHONEOS_DEPLOYMENT_TARGET = 15.0

Looks like the upstream branch didn't experience this issue, so not sure what's the difference here. I'm using a newer Gutenberg version (1.99.0) but I understand that the XCFramework produced for that version should also work.

cc @mokagio in case you have any insights about this issue. Thanks 🙇 !

dcalhoun

These changes make sense to me.

Builds are failing due to the XCFramework

The builds do fail locally for me too. I recall seeing the error referenced before in eadad98#commitcomment-119894859, which was referenced in wordpress-mobile/gutenberg-mobile@8f74c77 and included in wordpress-mobile/gutenberg-mobile#5924. We may need to include that fix in this branch, as I believe it was only merged into the ongoing React Native upgrade branch.

fluiddot · 2023-07-07T15:30:02Z

The builds do fail locally for me too. I recall seeing the error referenced before in eadad98#commitcomment-119894859, which was referenced in wordpress-mobile/gutenberg-mobile@8f74c77 and included in wordpress-mobile/gutenberg-mobile#5924.

I didn't encounter the failure in my case, but let me check it again cleaning the build caches.

We may need to include that fix in this branch, as I believe it was only merged into the ongoing React Native upgrade branch.

Yep, since this PR is pointing to the latest version integrated v1.99.0, makes sense that the failure is caused due to the lack of the fix (wordpress-mobile/gutenberg-mobile@8f74c77). This makes me wonder how an installable build was produced in the upstream branch (reference) 🤔.

In any case, I agree that incorporating the fix might address the issue. I'll try use a Gutenberg Mobile reference that includes it to validate it 👍.

peril-wordpress-mobile · 2023-07-07T17:05:20Z

	Warnings
⚠️	PR is not assigned to a milestone.

Generated by 🚫 dangerJS

fluiddot · 2023-07-07T17:34:39Z

@dcalhoun I've tested building the app using a local installation of Gutenberg, with the latest changes, and it's building successfully. I also test building using the XCFramework and worked as well. Let me know if you still encounter the issue you mentioned here, thanks 🙇 !

Gutenberg/cocoapods_helpers.rb

wpmobilebot · 2023-07-07T18:00:02Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr21021-170d7f4
	Version	22.8
	Bundle ID	org.wordpress.alpha
	Commit	`170d7f4`
	App Center Build	WPiOS - One-Offs #6444

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2023-07-07T18:01:27Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr21021-170d7f4
	Version	22.8
	Bundle ID	com.jetpack.alpha
	Commit	`170d7f4`
	App Center Build	jetpack-installable-builds #5472

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

fluiddot · 2023-07-10T14:39:00Z

I noticed that UI tests failed consistently in this branch and failures are related to some of the editor's test cases. I'll restart them just in case is something temporary. If they keep failing, we'd need to investigate this further.

fluiddot · 2023-07-11T09:57:33Z

Following an internal discussion with @dcalhoun (p1689007827034449/1688993364.279019-slack-C01NFTM4BMY), we spotted that building iOS locally using Gutenberg via XCFramework leads to a crash when using a physical device.

Error:

dyld[7821]: Library not loaded: @rpath/hermes.framework/hermes
  Referenced from: /private/var/containers/Bundle/Application/CA80B831-AD5D-410D-B804-6FE1C4E24BC3/Jetpack.app/Frameworks/Gutenberg.framework/Gutenberg
  Reason: tried:
  '/private/var/containers/Bundle/Application/CA80B831-AD5D-410D-B804-6FE1C4E24BC3/Jetpack.app/Frameworks/hermes.framework/hermes' (no such file),
  '/private/var/containers/Bundle/Application/CA80B831-AD5D-410D-B804-6FE1C4E24BC3/Jetpack.app/Frameworks/Gutenberg.framework/Frameworks/hermes.framework/hermes' (code signature in <9167F8E3-C14A-3646-A21E-3D29FD7B9660> '/private/var/containers/Bundle/Application/CA80B831-AD5D-410D-B804-6FE1C4E24BC3/Jetpack.app/Frameworks/Gutenberg.framework/Frameworks/hermes.framework/hermes' not valid for use in process: mapped file has no cdhash, completely unsigned? Code has to be at least ad-hoc signed.),
  '/usr/lib/swift/hermes.framework/hermes' (no such file),
  '/private/var/containers/Bundle/Application/CA80B831-AD5D-410D-B804-6FE1C4E24BC3/Jetpack.app/../../Frameworks/hermes.framework/hermes' (no such file),
  '/Users/fluiddot/Library/Developer/Xcode/DerivedData/WordPress-agdhszuzkszsmdbuismubfwivokv/Build/Products/Debug-iphoneos/PackageFrameworks/hermes.framework/hermes' (no such file),
  '/System/Library/Frameworks/hermes.framework/hermes' (no such file)

One of the entries the error output has caught my attention:

[...] Gutenberg.framework/Frameworks/hermes.framework/hermes' not valid for use in process: mapped file has no cdhash, completely unsigned? Code has to be at least ad-hoc signed.

I understand that the Hermes framework is not signed, or at least there's an issue related to signing. However, the installable build isn't affected by this issue. Not sure what's the difference, especially related to signing, which only causes the crash when building locally 🤔.

AliSoftware · 2023-07-11T10:52:54Z

This SO answer led me to this Apple technote, which is a bit old but gave me pause…

An umbrella framework is a framework bundle that contains other frameworks. While it is possible to create umbrella frameworks for macOS apps, doing so is unnecessary for most developers and is not recommended. Umbrella frameworks are not supported on iOS, watchOS, or tvOS.

Which might require us to revisit the whole approach of embedding hermes.framework within Gutenberg.framework (as opposed to using a flat structure for those) 🤔 cc @mokagio

Btw, even if the (ad-hoc-signed) Installable Build seems to work, in practice I'm pretty sure nested frameworks would be rejected by Apple on submission anyway… as they have been in the past for WCiOS (paNNhX-ee-p2). Which is one reason why I wrote a Script Build Phase on WCiOS back then to explicitly detect nested frameworks during builds, to avoid finding out about this AppStore-incompatible setup early on instead of only finding out at submission time… (except if this has changed in more recent iOS versions?)

SiobhyB · 2023-07-17T10:33:15Z

An update on progress so far can be found at p1689191819837949-slack-CSYKQSY8G, which includes a link to documentation of various errors/blockers encountered when attempting to flatten the Hermes XCFramework embed. A few of us on the team have continued attempts to get this working, but to no success so far. It feels like we may really benefit from your expertise @mokagio, if you have availability to take a look. Thank you in advance 🙇‍♀️

fluiddot · 2023-07-17T17:31:22Z

Following the above comments, and as a last try, I explored how to flatten the Hermes framework and managed to do it in this PR. I'm not completely sure this is the best approach, so I'd appreciate any feedback. I haven't tested all scenarios yet, but so far it's solving the issue shared here (i.e. building the app locally using a physical device). I'll share a final confirmation when we manage to generate an installable build 🤞.

cc @mokagio @AliSoftware

mokagio · 2023-07-18T06:15:28Z

Just a note to say that I'm back from AFK and I've started looking into this.

fluiddot · 2023-07-18T08:27:40Z

Using the changes from wordpress-mobile/gutenberg-mobile#5968, I managed to build and run successfully the app locally for both the simulator and physical device. I haven't encountered the crash mentioned in this comment 🎊. Besides, the installable build was also generated by CI successfully.

mokagio · 2023-07-19T06:46:03Z

Gutenberg/cocoapods_helpers.rb

+def apply_rnreanimated_workaround!(dependencies:, gutenberg_path:)
+  # RNReanimated needs React-jsc
+  # Reference: https://github.com/WordPress/gutenberg/pull/51386/commits/9538f8eaf73dfacef17382e1ab7feda40231061f
+  dependencies.push('React-jsc')


This diff removes React-jsc from the list of dependencies at the start of the file, only to add it back here, despite those dependencies only being used in the code path that hits this method.

Is the rationale that React-jsc is required only because of the particular RNReanimated setup that we are using here?

We had to do something similar for the Gutenberg demo app (WordPress/gutenberg@9538f8e) when using the newer version of Reanimated. It's unclear to me why it really needs it, but now that we are using Hermes, probably it's not required. I'll remove it and check if it produces any build failure.

mokagio · 2023-07-19T06:46:33Z

Gutenberg/cocoapods_helpers.rb


-  raise "[Gutenberg] Could not find local Gutenberg at given path #{local_gutenberg_path}" unless File.exist?(local_gutenberg_path)
+  # Use a custom RNReanimated version while we coordinate a fix upstream
+  dependencies.delete('RNReanimated')


For reference, software-mansion/react-native-reanimated#4684

mokagio · 2023-07-19T06:49:51Z

Gutenberg/cocoapods_helpers.rb

+  react_native_path = react_native_path!(gutenberg_path: gutenberg_path)
+  package_json_path = File.join(react_native_path, 'package.json')
+  package_json_content = File.read(package_json_path)
+  package_version_match = package_json_content.match(/"version":\s*"(.*?)"/)


Nitpick: Ruby has built in support for parsing JSONs

Suggested change

package_version_match = package_json_content.match(/"version":\s*"(.*?)"/)

package_json_version = JSON.parse(package_json_content)['version']

This will also required importing Ruby's JSON module

# At the top of the file require 'json'

Yep, I was tempted to use the JSON library but went that way (for no particular reason) to avoid adding it to Gemfile.

Even if you indeed need to explicitly require 'json' to use it, iinm the JSON module is part of Ruby core, so there's no need to add it to the Gemfile.

(Think of if you were using import Foundation in a Swift project 😛 )

Ah, thanks for pointing that out @AliSoftware. Not sure why I thought it wasn't part of the core. I'll use this approach then.

Suggestion applied in 80db7ba.

The pre-install logic is only needed when using local Gutenberg installation. Hence, we can merge it into the `gutenberg_dependencies`, which is only used in that case. Additionally, the React Native modules path has been fixed to point to the local installation. Fix empty line after guard clause lint issue

fluiddot · 2023-07-19T16:01:23Z

Gutenberg/version.rb

@@ -11,8 +11,8 @@
 #
 #   LOCAL_GUTENBERG=../my-gutenberg-fork bundle exec pod install
 GUTENBERG_CONFIG = {
-  # commit: ''
-  tag: 'v1.100.0-alpha1'
+  commit: 'd2eab790e3c4333a059f920ce6c4ced5b9c11112'


Points to wordpress-mobile/gutenberg-mobile#5973 to use the workaround for the Hermes framework.

…mework-and-ui-tests Fix failing UI tests after React Native upgrade in Gutenberg

fluiddot · 2023-07-20T16:36:08Z

Now with UI test failures solved, this PR is ready for a final review.

fluiddot · 2023-07-21T08:56:53Z

Closing in favor of #20956, as these changes need to be merged along with the React Native upgrade 0.71.11.

fluiddot added Gutenberg Editing and display of Gutenberg blocks. Tooling Build, Release, and Validation Tools labels Jul 6, 2023

fluiddot requested review from dcalhoun and mokagio July 6, 2023 16:39

fluiddot marked this pull request as ready for review July 6, 2023 16:39

fluiddot mentioned this pull request Jul 6, 2023

Change the Gutenberg integration to use XCFramework #20717

Closed

4 tasks

dcalhoun reviewed Jul 7, 2023

View reviewed changes

wpmobilebot reviewed Jul 7, 2023

View reviewed changes

Gutenberg/cocoapods_helpers.rb Outdated Show resolved Hide resolved

fluiddot mentioned this pull request Jul 17, 2023

Flatten embedded frameworks when generating XCFramework wordpress-mobile/gutenberg-mobile#5968

Closed

2 tasks

This was referenced Jul 18, 2023

Test alternative Gutenberg + XCFramework + Hermes setup #21128

Closed

Add build phase to detect nested frameworks #21130

Merged

mokagio reviewed Jul 19, 2023

View reviewed changes

fluiddot force-pushed the gutenberg/tweak-pod-deps-by-rn-version branch from 82f2d79 to 1c5fb5f Compare July 19, 2023 14:21

fluiddot requested a review from a team as a code owner July 19, 2023 14:21

fluiddot mentioned this pull request Jul 19, 2023

Do not embed Hermes in the Gutenberg XCFramework. Add to distribution archive instead. wordpress-mobile/gutenberg-mobile#5973

Merged

fluiddot added 2 commits July 19, 2023 17:53

Update RNReanimated pod source to wp-fork-2.17.0

381d49a

Add gutenberg_local_pod helper

ebd4cca

fluiddot added 6 commits July 19, 2023 17:53

Tweak pod dependencies based on the React Native version

2b1138f

Fix typo in post install hook

2d40a68

Move RNReanimated dependency workaround to a separate function

598b038

Fetch React Native version using JSON lib

341f617

Update Gutenberg Mobile ref to use workaround for Hermes framework

551968e

fluiddot force-pushed the gutenberg/tweak-pod-deps-by-rn-version branch from 1c5fb5f to 551968e Compare July 19, 2023 15:59

fluiddot commented Jul 19, 2023

View reviewed changes

fluiddot requested review from mokagio and dcalhoun July 19, 2023 16:07

fluiddot mentioned this pull request Jul 19, 2023

[Gutenberg] Upgrade React Native 0.71.11 #20956

Merged

13 tasks

mokagio and others added 3 commits July 20, 2023 13:11

Use CI toolkit version 2.18.1 for better failure annotations

9ceb947

Add workaround for editor UI tests failing with keyboard not available

4530832

Merge pull request #21143 from wordpress-mobile/test/latest-wip-xcfra…

170d7f4

…mework-and-ui-tests Fix failing UI tests after React Native upgrade in Gutenberg

fluiddot requested a review from SiobhyB July 20, 2023 16:36

fluiddot closed this Jul 21, 2023

jkmassel deleted the gutenberg/tweak-pod-deps-by-rn-version branch July 26, 2024 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Gutenberg] Tweak Pod dependencies based on React Native version used by Gutenberg #21021

[Gutenberg] Tweak Pod dependencies based on React Native version used by Gutenberg #21021

fluiddot commented Jul 6, 2023 •

edited

Loading

fluiddot commented Jul 6, 2023

dcalhoun left a comment

fluiddot commented Jul 7, 2023

peril-wordpress-mobile bot commented Jul 7, 2023 •

edited

Loading

fluiddot commented Jul 7, 2023 •

edited

Loading

wpmobilebot commented Jul 7, 2023 •

edited

Loading

wpmobilebot commented Jul 7, 2023 •

edited

Loading

fluiddot commented Jul 10, 2023

fluiddot commented Jul 11, 2023

AliSoftware commented Jul 11, 2023 •

edited

Loading

SiobhyB commented Jul 17, 2023

fluiddot commented Jul 17, 2023

mokagio commented Jul 18, 2023

fluiddot commented Jul 18, 2023

mokagio Jul 19, 2023

fluiddot Jul 19, 2023

mokagio Jul 19, 2023

mokagio Jul 19, 2023

fluiddot Jul 19, 2023

AliSoftware Jul 19, 2023

fluiddot Jul 19, 2023

fluiddot Jul 19, 2023

fluiddot Jul 19, 2023

fluiddot commented Jul 20, 2023

fluiddot commented Jul 21, 2023

	package_version_match = package_json_content.match(/"version":\s"(.?)"/)
	package_json_version = JSON.parse(package_json_content)['version']

[Gutenberg] Tweak Pod dependencies based on React Native version used by Gutenberg #21021

[Gutenberg] Tweak Pod dependencies based on React Native version used by Gutenberg #21021

Conversation

fluiddot commented Jul 6, 2023 • edited Loading

Description

To test

Regression Notes

fluiddot commented Jul 6, 2023

dcalhoun left a comment

Choose a reason for hiding this comment

fluiddot commented Jul 7, 2023

peril-wordpress-mobile bot commented Jul 7, 2023 • edited Loading

fluiddot commented Jul 7, 2023 • edited Loading

wpmobilebot commented Jul 7, 2023 • edited Loading

wpmobilebot commented Jul 7, 2023 • edited Loading

fluiddot commented Jul 10, 2023

fluiddot commented Jul 11, 2023

AliSoftware commented Jul 11, 2023 • edited Loading

SiobhyB commented Jul 17, 2023

fluiddot commented Jul 17, 2023

mokagio commented Jul 18, 2023

fluiddot commented Jul 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluiddot commented Jul 20, 2023

fluiddot commented Jul 21, 2023

fluiddot commented Jul 6, 2023 •

edited

Loading

peril-wordpress-mobile bot commented Jul 7, 2023 •

edited

Loading

fluiddot commented Jul 7, 2023 •

edited

Loading

wpmobilebot commented Jul 7, 2023 •

edited

Loading

wpmobilebot commented Jul 7, 2023 •

edited

Loading

AliSoftware commented Jul 11, 2023 •

edited

Loading