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

Support tvOS via SPM #494

Closed
wants to merge 5 commits into from
Closed

Support tvOS via SPM #494

wants to merge 5 commits into from

Conversation

asshoffm
Copy link
Contributor

@asshoffm asshoffm commented Sep 21, 2023

Hi! 👋

This MR adds tvOS support to the distribution via SPM.

Because of the XIBs for tvOS, it is not trivial to have regular targets. Therefore, the framework is now built and distributed as a binary framework.

The import of Down is changed to @_implementationOnly for the SPM build since the compiler cannot resolve the dependency otherwise. See https://forums.swift.org/t/update-on-implementation-only-imports/26996.

The changes do not affect the distribution of cocoa pods and the existing binary frameworks.

For future updates, you only need to run the spm script which generates the SPM binaries.

The supported tvOS and iOS versions are also bumped for Xcode 14 compatibility.

@asshoffm
Copy link
Contributor Author

asshoffm commented Oct 6, 2023

@andresilveirah Did you already have some time to have a first look at the MR?

@andresilveirah
Copy link
Member

Hi @asshoffm just got back from a looong vacation. Looking into your MR now.

@andresilveirah
Copy link
Member

andresilveirah commented Oct 11, 2023

@asshoffm I know it's a bit of an effort, but do you mind opening a PR with only the changes necessary to introduce support to SPM for tvOS (ie. keeping iOS and tvOS minimum version down to 10, not running pod install nor the scripts to update/create the XCFrameworks).
It's not really feasible to review a 200+ files PR and it wouldn't be wise to merge it without a review and internal QA.

@andresilveirah andresilveirah self-assigned this Oct 11, 2023
@asshoffm
Copy link
Contributor Author

asshoffm commented Oct 12, 2023

@andresilveirah Updated the MR 👍🏻 But to be able to test it, you now need to run the build script first.

@andresilveirah
Copy link
Member

@asshoffm thank you, appreciate you taking the time to submit the PR. I moved the code with the generated XCFrameworks to a branch called internal_feature/tvos-spm. There's only a key difference, I rolled the minimum iOS and tvOS version back to .v10. I'd love to bump it to v11, but I'm not sure I can do that just yet. Is it a dealbreaker for you?

Can you give it a final try using branch internal_feature/tvos-spm on your SPM config and let me know if it all works for you? If it does, I'll merge the internal branch and have a new version of the SDK released (probably 7.4.2).

@asshoffm
Copy link
Contributor Author

I tested your branch. On tvOS, we get the error sp_metric_error_invalid_JSON_encode_result.

However it should still prove that the framework is built and integrated correctly.

So yes, both iOS and tvOs work via SPM 👍🏻

@asshoffm
Copy link
Contributor Author

@andresilveirah Are there still open points on your side?

@andresilveirah
Copy link
Member

@asshoffm I've been fighting with XCode 15.0.1 for the last week or so. I was ready to merge, but decided to update XCode and after that the SPM XCFramework stopped archiving via command line.

I think I finally found a solution, just running final tests again and, if it's all good, I'll create a release today still.

@andresilveirah
Copy link
Member

Everything seems to be working again with XCode 15.0.1. Closing in favour of #503 . That PR will be closed and we'll have a release today. Thank you @asshoffm .

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