Consider module stability when building frameworks

[DavidBrunow]

Don't fail builds because the local Swift version doesn't match the framework Swift version if both are module stable

• Consider whether the framework is built for distribution
• Bikeshed naming
• Reformat spaces to tabs
• Discuss better approaches with reviewers
• Update tests

[DavidBrunow]

I branched off of release/0.34.0 because that seems to be the latest code -- please let me know if this needs to be branched off of master or some other branch instead.

[kenji21]

When compiling with BUILD_LIBRARY_FOR_DISTRIBUTION setting, .swiftinterface files are created within the swiftmodule:

echo 'BUILD_LIBRARY_FOR_DISTRIBUTION=YES'>/tmp/config.xcconfig; XCODE_XCCONFIG_FILE=/tmp/config.xcconfig carthage build --no-skip-current ; rm /tmp/config.xcconfig

tree Carthage/Build/iOS/SwiftiumKit.framework/
Carthage/Build/iOS/SwiftiumKit.framework/
├── Headers
│   ├── Crypto.h
│   ├── SwiftiumKit-Swift.h
│   └── SwiftiumKit.h
├── Info.plist
├── Modules
│   ├── SwiftiumKit.swiftmodule
│   │   ├── arm.swiftdoc
│   │   ├── arm.swiftinterface
│   │   ├── arm.swiftmodule
│   │   ├── arm64-apple-ios.swiftdoc
│   │   ├── arm64-apple-ios.swiftinterface
│   │   ├── arm64-apple-ios.swiftmodule
│   │   ├── arm64.swiftdoc
│   │   ├── arm64.swiftinterface
│   │   ├── arm64.swiftmodule
│   │   ├── armv7-apple-ios.swiftdoc
│   │   ├── armv7-apple-ios.swiftinterface
│   │   ├── armv7-apple-ios.swiftmodule
│   │   ├── armv7.swiftdoc
│   │   ├── armv7.swiftinterface
│   │   ├── armv7.swiftmodule
│   │   ├── i386-apple-ios-simulator.swiftdoc
│   │   ├── i386-apple-ios-simulator.swiftinterface
│   │   ├── i386-apple-ios-simulator.swiftmodule
│   │   ├── i386.swiftdoc
│   │   ├── i386.swiftinterface
│   │   ├── i386.swiftmodule
│   │   ├── x86_64-apple-ios-simulator.swiftdoc
│   │   ├── x86_64-apple-ios-simulator.swiftinterface
│   │   ├── x86_64-apple-ios-simulator.swiftmodule
│   │   ├── x86_64.swiftdoc
│   │   ├── x86_64.swiftinterface
│   │   └── x86_64.swiftmodule
│   └── module.modulemap
└── SwiftiumKit

3 directories, 33 files

maybe presence of such files must be checked in your isModuleStableAPI function

[DavidBrunow]

maybe presence of such files must be checked in your isModuleStableAPI function

I would think that the presence of those files should be checked in the frameworkSwiftVersion method that determines the version of the framework and that effort would be separate from this one. Happy to rethink that if you disagree.

[ikesyo]

IIUC module stability is only effective when .swiftinterface exists (when a framework is built with BUILD_LIBRARY_FOR_DISTRIBUTION=YES), so I agree that the existence must be checked.

[DavidBrunow]

Module stability is only effective when .swiftinterface exists, so I agree that the existence must be checked.

Ahh OK I understand now. Sorry I was a bit dense there. I'll look into that.

[DavidBrunow]

I've added some logic to check for the existence of a .swiftinterface file but I have not found an open source framework that has pre-built binaries and also supports Swift 5.1 and is built for distribution. Do y'all have any examples that I could test against? All the internal frameworks I use are built as XCFrameworks so I can't use them for this testing.

[tmspzz]

@DavidBrunow build Alamofire

[kenji21]

Alamofire hasn't yet support for it : Alamofire/Alamofire#2990

I setted BUILD_LIBRARY_FOR_DISTRIBUTION to YES in this small framework: https://github.com/openium/SwiftiumKit

[DavidBrunow]

Alamofire hasn't yet support for it : Alamofire/Alamofire#2990

I setted BUILD_LIBRARY_FOR_DISTRIBUTION to YES in this small framework: https://github.com/openium/SwiftiumKit

@kenji21 I might be thinking about this wrong, but wouldn't there need to be a new release on that framework for me to test this functionality?

[tmspzz]

@DavidBrunow I would checkout Almofire (or what @kenji21 has posted), change the project and produce the artifact. The create a binary dependency.

[kenji21]

Just pushed a 1.6.0 tag with binary framework generated by the tag-release.sh script : https://github.com/openium/SwiftiumKit/releases/tag/v1.6.0

[DavidBrunow]

Thanks to you both -- I'll test both scenarios.

[DavidBrunow]

Just pushed a 1.6.0 tag with binary framework generated by the tag-release.sh script : https://github.com/openium/SwiftiumKit/releases/tag/v1.6.0

It looks like that binary was built with Swift 4.2.1.

[kenji21]

Sorry, I was re-archiving an in-house app... just updated the zip with one compiled using:

$ xcodebuild -version
Xcode 11.2.1
Build version 11B53

$ carthage build --no-skip-current && carthage archive SwiftiumKit
*** xcodebuild output can be found in /var/folders/7v/dfg0p2vj1z58n65d99l2n1lh0000gp/T/carthage-xcodebuild.T2zD2j.log
*** Building scheme "SwiftiumKit" in SwiftiumKit.xcodeproj
Please update to the latest Carthage version: 0.34.0. You currently are on 0.33.0
*** Found Carthage/Build/iOS/SwiftiumKit.framework
*** Found Carthage/Build/iOS/SwiftiumKit.framework.dSYM
*** Found Carthage/Build/iOS/782D37DE-371F-3CC7-B1CC-F1EF9C87014C.bcsymbolmap
*** Found Carthage/Build/iOS/4AE0812B-5F19-3D66-B666-29E7284FB40D.bcsymbolmap
*** Found Carthage/Build/iOS/909D4621-BA13-3654-A185-9DE9453B0801.bcsymbolmap
*** Found Carthage/Build/iOS/E38D4DA2-8222-3C85-9557-E4836BC7F0F7.bcsymbolmap
*** Created SwiftiumKit.framework.zip

[EmDee]

You can also try Firebase:

binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseABTestingBinary.json" "6.9.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseAnalyticsBinary.json" "6.9.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseRemoteConfigBinary.json" "6.9.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseMessagingBinary.json" "6.9.0"

Actually, I'm not sure if Firebase or Crashlytics was responsible for the error.

Just in case:

binary "Crashlytics.json" "3.14.0"

Where Crashlytics.json:

{
    "3.14.0": "https://storage.googleapis.com/firebase-preview-drop/ios/crashlytics/com.crashlytics.ios-manual.zip"
}

[DavidBrunow]

I was able to successfully test this change using the SwiftiumKit release in Xcode 11.1.

I did not see any issues with the Firebase binaries in any version of Xcode with the change removed so I don't think that is a good test scenario.

I was not able to get the Crashlytics example to work -- I got this error:

*** Downloading binary-only framework Crashlytics at "file:///Users/davidbrunow/Development/SmallSampleProject/Crashlytics.json"
2019-11-07 10:20:35.903677-0600 carthage[30281:4932184] Task <EC8F7C5F-BF40-4187-AB44-04F8E0CE3687>.<3> finished with error [-1100] Error Domain=NSURLErrorDomain Code=-1100 "The requested URL was not found on this server." UserInfo={NSUnderlyingError=0x101021de0 {Error Domain=kCFErrorDomainCFNetwork Code=-1100 "(null)"}, NSErrorFailingURLStringKey=file:///Users/davidbrunow/Development/SmallSampleProject/Crashlytics.json, NSErrorFailingURLKey=file:///Users/davidbrunow/Development/SmallSampleProject/Crashlytics.json, NSLocalizedDescription=The requested URL was not found on this server.}
Failed to read file or folder at /Users/davidbrunow/Development/SmallSampleProject/Crashlytics.json: Error Domain=Result.AnyError Code=1 "The requested URL was not found on this server."

[kenji21]

I was able to successfully test this change using the SwiftiumKit release in Xcode 11.1.

nice to read, did you try both with the binary framework built using Xcode 11.2.1 ? and with the flag --no-use-binaries ?

[DavidBrunow]

I was able to successfully test this change using the SwiftiumKit release in Xcode 11.1.

nice to read, did you try both with the binary framework built using Xcode 11.2.1 ? and with the flag --no-use-binaries ?

Yes I tested with the binary framework built using Xcode 11.2.1 and I just tested with the --no-use-binaries flag and everything builds with no errors.

[MikeKasperlik]

I tested this branch locally and everything seems to work 👍 Thanks.

[EmDee]

Same here. Works like a charm. Thanks, @DavidBrunow !

Also noticed that our own framework didn't have the BUILD_LIBRARY_FOR_DISTRIBUTION flag set to true, so yay 🎉

[simonmitchell]

@ikesyo any idea when this may get merged and distributed in a release? 😊

[EmDee]

Maybe @tmspzz or @mdiep can say something regarding a possible merge/release.

[telip007]

Any updates on this? :)

[hujunfeng]

Thanks, @DavidBrunow. The patch works well.

However, I find that version files needs to be checked against Swift 5.1 too. For Swift frameworks, version files save swiftToolchainVersion inside. Because of this, currently, Carthage will consider a mismatch of versions when resolving Cartfile, if a binary framework is built in 5.1 and the current Swift toolchain is 5.1.3, for example. As a result, Carthage will then download that binary framework again. Although Carthage will not throw the error of incompatible Swift version (because of this patch), it is annoying that it will download such frameworks every time I run carthage bootstrap.

[dlewanda]

Thanks @DavidBrunow for your work here. Heads up that you will need to rebase on the latest master at this point, but otherwise it works for us. We'd love to see this PR get merged ASAP!

[jackmichalak]

@DavidBrunow Thank you. I don't mean to rush, and I know I wouldn't be able to make these changes myself. :)

I know that Swift versions weren't module stable previously, but I've always had multiple versions of Xcode on my computer and which one is selected is basically random. I don't understand why Carthage wants to prevent downloading based on which version of Xcode is selected; we'll get the same error when we try to build if it actually matters, and if it doesn't matter then that's great.

Unfortunately this is for distribution to clients, so I have to support every version of Xcode (particularly since CI isn't always very flexible). If it were internal I'd just be telling people to download my preferred version of Xcode.

[DavidBrunow]

I know that Swift versions weren't module stable previously, but I've always had multiple versions of Xcode on my computer and which one is selected is basically random. I don't understand why Carthage wants to prevent downloading based on which version of Xcode is selected; we'll get the same error when we try to build if it actually matters, and if it doesn't matter then that's great.

I wasn't part of the folks that decided that Carthage shouldn't download frameworks if they are not compatible with the system's Swift version but the decision makes sense to me. I'd rather know sooner rather than later that a framework won't work for my environment and have that framework built for me if I cannot download a compatible version.

[DavidBrunow]

@tmspzz @jdhealy Any chance y'all could take a look at this in the near future?

[jacobmitchinson]

@DavidBrunow Thank you for doing this! It's been a source of much headache in our team! I'd love to see this merged.

[txaiwieser]

I know everyone is trying really hard to make this happen, but is there any blockers to this getting merged?

[rzulkoski]

@tmspzz @jdhealy Any chance y'all could take a look at this in the near future?

Any movement on this?

[Sohil]

@tmspzz @jdhealy Any chance y'all could take a look at this in the near future?

@tmspzz @jdhealy any chance we get a new release with this fix ?

[mozeryansky]

@ikesyo @tmspzz @jdhealy Just posting again on this. Could we get this merged into a new release soon?

[tmspzz]

LGTM

[michaeleisel]

Will there be a release soon that includes this?

[michaeleisel]

Actually, I've tried building Carthage from source with the latest master (c7d6b8dbe), and I still see this problem. When I clone https://github.com/mozilla-mobile/firefox-ios and build with carthage boostrap --platform ios --color auto --cache-builds, I get:

*** Downloading application-services.framework binary at "v0.48.0"
***  Skipped installing application-services.framework binary due to the error:
	"Incompatible Swift version - framework was built with 5.1.3 (swiftlang-1100.0.282.1 clang-1100.0.33.15) and the local version is 5.1.2 (swiftlang-1100.0.278 clang-1100.0.33.9)."

[DavidBrunow]

@michaeleisel When looking at the common.xcconfig used in application-services.framework I don't see the key BUILD_LIBRARY_FOR_DISTRIBUTION which is necessary for module stability. I'm not familiar with that project so please let me know if I am missing something.

[michaeleisel]

Good catch, that's probably it

[dchohfi]

hey, any idea when a new version of carthage is going to be released?

[johnngoi]

@rzulkoski @tmspzz do we have a plan to add this enhancement in a release soon?