Remove CocoaPods #23958
Remove CocoaPods #23958
Conversation
…thenticatedImageDownload redirect)
Strange that `pod deintegrate` did not remove it. Maybe it was some custom piece of setup...
We don't have pods anymore
We don't use Pods anymore
mokagio
force-pushed
the
mokagio/remove-cocoapods-christmast-branch
branch
from
1e62678 to
b2e22a1
Compare
|
Hey, nice work. It's a historic moment 😃 I've updated the tests in the target branch. It should be green now. I don't think I should/can review the changes in infra, but you get 👍 |
| end | ||
|
|
||
| abstract_target 'Tools' do | ||
| pod 'SwiftLint', swiftlint_version |
There was a problem hiding this comment.
Where does the SwiftLint version get pinned now? It doesn't appear to be in Gemfile. I'd probably suggest trying SwiftPM since this project is not itself a dependency, so it should be fine.
There was a problem hiding this comment.
Where does the SwiftLint version get pinned now?
The version is defined in .swiftlint.yml:
Line 1 in 388dbb1
As for installing the tool, I'm happy to have a go at setting it up via SwiftPM. I ignored the problem for the context of this PR because there was already a lot going on.
I was also keen to ask everyone in the team what they'd prefer to do for provisioning. @jkmassel and I discussed. in the past the idea of having Apps Infra recommend and maintain a golden path for integration (which was CocoaPods but now needs to be updated) but leave the teams the option to decide how to go about it. For example, some people/team in the past expressed the feeling that running the linter on every build was too much friction when just hammering code together and they'd rather get linter feedback in the PR via CI only.
There was a problem hiding this comment.
I'd suggest removing swiftlint_version from .swiftlint.yml and pinning it using SwiftPM. It should solve both issues: installation and pinning. Wdyt?
Note: I never tried it. I usually use Gemfile for tools.
rather get linter feedback in the PR via CI only
I'm sorry, I don't believe anyone would've preferred this 😄
There was a problem hiding this comment.
I'd suggest removing
swiftlint_versionfrom.swiftlint.ymland pinning it using SwiftPM. It should solve both issues: installation and pinning. Wdyt?
Sounds good, but... currently the linter expects swiftlint_version to be defined in the config file, see https://github.com/Automattic/a8c-ci-toolkit-buildkite-plugin/blob/29064ab1e6e49135f9700d86b7b3279081b9ebc9/bin/run_swiftlint#L25
I think we could satisfy both the SPM and swiftlint_version requirements by reading the SwiftLint version from the YML in the Package file, see https://github.com/wordpress-mobile/WordPress-iOS-Shared/blob/6e8a44120452b08075d1838506bd632a5f83b044/Package.swift#L55-L77
In the past, that approach has given us trouble when implemented in a dependency, but given this is a root project, we should be fine. I'll try it in a dedicated PR.
| tar -xvf Frameworks/Gutenberg.tar.gz -C Frameworks/ -k | ||
| mv -n Frameworks/Frameworks/*.xcframework Frameworks/ | ||
| rm -rf Frameworks/Frameworks Frameworks/dummy.txt | ||
| mkdir -p Frameworks/hermes.xcframework/ios-arm64/dSYMs Frameworks/hermes.xcframework/ios-arm64_x86_64-simulator/dSYMs |
There was a problem hiding this comment.
Given gutenberg-mobile should have an hopefully short time left in the project, I think this rudimental way to fetch it is acceptable.
Also, the version is not likely to change at all, which again makes me comfortable with simply hardcoding it there and duplicating it it in Fastlane.
| #import "WPAuthenticator-Swift.h" | ||
| #import <WordPressAuthenticator/WordPressAuthenticator-Swift.h> |
There was a problem hiding this comment.
The header above was necessary when the lib was distributed via CocoaPods, but now we can access the Xcode-generated one directly.
This is unrelated with the CP removal for Gutenberg, but I noticed it when grepping "CocoaPods".
|
Hey, we've merged the feature branch in |
…pods-christmast-branch There were various conflicts, all of which I addressed by picking the `trunk` version assuming that version was the one already processed after the merge of #23923, which the branch for this work stems from, into `trunk`. Conflicts: Modules/Sources/AsyncImageKit/ImagePrefetcher.swift Modules/Sources/AsyncImageKit/ImageRequest.swift Modules/Sources/AsyncImageKit/Views/AsyncImageView.swift WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved WordPress/Classes/ViewRelated/Blog/Blog Details/Detail Header/BlogDetailHeaderView.swift WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController+SiteActions.swift WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift WordPress/Classes/ViewRelated/Media/External/ExternalMediaPickerViewController.swift WordPress/Classes/ViewRelated/NewGutenberg/NewGutenbergViewController.swift WordPress/Classes/ViewRelated/Notifications/Views/NoteBlockHeaderTableViewCell.swift WordPress/Classes/ViewRelated/Post/Views/PostListCell.swift WordPress/Classes/ViewRelated/Reader/Cards/ReaderCrossPostCell.swift WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCell.swift
I run `grep swiftlint` in the context of replacing the CocoaPods installation with a SwiftPM one, #23958 (comment), and got a hit on `.hound.yml`. We have not been using Hound since at least a year, see paaHJt-68e-p2, so there is no reason to keep carrying the config file in the repo.
This is a replay of #23951 but onto #23923, as discussed with @kean here. I copied the PR description below, but you might want to check the original PR, too, for the inline comments.
Notice the base is #23960, which addresses the build failures from #23923, so that we can verify that CI is green with only the CocoaPods changes. Refer to that PR for explanations for UI tests failures, too.
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:
xcframeworksApp.jsfile from the XCFramework to the build products as ajsbundleto use the new locationTheree things noteworthy:
CLANG_WARN_STRICT_PROTOTYPESandCLANG_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.make dependenciesbefore building the app. It felt a build phase was a bit too much, but it also feels like a line in theREADMEto call attention to this is not enough. Open to suggestions.xcworkspacein favor of using only thexcproject, which makes sense given the workspace was required by the CocoaPods setup which is now gone. I decided not to remove the workspace in this PR just to not add to an already big diff. Also, when I opened the standalone project file, some things weren't working out of the box, so I decide to postpone the move to it.SwiftLint
SwiftLint was previously fetched via CocoaPods and the automation was configured with that in mind. Removing CocoaPods also removes SwiftLint. In the interest of moving on with the removal process, I didn't provide an alternative setup for SwiftLint. The
rake linttask merely prints a "no linter configured" message at the moment. As you can see in CI, this does not affect SwiftLint there, so we still have reliable checks in place.Testing
I published this from the Simulator: https://giotestdotsite.wordpress.com/2025/01/07/via-gutenbegkit-without-cocoapods/ (please ignore the slug, it's indeed mobile-gunteber) — 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
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.
Had a go at publishing a post from the Simulator.
N.A.
PR submission checklist:
RELEASE-NOTES.txtif necessary. - N.A.Testing checklist: