Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update XCFramework pods after running bin/generate-podspecs.sh #5846

Merged

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Jun 9, 2023

@jhnstn recently opened #5843 to address a bundle exec pod install failing locally for the XCFramework generation project.

As far as I can tell, the sequence of events that resulted in that failure was:

  1. Some RN dependencies were updated in Gutenberg via [RNMobile] Upgrade compile and target sdk version to Android API 33 WordPress/gutenberg#50731
  2. Those new versions were eventually fetched in this repo (I'm guessing via bin/generate-podspec.js?) in ed9625d
  3. Suddenly, the version in the XCFramework Podfile.lock was pointing to one that no longer existed locally

Assuming that bin/generate-podspec.js is the single point through which we updated the third-party-podspecs, then I think we can avoid this issue in the future by making the script also update the pods used by the XCFramework project, which is what this PR does.

Testing

I tried to run the script locally, but it failed like so:

WARNING! hermes-engine.podspec doesn't have a 'tag' or 'commit' field, or doesn't point to a SHA verified file. Either modify this script to add a patch during the podspec generation or modify the original hermes-engine.podspec in the source repo.
Full log
➜ ./bin/generate-podspecs.sh
Working directory: /Users/gio/Developer/a8c/gutenberg-mobile
If your node_modules folder isn't up-to-date please run 'npm install' first and re-run the script. Is your node_modules folder up-to-date? [y/N] y
Enter the commit hash of previous commit. If this is the first-time running this script, enter 0, then commit generated files and re-rerun the script and this time use the previous commit hash: ddd8905b097ace9177cb2aa5d5414c923ccb482a
Please delete '/Users/gio/Developer/a8c/gutenberg-mobile/third-party-podspecs' folder manually before continuing. This script will re-generate it. Did you delete it? [y/N] y
Generating podspec for boost
Generating podspec for RCT-Folly
Generating podspec for DoubleConversion
Generating podspec for glog
Generating podspec for react-native-blur
   ==> Patching react-native-blur podspec
Generating podspec for RNCMaskedView
Generating podspec for react-native-slider
Generating podspec for RNCClipboard
Generating podspec for RNGestureHandler
Generating podspec for react-native-get-random-values
Generating podspec for BVLinearGradient
Generating podspec for RNReanimated
Generating podspec for react-native-safe-area
Generating podspec for react-native-safe-area-context
Generating podspec for RNScreens
Generating podspec for RNSVG
Generating podspec for react-native-video
Generating podspec for react-native-webview
Generating podspec for RNFastImage
Generating podspec for React-RCTVibration with path Libraries/Vibration
Generating podspec for React-RCTSettings with path Libraries/Settings
Generating podspec for React-RCTAnimation with path Libraries/NativeAnimation
Generating podspec for FBLazyVector with path Libraries/FBLazyVector
Generating podspec for React-RCTActionSheet with path Libraries/ActionSheetIOS
Generating podspec for React-RCTNetwork with path Libraries/Network
Generating podspec for RCTRequired with path Libraries/RCTRequired
Generating podspec for React-RCTImage with path Libraries/Image
Generating podspec for RCTTypeSafety with path Libraries/TypeSafety
Generating podspec for React-RCTLinking with path Libraries/LinkingIOS
Generating podspec for React-RCTText with path Libraries/Text
Generating podspec for React-RCTBlob with path Libraries/Blob
Generating podspec for React-RCTPushNotification with path Libraries/PushNotificationIOS
Generating podspec for React-CoreModules with path React/CoreModules
Generating podspec for FBReactNativeSpec with path React/FBReactNativeSpec
/Users/gio/Developer/a8c/gutenberg-mobile/gutenberg/node_modules/react-native/scripts/react_native_pods_utils/script_phases.rb:51: warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
/Users/gio/Developer/a8c/gutenberg-mobile/gutenberg/node_modules/react-native/scripts/react_native_pods_utils/script_phases.rb:51: warning: Passing trim_mode with the 3rd argument of ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.
   ==> Patching FBReactNativeSpec podspec
Generating React-Codegen
Generating schema from Flow types
Generating native code from schema (iOS)
Generating podspec for React-Core with path .
Generating podspec for React with path .
Generating podspec for React-jsiexecutor with path ReactCommon/jsiexecutor
Generating podspec for Yoga with path ReactCommon/yoga
Generating podspec for React-logger with path ReactCommon/logger
Generating podspec for React-jsinspector with path ReactCommon/jsinspector
Generating podspec for React-bridging with path ReactCommon
Generating podspec for React-cxxreact with path ReactCommon/cxxreact
Generating podspec for React-jsi with path ReactCommon/jsi
Generating podspec for React-perflogger with path ReactCommon/reactperflogger
Generating podspec for React-runtimeexecutor with path ReactCommon/runtimeexecutor
Generating podspec for ReactCommon with path ReactCommon
Generating podspec for React-hermes with path ReactCommon/hermes
Generating podspec for React-callinvoker with path ReactCommon/callinvoker
Generating podspec for React-graphics with path ReactCommon/react/renderer/graphics
Generating podspec for hermes-engine with path sdks/hermes-engine
WARNING! hermes-engine.podspec doesn't have a 'tag' or 'commit' field, or doesn't point to a SHA verified file. Either modify this script to add a patch during the podspec generation or modify the original hermes-engine.podspec in the source repo.

I'm not sure whether the failure is due to the SHA1 I used or there's something currently wrong with hermes-engine.


PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

mokagio added 2 commits June 9, 2023 12:14
The XCFramework pods refer to the local third-party podspecs. When those
are updated, it can happen that `bundle exec pod install` cannot find
the new version. Running `bundle exec pod update` ensures that all
dependencies point to the version defined in the local third-party
podspecs.
@mokagio mokagio requested review from jhnstn and fluiddot June 9, 2023 02:29
@mokagio mokagio enabled auto-merge June 10, 2023 10:42
@SiobhyB SiobhyB self-requested a review June 23, 2023 08:14
dcalhoun added 2 commits June 23, 2023 10:25
Rely upon pushd and popd to simplify the "breadcrumbs" of navigating
into and out of multiple directories.
We are required to run `bin/generate-podspecs.sh` twice to capture the
correct target. This change ensures the xcframework is only updated on
the second run with the correct target, otherwise the following error
occurs:

```
Installing FBReactNativeSpec 0.69.4

[!] Error installing FBReactNativeSpec
[!] /opt/homebrew/bin/git -C /var/folders/c5/mkbb257d7fg4myx88dvsndp80000gn/T/d20230623-6856-98pfen checkout --quiet 0

error: pathspec '0' did not match any file(s) known to git
```
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to address this issue! 🙇🏻

I tried to run the script locally, but it failed like so:

WARNING! hermes-engine.podspec doesn't have a 'tag' or 'commit' field, or doesn't point to a SHA verified file. Either modify this script to add a patch during the podspec generation or modify the original hermes-engine.podspec in the source repo.

[...]

I'm not sure whether the failure is due to the SHA1 I used or there's something currently wrong with hermes-engine.

I circumvented this error by manually setting a tag within my local node_modules directory. That is obviously not a long-term solution, but it let me proceed.

I do not believe the hermes-engine error relates to this PR specifically, but I could be wrong. For hermes-engine, I am unsure whether we need to patch it, filter it out, or something else. My limited understanding of the purpose1 of bin/generate-podspecs.sh inhibits me from knowing the best path forward.

Footnotes

  1. If you are able to share a high level description of its purpose, that would be very helpful for me. "Explain it to me like I'm five." 😄

bin/generate-podspecs.sh Outdated Show resolved Hide resolved
dcalhoun and others added 3 commits June 26, 2023 09:03
Co-authored-by: Gio Lodi <gio.lodi@automattic.com>
…rk-integration

build: Fix generate-podspecs script errors
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 26, 2023

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@dcalhoun
Copy link
Member

  1. If you are able to share a high level description of [the script's] purpose, that would be very helpful for me. "Explain it to me like I'm five." 😄

For posterity, I am answering my own question to the best of my understanding. We have the ./bin/generate-podspecs.sh script to allow the WordPress-iOS project to avoid including any React Native or JavaScript references in its source code or CI tasks. The WordPress-iOS project instead points its Podfile to the Podspec files generated in the gutenberg-mobile repository, which "manages" all things related to React Native. (ref: p9ugOq-29e-p2)

Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

As mentioned in #5846 (review), I do not believe the lingering hermes-engine error relates to the proposed changes. The error occurs without the proposed changes. Therefore, I believe it is safe to merge this work.

Thank you for addressing this issue. 🙇🏻

@mokagio mokagio merged commit 5558db0 into trunk Jun 26, 2023
@mokagio mokagio deleted the mokagio/update-xcframework-pods-after-generating-new-podspecs branch June 26, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants