apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.88k stars 728 forks source link

Carthage dependency cycle #1079

Closed odanu closed 4 years ago

odanu commented 4 years ago

Today, I wanted to upgrade the client to the latest version 0.23.0. As soon as I executed carthage update apollo-ios the Starcream and SQLite dependencies appeared in my Cartfile.resolved. As far as I remember those 2 dependencies are optional, so I just removed those from the Cartfile.resolved. Now, when I perform the carthage bootstrap command, it fails with the error The dependency graph contained a cycle:. As soon as I add those 2 dependencies back, the error disappears.

The full error is

The dependency graph contained a cycle:
FeedKit: 
TPKeyboardAvoiding: 
Crashlytics: 
SkeletonView: 
SwiftyMocky: 
Swinject: 
analytics-ios: 
Fabric: 
SkyFloatingLabelTextField: 
stripe-ios: 
FirebaseAnalyticsBinary: 
intercom-ios: 
Kingfisher: 
Charts: 
apollo-ios: Starscream, SQLite.swift
ios-branch-deep-linking: 
ImageSlideshow: 
RxSwift: 
FirebaseMessagingBinary: 
designatednerd commented 4 years ago

The problem is that there's not a great way to mark those dependencies as optional, or tied to a specific lib within the dependency, with Carthage.

Previously we were putting them in a Cartfile.private because the two libs were set up within the Apollo repo as Git Submodules, and they'd get checked out by Carthage when it checked out the repo. We've switched to using Swift Package manager under the hood instead of submodules, so now they do need to be included or you'll get build errors on the libraries which use those when you build with Carthage.

In theory we could go back to Cartfile.private, but it'd lead to a significantly harder installation process for people using Carthage with either of those two libraries, and it may lead to build errors for people who just run carthage build without specifying which library to build.

It's clear that we can't drop Carthage support until a couple more Swift Evolution proposals are implemented for Swift Package Manager, so at this point, I'm trying to get the least worst installation process for people still using it. It's not ideal that you can't exclude downloading things you're not using, but it's way better than leaving it out and making it much harder for people using the libs that do require them.

odanu commented 4 years ago

Thanks @designatednerd for your time to write such an explicit answer.

designatednerd commented 4 years ago

I have had to do a lot of thinking about Carthage lately 🙃

odanu commented 4 years ago

Yeah, it was noticed ;)

designatednerd commented 4 years ago

Since this is the result of intended behavior, do you mind if we close this out?

odanu commented 4 years ago

@designatednerd sorry for missing this, you had to close it at that time since we clarified the situation. So I will close it now. Thanks again for your time.