-
Notifications
You must be signed in to change notification settings - Fork 743
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
Remove Cartfile #1311
Remove Cartfile #1311
Conversation
We don't actually need this because Xcode uses SPM to get these deps
@soffes: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
I'm somewhat confused by this - doesn't Carthage need to know about the dependencies in order to build them? Or are you saying that Carthage will see the dependencies fetched by SPM? |
Since Apollo.framework, as defined within the Since Carthage will invoke Here's a snippet of the build logs for
By including this Cartfile within the repo, Carthage thinks that it needs to manage all the dependencies within the Cartfile for Apollo, and thus, we end up with two sets of the dependencies: statically linked ones from SPM within Xcode, and dynamically linked ones from Carthage. Since Carthage is gonna invoke |
Oh! Another bit of evidence that we found related to this: We forked SQLite.swift to get around some funky build errors with it we were seeing related to it's usage within our project, and completely removed any and all symbols related to When pointing Carthage at this fork of the dependency, we still encountered that
Even though we had pointed Carthage at our fork of the project. This turned out to be because ApolloSQLite.framework pulled in this dependency via SPM completely outside of Carthage! |
Aha! Thank you for the thorough explanation, that makes a lot more sense now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why circle never ran on this but I'm gonna let it slide since theoretically this file is unnecessary anyway 🙃
Thanks everyone ❤️ |
Is this related to this issue here #1205 |
@teodorpenkov I don't believe so. We encountered the "missing module" issue with SQLite.swift outside of the context of Apollo as well. |
Any ideas why is this happening? I've spent a good amount of hours trying to find the root cause without success. |
@teodorpenkov I have found it to work WAY more consistently after updating to the newest version of carthage that came out a few weeks ago. But yeah prior to that it was a complete crapshoot as to whether it would work and I constantly wanted to throw my laptop in the lake |
Any plans to release Apollo version without this Cartfile? I tried pinning apollo to a commit where this Cartfile is removed and then added SQLite.swift as an explicit dependency, it builded successfully 5 out of 5 times, which never happened before. I'm currently at |
Yes - was planning to bundle a couple other things into the release since it'll need to be a a major, though. In the meantime, you can point your |
We don't actually need this because Xcode uses SPM to get these deps. Specifying the Cartfile causes Carthage to get the dependencies, but Xcode will fetch and use the SPM dependencies instead.
@eliperkins and I spent a long time trying to debug why changing the version of something in the Cartfile in our fork wasn't working and then we realized it was this issue.