-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Create XCFrameworks and retain project compatibility #3071
Conversation
Hey @elliottwilliams thanks for the great effort! I'm not sure however now what to do with this. If I understand correctly this build one .xcframework per platform instead of a combined version (when needed). In the other PR the Can you think of a way of combining both PRs to get to finish line? I'm a bit wary of supporting both combined and singe xcframework packaging because it add a lot of complexity to an already complex project. |
@tmspzz Yeah – I can see the value of building a combined xcframework, and agree with you there's no reason to support combined xcframeworks and one-per-platform xcframeworks. Let me see if I can refactor to make that work! |
@tmspzz updated this to create a combined xcframework. I had to give up the behavior where it would automatically create an xcframework if build settings indicated |
@elliottwilliams thanks! I'll review asap |
On top of being closer to the status quo for Carthage, this makes sense for dependencies which build multiple, unrelated frameworks, where users need control over which ones get linked.
We can't easily write unit tests for this, because Carthage's version of Nimble needs to be upgraded to compile on Swift 5.2+, and that would introduce breaking changes to the tests. So I'd prefer to tackle that later if possible :) We can write integration tests, though. I'd like to get the current suite of bats tests passing with --create-xcframework before merging. |
Just want to say that I'm really excited about this, and I like the strategy of making things just work by default out of the box for existing projects. However, I'm not sure if I don't think it should be a blocker for merging this PR, but I think that longer term that frameworks produced via |
@chrisballinger good points! Definitely agree that there's value in looking at BUILD_LIBRARY_FOR_DISTRIBUTION in the future, but that it's an intentional non-goal for this PR. My understanding is that from Apple's perspective, "distribution" means "a binary framework that you provide to users without source code". If you do ship source code, Apple wants you to use a Swift package, which is much more analogous to Carthage's model. So while BUILD_LIBRARY_FOR_DISTRIBUTION builds might be valuable for us in the future, the status quo is building an xcframework with only the local machine in mind. |
@elliottwilliams BUILD_LIBRARY_FOR_DISTRIBUTION generates a stable swift interface and binary compatible object files that guarantee that the framework can be linked into the app with potentially newer versions of swift. So theoretically speaking, if you |
Exactly, and that's the status quo behavior of Carthage 🙂. Using --cache-builds is the current workflow to handle scenarios like these, and this PR doesn't change that. You can run
before you start building in Xcode to rebuild any out-of-date frameworks. (Depending on how your project is structured, you could even run this as a build phase!) The version file logic will invalidate frameworks inside an xcframework unless:
So if you care a lot about portability of built frameworks, you can set |
…rted Xcode version
Co-authored-by: Elliott Williams <emw@yelp.com>
So when I remove the workout and just run I also pulled this repo and did a |
Run |
Ahhh! Do I need to be on this branch? EDIT: I assume I do. So it now built perfectly thank you so much. I notice I have some pre-built frameworks (e.g. Firebase). Can I assume I should drag in the 2 xcframework’s I’ve now created and remove them from the copy phase. But everything else, remains as is? |
Hmmm... so when I run For pre-built frameworks, I still need to do a full Trying it out, hopefully this works. |
@renep thank you so much for your support/help here, really needed it. Ok, so I’ve managed to get all the frameworks to compile as above which is great. Xcode also compiles for device. However when building for iPhone 12 mini Simulator for example:
This is a pre-built library, so Carthage isn’t building this one. I’m guessing this is unrelated? I can see there are newer releases on their repo, I’ll try updating. EDIT: Also, when I run on device I'm getting an "Image not found" error for another framework |
@elliottwilliams A few of my Carthage dependencies do not get an xcframework built when I run |
@CorbinMontague Downloading binary dependencies is a separate codepath, and is something I intend to address in a follow-up PR. The "Downloading at " is a giveaway that Carthage has found a binary version and is preferring it. Branch's repo has source code available, so you should be able to pass |
@kcharwood See instructions in README.md on this branch: https://github.com/elliottwilliams/Carthage/blob/xcframeworks/README.md#building-platform-independent-xcframeworks-xcode-12-and-above Signing shouldn't be any different than it has been. The only change wrt embedding is that you'll use a Copy Files phase instead of |
I'm using
The last part of the log
Not sure if its relevant but MessageKit has a subdependency. Cartfile:
And Cartfile.private
Has someone an idea what's wrong? |
This could be the case. Try to checkout the MessageKit (and maybe the other dependencies) and see if can be build as xcframwork using: I myself had to update some framework that I use to support xcframeworks. |
@elliottwilliams / @renep sorry to be a pain. I have a closed-source dependency. I assume this is not something you can resolve, so if they don't support Apple Silicon, I'm essentially screwed? EDIT: I've taken the strip-frameworks approach and just added it as a step right after running |
@renep Thanks for your help. I tried that. I had to add
However adding MessageKit as a dependency to my project results in the error message in my previous comment. The same issue also happens for PS: I'm using Version 3.0.0 of MessageKit if it helps reproducing. |
You can ignore this error message. The reason is that this pull request cannot create a zip file for the created binaries yet. As @elliottwilliams already mentioned, this will be part of an additional pull request. |
…ures in common This happens when an xcframework needs to be created. Since we're no longer creating them automatically, a guided error message is the next best thing.
Thanks @elliottwilliams. Off we go 🚀 |
@tmspzz Do you have a rough ETA for a new release? |
Thanks everyone! |
When I use both Can Carthage check if |
@kientux not without additional code changes. My intention was that you'd need to clear out your Carthage/Build folder when first switching to xcframeworks. Though I think it would be reasonable to have the version file logic only consider xcframeworks when |
Hey! I want to propose an alternate xcframeworks implementation to #2801 and #2881. This PR fixes #3019 by combining built frameworks into an xcframework when supported.
I don't mean to steal @tmspzz's thunder here, just wanted to give us another implementation to consider. I'm trying to keep the PR small and retain full compatibility with existing projects.
I've tested this in a large iOS+watchOS project (the Yelp app) and on some smaller multiplatform projects. I'm sure there are edge cases I'm missing -- would love for others to try this branch out and let me know what you find 🙂
Summary of changes
By default, all frameworks built for a dependency will be merged into an xcframework in Carthage/Build.
--platform
is supported -- new build products will be merged into an existing xcframework.--no-create-xcframework
, and is disabled by default on systems without Xcode 12.No configuration changes required for existing dependencies to support xcframeworks.
-allow-internal-distribution
, which prevents it from requiring the usualBUILD_LIBRARY_FOR_DISTRIBUTION=YES
/SKIP_INSTALL=NO
.Xcode 12 is required when creating an xcframework
Version file (
--cache-builds
) support is complete.container
andidentifier
fields that describe their location within the .xcframework directory.container
directory when present.No copy-frameworks support. Users should embed xcframeworks directly in Xcode instead of using
copy-frameworks
.