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

Swift Package Manager Support #19

Closed
Sherlouk opened this issue Jan 6, 2021 · 16 comments
Closed

Swift Package Manager Support #19

Sherlouk opened this issue Jan 6, 2021 · 16 comments
Labels
enhancement New feature or request help wanted Extra attention is needed upstream Blocked by something in firebase-ios-sdk repo

Comments

@Sherlouk
Copy link

Sherlouk commented Jan 6, 2021

Hey there 👋

This looks like a super interesting approach to a frustrating side effect of using Firebase - awesome work!

I was just wondering if you have considered publishing a Package.swift as part of this project?

It would allow Swift Package Manager users to benefit from the same speed improvements of an xcframework rather than recompiling every time 🙌

https://developer.apple.com/documentation/swift_packages/distributing_binary_frameworks_as_swift_packages

Thanks 😄

@mikehardy
Copy link
Collaborator

Very interesting! This compile time optimization is large enough I know it would be popular, so I researched this.

It's important to realize that what we're doing here is not actually compiling anything - we are just grabbing the already compiled binaries from the github releases so they may be a direct dependency vs the source:

this shows some of the bits doing the work:

echo "Extracted asset redirect URL:"
echo "$redirect_url"
echo ""
echo "Downloading asset..."
echo ""
curl --fail "$redirect_url" > .tmp/Firebase.zip
echo "Download successful, extracting archive..."
unzip -q .tmp/Firebase.zip 'Firebase/FirebaseFirestore/*' -d .tmp/
# Make sure xcframework exists in extracted folder
if [ ! -d ".tmp/Firebase/FirebaseFirestore/FirebaseFirestore.xcframework" ]; then

Upstream (firebase-ios-sdk) has done a lot of work for Switch Package Manager

...but it appears to support this more work is left to be done:

Then I suppose we would need someone that knows how it works to generate the correct SPM manifest file here, ideally with correct versioning support (right now we just use tags - version support I think is just putting versions into the SPM manifest like https://github.com/firebase/firebase-ios-sdk/pull/6860/files#diff-f913940c58e8744a2af1c68b909bb6383e49007e6c5a12fb03104a9006ae677e)

This would need to be something contributed by the community from someone using it so it would be known-working and supported over time while initial kinks are worked out because it was in actual use

A workplan I think would be to engage with firebase-ios-sdk team (who are very receptive to outside help for these things!) and port cocoapods Firestore symbol remapping to SPM so there were not underlying issues, then enhance ZipBuilder to generate SPM-compatible binaries and put them in the github releases.

Then they would be available so we could pull them here, at which point we'd need our release process script enhanced to generate a Package.swift

non-trivial, but for the time saved on a big project, might pay off over time if anyone was interested

@mikehardy mikehardy added enhancement New feature or request help wanted Extra attention is needed upstream Blocked by something in firebase-ios-sdk repo labels Jan 6, 2021
@Sherlouk
Copy link
Author

Sherlouk commented Jan 6, 2021

@mikehardy So I've done a little bit of digging...

This project works... kind of.

If I use the package in another Swift Package, and then access Firebase via a unit test and run it -- it works. I can see and use the different APIs just fine.

However, if I try to bring it into an actual iOS project and run it - I get an error as it tries to install the app. (It compiles fine)

It seems to be failing on the lack of an Info.plist within one of the frameworks. It feels like this is very close to working (especially given it works in an xctest) but I'm really stumped on this error. Sharing here in case you have any ideas 😄

Full Error
Unable To Install “FirebaseFrameworksExample”
Domain: IXUserPresentableErrorDomain
Code: 1
Failure Reason: Please try again later.
Recovery Suggestion: 
Failed to load Info.plist from bundle at path /Users/sherlock/Library/Developer/CoreSimulator/Devices/65CA1C92-F958-49ED-820A-1C009D48519E/data/Library/Caches/com.apple.mobile.installd.staging/temp.y4zQ7t/extracted/FirebaseFrameworksExample.app/Frameworks/gRPC-Core.framework; 
Extra info about Info.plist: Couldn't stat /Users/sherlock/Library/Developer/CoreSimulator/Devices/65CA1C92-F958-49ED-820A-1C009D48519E/data/Library/Caches/com.apple.mobile.installd.staging/temp.y4zQ7t/extracted/FirebaseFrameworksExample.app/Frameworks/gRPC-Core.framework/Info.plist: No such file or directory

Failed to load Info.plist from bundle at path /Users/sherlock/Library/Developer/CoreSimulator/Devices/65CA1C92-F958-49ED-820A-1C009D48519E/data/Library/Caches/com.apple.mobile.installd.staging/temp.y4zQ7t/extracted/FirebaseFrameworksExample.app/Frameworks/gRPC-Core.framework; Extra info about Info.plist: Couldn't stat /Users/sherlock/Library/Developer/CoreSimulator/Devices/65CA1C92-F958-49ED-820A-1C009D48519E/data/Library/Caches/com.apple.mobile.installd.staging/temp.y4zQ7t/extracted/FirebaseFrameworksExample.app/Frameworks/gRPC-Core.framework/Info.plist: No such file or directory
Domain: MIInstallerErrorDomain
Code: 35
User Info: {
FunctionName = "-[MIBundle _validateWithError:]";
LegacyErrorString = PackageInspectionFailed;
SourceFileLine = 131;
}

System Information

macOS Version 11.1 (Build 20C69)
Xcode 12.2 (17535) (Build 12B45b)
Timestamp: 2021-01-06T17:30:24Z

@mikehardy
Copy link
Collaborator

gRPC-Core is one of the network libraries Firestore transitively depends on, as you swap between using this pre-compiled cocoapods dependency and the Firebase/Firestore cocoapods dependency, you'll see that's one of the Pods that is either out (pre-compiled) or in (source dependency).

That may not help, but it's the context.

No idea why it is missing it's plist, but I'd unpack that zip and/or try https://github.com/mikehardy/rnfbdemo/blob/master/make-demo.sh (for a cocoapods comparison - you can depend on this pre-compiled framework or not to see the difference) and see what's different, maybe get to "why" then "how to fix"

@Sherlouk
Copy link
Author

Sherlouk commented Jan 9, 2021

Just a little update on this... I've done some manual builds and a tonne of investigation and I've found the Info.plist that Xcode is complaining about. If I inject the plist into each of the framework files, everything works.

I still need to investigate exactly why the xcframework that Firebase generate doesn't contain this plist though - ideally an upstream change to introduce this plist would essentially solve all the problems I'm having. Shy of that, it's something that I can "patch" in using a small script (which is what I'll likely do for the sake of an initial POC).

On my machine (2019 MBP) I'm seeing clean build times go from 80s to 1.6s, and incremental builds from 0.5s down to 0.1s.

I still need to:

  • Create a sample project using Firestore to make sure the SDK still works when you hit more complex parts - at the moment it's a super basic test I have going.
  • Once I have the project I also want to trigger some crashes using both the local debugger and something like Crashlytics to see what sort of data we get about a crash within the SDK. The initial crash I've seen seems to give absolutely no information via Xcode besides the assertion message.

The second point on crash reporting is potentially a bit of a blocker for us, so going to invest some time there. I have the dSYMs so if there is a way to symbolicate then that'd be a huge win 👀 if you have more info, feel free to share!

Getting there... real close now!

@mikehardy
Copy link
Collaborator

Wow that's very cool! I think the upstream issue is linked here, so you can pursue the necessary upstream fix (if there is one). We could potentially ingest a workaround here as long as we were tracking progress upstream, in the service of allowing more people to test it and share experience + knowledge + progress

w.r.t. symbolication and dSYMs I've been experimenting with framework builds myself and I think you might be a little state-of-the-art / leading-edge-of-progress here as well. I believe the dSYMs need to be packaged with the .xcframework and we are not doing so.

Here's another .xcframework I collaborate on and probably suffers same issue! invertase/react-native-notifee@3f8b4e2#diff-0dc3949d8de9bd438b6d52f50104c868dc31c70a054e2d0972aaada25a6b9d25

I don't find dSYMs in others either:
https://github.com/OneSignal/OneSignal-iOS-SDK/tree/master/iOS_SDK/OneSignalSDK/OneSignal_XCFramework/OneSignal.xcframework

...but it appears this is cocoapods prior art in the area suggests that for Xcode 12 they need to be next to each framework in the xcframework

CocoaPods/CocoaPods#10111

https://github.com/CocoaPods/CocoaPods/tree/1-10-stable/examples/Vendored-XCFramework12-Example/CoconutLib/build/CoconutLib.xcframework

Xcode 11 cocoapods at least would take dSYMs that were sibling at top level (e.g. https://github.com/bielikb/xcframeworks) but Xcode 12 went past that community convention and now supports the style from the cocoapods issue when you drag and drop an xcframework (all per that cocoapods issue) so I'd focus on that

Carthage is not as far along but mention per-arch dsyms too Carthage/Carthage#2799 (comment)

and then confirmation that is SPM specific, https://forums.swift.org/t/dsym-support-for-se-0272-package-manager-binary-dependencies/37713/5

@paulb777
Copy link

paulb777 commented Jan 9, 2021

I may not be following this exactly right, but Firebase addressed the missing Info.plist issue in firebase/firebase-ios-sdk#7123 - and I verified the upcoming 7.4.0 release has them:

$ find . | grep gRPC | grep Info.plist

./FirebaseFirestore/gRPC-Core.xcframework/ios-x86_64-maccatalyst/gRPC-Core.framework/Info.plist
./FirebaseFirestore/gRPC-Core.xcframework/tvos-arm64/gRPC-Core.framework/Info.plist
./FirebaseFirestore/gRPC-Core.xcframework/ios-arm64_i386_x86_64-simulator/gRPC-Core.framework/Info.plist
./FirebaseFirestore/gRPC-Core.xcframework/tvos-arm64_x86_64-simulator/gRPC-Core.framework/Info.plist
./FirebaseFirestore/gRPC-Core.xcframework/macos-x86_64/gRPC-Core.framework/Info.plist
./FirebaseFirestore/gRPC-Core.xcframework/Info.plist
./FirebaseFirestore/gRPC-Core.xcframework/ios-arm64_armv7/gRPC-Core.framework/Info.plist
./FirebaseFirestore/Resources/gRPCCertificates-Cpp.bundle/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/ios-x86_64-maccatalyst/gRPC-C++.framework/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/tvos-arm64/gRPC-C++.framework/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/ios-arm64_i386_x86_64-simulator/gRPC-C++.framework/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/tvos-arm64_x86_64-simulator/gRPC-C++.framework/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/macos-x86_64/gRPC-C++.framework/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/Info.plist
./FirebaseFirestore/gRPC-C++.xcframework/ios-arm64_armv7/gRPC-C++.framework/Info.plist

@mikehardy
Copy link
Collaborator

I think you are following and that should address the plist part @paulb777, leaving dSYMs integration as the remaining bit?

@Sherlouk
Copy link
Author

Sherlouk commented Jan 9, 2021

So I have found that the xcodebuild -create-xcframework command does allow you to specify a -debug-symbols flag which will accept paths to either a .dSYM file or .bcsymbolmap file. These files are moved to the right place in the xcframework and the Info.plist does reference them.

These links were particularly helpful:
https://developer.apple.com/forums/thread/655768
https://pspdfkit.com/guides/ios/9.5/getting-started/integrating-pspdfkit/#integrating-the-xcframework

I have managed to create an xcframework for Firebase which has dSYMs, Bitcode symbol mapping, headers, the Info.plist it needs all wrapped in a Package.swift file. I've tested it in a sample app showing that I can access a Firestore document successfully.

However, even with all the symbol files being there I'm not sure it actually has a huge impact on crash stacktraces - in fact with or without it feels like you get the same amount of information (I've not rigorously tested this, admittedly - so still some investigation needed)

I've pushed up my frameworks and the SPM stuff here for now (just testing a remote package installation): https://github.com/Sherlouk/firebase-ios-sdk-precompiled

edit:// looks like Xcode hates the fact I'm using LFS for the binaries so not working remotely.
edit2:// above is an issue with Xcode/SPM and with a quick workaround proves that the package does work as expected 🎉

@Sherlouk
Copy link
Author

Sherlouk commented Jan 9, 2021

That's awesome @paulb777! I'm tending to agree with you there @mikehardy but from what I've seen the symbols are really a "nice to have" in the fact that everything seems to work perfectly without them.

The plists were the only thing that needed to be there.

In theory with 7.4.0, we can slap a basic Package.swift in here and we'll be good to go.

@Salakar
Copy link
Member

Salakar commented Mar 5, 2021

In theory with 7.4.0, we can slap a basic Package.swift in here and we'll be good to go.

Fancy sending a PR for this @Sherlouk? 🤓

@Sherlouk
Copy link
Author

Sherlouk commented Mar 5, 2021

Hey @Salakar, so I do have a fork where I experimented with this but unfortunately I was running into numerous issues with certain packages not being available or just straight up not compiling.

I was planning on waiting for Xcode 12.5 (which is now in beta) and trying again to see if things had improved

@mikehardy
Copy link
Collaborator

I chatted with @paulb777 about this, and upstream they'll work on a uniform / more coherent binary distribution strategy now that the need is more understood but it isn't the highest thing on the priority queue, especially with this as a stopgap. So - if someone wanted to PR a change here that would implement the SPM Package.swift generate as well, I'd be happy to merge it to continue the temporary support / bridge the existing binary zip built upstream to downstream consumers for binary integration. Seems it would probably be a useful continuation of this strategy for some months yet or more at least.

@WedgeSparda
Copy link

Hi, any update related to this?

@paulb777
Copy link

paulb777 commented Oct 6, 2021

Hi @WedgeSparda We're tracking in firebase/firebase-ios-sdk#6564, but no implementation yet.

@0x7067
Copy link

0x7067 commented May 17, 2023

Hi, is there any update on this?

@paulb777
Copy link

Binary SPM support for Firestore was released in Firebase 10.8.0

@Salakar Salakar closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed upstream Blocked by something in firebase-ios-sdk repo
Projects
None yet
Development

No branches or pull requests

6 participants