-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Import most third party libraries as Swift packages #23378
Merged
Merged
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
Generated by 🚫 Danger |
crazytonyli
changed the title
Tonyli xcode target authenticator
Import most third party libraries as Swift packages
Jun 20, 2024
They are included as part of the 'WordPress' scheme test plan
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
14 tasks
kean
force-pushed
the
tonyli-xcode-target-authenticator
branch
from
July 5, 2024 19:48
8214071
to
13c85e5
Compare
kean
approved these changes
Jul 5, 2024
jkmassel
approved these changes
Jul 5, 2024
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.
Description
This PR replaces most third-party libraries with their Swift Package. That'll bring us much closer to fully get rid of CocoaPods.
From now on, we should add new dependencies as Swift Package. Once new Swift packages are added, you can add them to targets that depend on them.
I imagine, eventually, we can take advantage of the "Modules" local package, where we can add shared code and dependencies. Which will save us from manually click buttons in Xcode and changes the pbxproj files.
Details
The Swift Packages are added manually by using the "Package Dependencies" tab in Xcode, which is a pretty terrible experience comparing to editing a plain text Podfile.
WPKit and WPAuthenticator
Using Swift Package is possible because we now have WPKit and WPAuthenticator inside the repository (see #23366). WPKit, which WPAuthenticator depends, doesn't have full SPM support yet. WPAuthentication of couase can't support SPM because it depends on WPKit. WPAuthenticator depends on third party libraries (like SVProgressHUD) that the apps also depend on. The fact there are common dependencies prevent us from using SPM for the app, because we can't import those common dependencies as Swift Packages and CocoaPods pods.
This PR adds two new targets in the Xcode project, for WPKit and WPAuthenticator. They are plain and simple Xcode targets, which are not integrated with CocoaPods. That means we have to do some "wiring" ourselves.
These two new targets of course still have their dependencies. Their dependencies are added to their target configuration manually. They come from two places: Swift Packages in the "Package Dependencies" of the Xcode project (like SVProgressHUD) and CocoaPods pods (like WordPressUI).
Declaring dependencies ultimately lead to two things: compiled library binary are linked to the end product and library resources are copied to the end product bundle. For Swift Packages, Xcode takes care both for us. However, we have to do some manual configuration to achieve the same with CocoaPods dependencies.
First, we need to set compiler flags to link the dependencies, which is done in the xcconfig files in the
config
directory. Second, we need to copy dependencies' resources bundles, which is done in the scripts inScripts/BuildPhases/<WordPressKit|Authenticator>
directories. Those scripts are run as build phases in their test targets.Other targets
WPKit and WPAuthenticator are the only slightly complicated part. The rest of Swift package integration is pretty plain and simple, because we don't need to think about CocoaPods.
Tests
Regression tests with the installable builds in this PR, especially on features that uses libraries which use external resources (storyboards, images, etc). For example, WPAuthenticator uses storyboard to present its UI.
Next steps
We should create a Test Flight build to do a regression tests.
There are still some libraries (mostly our own ones) in the Podfile. We should look into replacing them with Swift Package one by one.
We can start with adding SPM support to MediaEditor, since it depends on CropViewController which is also a dependency of Kanvas. It should be pretty straightforward to support SPM in MediaEdtiro, since it looks like a pure Swift library.
WordPressShared already supports SPM. However, due to the way it integrates SwiftLint, Xcode fails to add it as Swift Package. See wordpress-mobile/WordPress-iOS-Shared#358.
WordPressUI also supports SPM, but due to this Xcode bug, Xcode fails to compile the project when it's added as SPM. The same issue may also occur with WordPressShared Swift Package.
I think we should try to get WordPressShared and WordPressUI integrated with Swift Packages, which will simplify WPKit and WPAuthenticator setup a lot: The "CopyResources" and "EmbedFrameworks" build phase scripts can be deleted.