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

Remove CocoaPods #23951

Closed
wants to merge 27 commits into from
Closed

Remove CocoaPods #23951

wants to merge 27 commits into from

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Jan 7, 2025

Picks up where #23750, but starting from scratch because of too many project file changes since then, and completes the migration away from CocoaPods as the mean of linking the Gutenberg XCFramework.

The sequence of changes is roughly:

  • De-integrate CocoaPods
  • Drag-n-drop the xcframeworks
  • Tweak build settings to work around compilation errors due to the frameworks see fbb0997 and 4a29aea
  • Update the script that copies the React Native App.js file from the XCFramework to the build products as a jsbundle to use the new location
  • Remove the various "if cocoapods" kind of checks from app code and automation

Two things noteworthy:

  • I had to disable a couple of clang checks, CLANG_WARN_STRICT_PROTOTYPES and CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER, because of build failures in the XCFramework imports. It doesn't seem ideal, but also we have increasingly less Objective-C so it should be acceptable. Open to suggestions.
  • @jkmassel 's implementation had a build phase that downloaded the XCFrameworks sources. I omitted it and expect devs to run make dependencies before building the app. It felt a build phase was a bit too much, but it also feels like a line in the README to call attention to this is not enough. Open to suggestions.

Testing

I published this from the Simulator: https://giotestdotsite.wordpress.com/2025/01/07/via-gutenbegkit-without-cocoapods/ — Not sure if the HTML source can show it, but I did check locally that the experimental editor toggle was disabled. Besides, the UI being a bit different between the two helps being aware of it.

Apart from that, I'd say run the prototype build on device and see if the Gutenber-via-XCFramework editor works as expected.

Regression Notes

  1. Potential unintended areas of impact

It's possible that some behavior in the Gutenber-XCFramework editor will be compromised. However, I don't see why, once the editor loads, all the code should be the same.

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

Had a go at publishing a post from the Simulator.

  1. 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. - N.A.
  • I have considered adding accessibility improvements for my changes. - N.A.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. - N.A.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • 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)

mokagio added 13 commits January 6, 2025 17:41
That's for the value `CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER`,
but it's useless to set `$(inherited)` for a Yes or No value because
removing the value from the target will automatically make Xcode read it
from the project, achieving the same result.
Was giving errors in Yoga:

> Double-quoted include "YGEnums.h" in framework header, expected angle-bracketed instead
It resulted in a build failure:

> React.framework/Headers/RCTAppearance.h:16:60:
> A function declaration without a prototype is deprecated in all versions of C
@dangermattic
Copy link
Collaborator

dangermattic commented Jan 7, 2025

2 Warnings
⚠️ Modules/Package.swift was changed without updating its corresponding Package.resolved. Please resolve the Swift packages as appropriate to your project setup (e.g. in Xcode or by running swift package resolve).
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

mokagio added a commit to Automattic/a8c-ci-toolkit-buildkite-plugin that referenced this pull request Jan 7, 2025
As we start removing CocoaPods from some of our projects, we can no
longer assume pods exist locally to install.

See wordpress-mobile/WordPress-iOS#23951 which
prompted this change.
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 7, 2025

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr23951-c61cb2d
Version25.6
Bundle IDorg.wordpress.alpha
Commitc61cb2d
App Center BuildWPiOS - One-Offs #11289
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@mokagio mokagio mentioned this pull request Jan 7, 2025
/// WordPressKit, now imported via CocoaPods, has the `AppExtension Safe API Only` flag set to *true*. Meaning that
/// the host app is, effectively as of now, responsible for presenting any alert onscreen (whenever a HTTP Challenge is
/// received). Capicci?
///
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a very old comment. It didn't seem to explain much to me, so I got rid of it.

@mokagio mokagio requested a review from jkmassel January 7, 2025 08:25
#import "WordPressAuthenticator-Swift.h"
#else
#import <WordPressAuthenticator/WordPressAuthenticator-Swift.h>
#endif
Copy link
Contributor Author

@mokagio mokagio Jan 7, 2025

Choose a reason for hiding this comment

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

This was only useful for the CocoaPods setup. I noticed it grepping for CocoaPods and removed it. There were only two usages anyway, which I updated to the generated header, see below.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 7, 2025

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr23951-c61cb2d
Version25.6
Bundle IDcom.jetpack.alpha
Commitc61cb2d
App Center Buildjetpack-installable-builds #10323
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@mokagio mokagio marked this pull request as ready for review January 7, 2025 08:40
@mokagio mokagio added this to the 25.8 milestone Jan 7, 2025
@mokagio mokagio added the Tooling Build, Release, and Validation Tools label Jan 7, 2025
@kean
Copy link
Contributor

kean commented Jan 7, 2025

I hope it'll merge with #23923. There wasn't anyone available to review stuff, so that's why there is this trunk #2.

mokagio added a commit to Automattic/a8c-ci-toolkit-buildkite-plugin that referenced this pull request Jan 7, 2025
As we start removing CocoaPods from some of our projects, we can no
longer assume pods exist locally to install.

See wordpress-mobile/WordPress-iOS#23951 which
prompted this change.
@mokagio
Copy link
Contributor Author

mokagio commented Jan 8, 2025

I hope it'll merge with #23923.

Love the christmas-feature-branch Nice work.

I'll try moving the changes there today.

@mokagio mokagio mentioned this pull request Jan 8, 2025
14 tasks
@mokagio
Copy link
Contributor Author

mokagio commented Jan 8, 2025

@kean seems like everything worked fine. See #23958 — There are CI failures, but look unrelated to the changes, as they match what's in the base branch, example https://buildkite.com/automattic/wordpress-ios/builds/25312

@mokagio
Copy link
Contributor Author

mokagio commented Jan 8, 2025

There are CI failures, but look unrelated to the changes, as they match what's in the base branch, example buildkite.com/automattic/wordpress-ios/builds/25312

@kean I opened #23960 to start addressing them

@mokagio
Copy link
Contributor Author

mokagio commented Jan 8, 2025

Closing in favor of #23958

@mokagio mokagio closed this Jan 8, 2025
@mokagio mokagio deleted the mokagio/gutenberg-via-xcframework branch January 8, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tooling Build, Release, and Validation Tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants