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 project and installation methods (CocoaPods and Swift Package Manager) for Xcode 15 #56

Merged
merged 1 commit into from May 3, 2024

Conversation

ghost
Copy link

@ghost ghost commented Apr 30, 2024

Background

As of April 29th, 2024, all apps submitted to the App Store must be built using Xcode 15 (ref: Apple News).

Additionally, all third-party SDKs are required/encouraged to include a Privacy Manifest file (ref: Apple Support).

Purpose

The purpose of these changes is to upgrade the project and bump the minimum deployment targets for Xcode 15:

  • iOS 12
  • macOS 10.13
  • tvOS 12
  • watchOS 4

These changes span across the following:

  • Xcode project
  • CocoaPods Podspec file
  • Swift Package Manager Package file

Additionally, the Swift Package Manager Package file points to a new "include" public header folder of symlinks. This allows the project to use angled bracket imports for public headers without having to suppress any warnings from clang.

This PR fulfills the following open issues and pull requests:

NOTE: A separate PR has been created on PINCache with similar changes.

Attachments

PINOperation after being installed via CocoaPods:
PINOperation - Installation - CocoaPods

PINOperation targets after being installed via CocoaPods (blue icon represents a resource bundle target that includes the Privacy Manifest file):
PINOperation - Targets - CocoaPods

PINOperation unit tests passing:
PINOperation - Unit Tests Passing

@@ -1,5 +1,14 @@
# Changelog

## [1.2.3](https://github.com/Pinterest/PINOperation/tree/1.2.3) (TBD)
Copy link
Author

Choose a reason for hiding this comment

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

I can update the version number and release date before merging, per guidance from the maintainers.

Makefile Outdated
# TODO: replace it with "swift test --enable-test-discovery --sanitize=thread" when swiftPM resource-related bug would be fixed.
# https://bugs.swift.org/browse/SR-13560
swift build
swift test --enable-test-discovery --sanitize=thread
Copy link
Author

Choose a reason for hiding this comment

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

The linked SPM bug has been resolved for some time.

s.osx.deployment_target = '10.8'
s.tvos.deployment_target = '9.0'
s.watchos.deployment_target = '2.0'
s.cocoapods_version = '>= 1.13.0'
Copy link
Author

Choose a reason for hiding this comment

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

This is a common pattern used across other popular open source libraries, ensuring the VisionOS platform is recognized and Resource Bundles are properly generated.

pch_PIN = <<-EOS
#ifndef TARGET_OS_WATCH
#define TARGET_OS_WATCH 0
#endif
EOS
s.prefix_header_contents = pch_PIN
s.source_files = 'Source/**/*.{h,m,mm}'
s.source_files = 'Source/*.{h,m}'
Copy link
Author

Choose a reason for hiding this comment

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

This matches the format used in PINCache. I figured it was best to keep things simple and consistent with PINCache.

pch_PIN = <<-EOS
#ifndef TARGET_OS_WATCH
#define TARGET_OS_WATCH 0
#endif
EOS
s.prefix_header_contents = pch_PIN
s.source_files = 'Source/**/*.{h,m,mm}'
s.source_files = 'Source/*.{h,m}'
s.resource_bundles = { 'PINOperation' => ['Source/PrivacyInfo.xcprivacy'] }
Copy link
Author

Choose a reason for hiding this comment

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

This is an important line. Adding a Privacy Manifest to resources would end up clobbering an app's Privacy Manifest. Popular open source libraries (Firebase, Alamofire, etc.) have been using this approach for several months with no issues.

name: "PINOperation",
path: "Source",
exclude: ["Info.plist"],
resources: [.process("PrivacyInfo.xcprivacy")],
Copy link
Author

Choose a reason for hiding this comment

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

The Privacy Manifest file needs to be added and referenced within the Xcode project, which is why we are processing the resource instead of copying.

Copy link
Author

@ghost ghost Apr 30, 2024

Choose a reason for hiding this comment

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

Using angled brackets is the preferred way of importing framework headers. Now that the Swift Package Manager Package file references a public header folder of symlinks, we can use angled brackets as expected.

Copy link
Author

Choose a reason for hiding this comment

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

This is essentially an empty Privacy Manifest, which is what Apple prefers in the case where a third-party SDK doesn't collect data nor use any sensitive APIs.

@ghost
Copy link
Author

ghost commented Apr 30, 2024

Ready for review.

NOTE: I have a similar PR open for PINCache, which I've linked in the PR description.

cc: @garrettmoon @garricn

@ghost
Copy link
Author

ghost commented May 2, 2024

@jparise @woshimaliang @rcancro @garrettmoon @garricn

I'd like to suggest we first work on landing these changes in an official release, and then move over to this PINCache PR. Otherwise, we're going to be blocked in the PINCache PR by the CocoaPods CI test failing during lint (specifically, lint would fail due to the existing minimum deployment target set in PINOperation 1.2.2, as well as lack of VisionOS support).

… Manager) to support building and running with Xcode 15.2
@rcancro rcancro merged commit a74f978 into pinterest:master May 3, 2024
5 checks passed
@rcancro
Copy link
Contributor

rcancro commented May 3, 2024

@jparise @woshimaliang @rcancro @garrettmoon @garricn

I'd like to suggest we first work on landing these changes in an official release, and then move over to this PINCache PR. Otherwise, we're going to be blocked in the PINCache PR by the CocoaPods CI test failing during lint (specifically, lint would fail due to the existing minimum deployment target set in PINOperation 1.2.2, as well as lack of VisionOS support).

Merged! I'll keep an eye on PINCache to kick off builds as required. Thanks for the contributions!!! (any chance you use PINRemoteImage and want to make a change there as well??? :) )

@ghost
Copy link
Author

ghost commented May 3, 2024

@rcancro You rock! Thanks for merging :)

I don't use PINRemoteImage, but if we can get these changes into an official release soon, I'll create a PR for PINRemoteImage that includes the necessary changes 👍

The first thing I'll need from you and the team is to publish a new version of PINOperation to CocoaPods trunk. After that, I can bring in the new version to my PINCache and PINRemoteImage PRs and ensure all CI tests pass and VisionOS is properly supported.

💪

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