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

[WIP] Catalyst support #3235

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Oct 5, 2021

Fixes #2799

This PR adds support for a new --platform macCatalyst when building with --use-xcframeworks.

Important: this is still a draft and not ready for production, but it does work. Feel free to check out the branch and make install it if you want to test it out.

Screen Shot 2021-10-05 at 10 16 17

The code isn't really worth reviewing at the moment. Its current state is more of a proof of concept, and the current implementation likely breaks under some scenarios. However, I was able to build my entire app (Watch Chess) with it for iOS, tvOS, watchOS, and macCatalyst.

Notes:

  • I've removed the manual detection of destination simulators, as well as the distinction of build/archive based on what's being built. Instead, this now uses "generic" destinations, which seem to provide a more deterministic and simpler approach.
  • The current implementation probably breaks anything that doesn't use --use-xcframeworks.
  • Building now uses BUILD_LIBRARY_FOR_DISTRIBUTION, which seems to be required now (see also https://pspdfkit.com/blog/2020/supporting-xcframeworks/)

TODO:

  • Re-add support for copying BCSymbolMaps
  • Fail more gracefully when attempting to build a macCatalyst "slice" for a target without SUPPORTS_MACCATALYST set to YES. Right now it just fails to compile without a clear error.
  • Clean up code in general. I'm all for leaving the code in a better state than I found it, but getting this to work was a lot of trial and error, so I pretty much did the opposite of that.
  • Display an error when trying to build --platform macCatalyst without --use-xcframeworks, as that's not supported by Xcode.

NachoSoto added a commit to NachoSoto/ReactiveSwift that referenced this pull request Oct 5, 2021
This `enum` isn't expected to change. It's important to mark it as `@frozen`
so that clients don't get warnings whem compiling with `BUILD_LIBRARY_FOR_DISTRIBUTION`,
which might be required in a future version of Carthage supporting Catalyst.
(See Carthage/Carthage#3235).
andersio pushed a commit to ReactiveCocoa/ReactiveSwift that referenced this pull request Oct 5, 2021
This `enum` isn't expected to change. It's important to mark it as `@frozen`
so that clients don't get warnings whem compiling with `BUILD_LIBRARY_FOR_DISTRIBUTION`,
which might be required in a future version of Carthage supporting Catalyst.
(See Carthage/Carthage#3235).
@NachoSoto

This comment has been minimized.

@NachoSoto

This comment has been minimized.

@tmspzz

This comment has been minimized.

@NachoSoto

This comment has been minimized.

@tmspzz

This comment has been minimized.

@NachoSoto
Copy link
Contributor Author

Disregard that linking issue, that was the same as ReactiveCocoa/ReactiveSwift#809.
Basically I had conflicting versions of ReactiveSwift. No idea why the app actually built and ran fine on simulator and device, but not when building bitcode 🤷

So bottomline, I can confirm this branch works fine, pending the remaining TODOs.

@stale
Copy link

stale bot commented Jan 8, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 8, 2022
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@NachoSoto
Copy link
Contributor Author

Thanks bot but I do want to finish this and merge it since I'm currently using it for my app.

@stale stale bot removed the stale label Apr 16, 2022
This fixes compilation on Xcode 14. Bitcode is going awaym so `ENABLE_BITCODE` defaults to `NO` now.
@NachoSoto
Copy link
Contributor Author

For anyone using this, it works well on Xcode 14 (especially after my last commit)

public var destination: String? {
switch (self.rawValue, self.variant) {
case (_, .macCatalyst?): return "generic/platform=macOS,variant=Mac Catalyst"
case ("macos", _): return "generic/platform=macOS"

Choose a reason for hiding this comment

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

This is causing an error later when building for macOS

CarthageKit/Xcode.swift:1046: Fatal error: Unexpectedly found nil while unwrapping an Optional value

I believe it should be:

Suggested change
case ("macos", _): return "generic/platform=macOS"
case ("macosx", _): return "generic/platform=macOS"

Choose a reason for hiding this comment

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

Works for me otherwise, and solves my issue (see getsentry/sentry-cocoa#2031)

Will be great to see this merged in. Thanks for you hard work!

@bruno-garcia
Copy link

This will really help us at with the cocoa SDK and building for .NET MAUI 👍

@IsaacMarovitz
Copy link

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catalyst support
5 participants