Skip to content

Move to xcframework bundles #4737

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

Merged
merged 8 commits into from
Jan 30, 2020

Conversation

steipete
Copy link
Contributor

@steipete steipete commented Jan 23, 2020

This PR changes the ZipBuilder script to generate xcframework bundles with dynamic frameworks, adding support for Mac Catalyst. Previously, the script generated fake frameworks which really were static libraries in framework form.

xcframeworks are supported on Xcode 11 and higher and are Apple's recommended way to distribute binary frameworks now. They are also future-proof in case of future architecture changes. The previous "fat" frameworks merged simulator and device binaries together - this concept will break when Apple moves to arm-based Macs.

Downside: Link times might be slightly higher than using static libraries, but iOS 13 comes with dyld3 for everyone, so this should be negligible.

This keeps the older i386 and armv7 architectures.

arm64e is missing here but with some massaging, this can be re-added. Apple doesn't recommend distributing this - folks that want to test arm64e can either tweak the script or use CocoaPods - I wouldn't make it too easy, if we get ABI stability for this architecture in a year and people use an outdated build, things will explode in the most wonderful ways. (Related to #4110)

Last issue: Protobuf is upper-caps named but linked lowercase (?). This requires manual editing of the Info.plist file in the xcframework container, else it will fail on case sensitive file systems (iOS). We could add a step that rewrites the Info.plist but IMO this should be addressed upstream.

Mixing with Binary SDKs

The binaries created from the open source release ca be mixed with the lastest closed-source binaries, since some components (Analytics, Performance) aren't yet open source. However, be wary that some might reference subspecs, so you need to take care of this in your release.json:

[
  {
    "name": "FirebaseCrashlytics",
    "version" : "4.0.0-beta.1"
  },
  {
    "name": "FirebaseRemoteConfig"
  },
  {
    "name": "GoogleUtilities/Network"
  },
  {
    "name": "GoogleUtilities/AppDelegateSwizzler"
  },
  {
    "name": "GoogleUtilities/ISASwizzler"
  },
  {
    "name": "GoogleUtilities/MethodSwizzler"
  }
]

And then call:

swift run ReleasePackager -templateDir $(pwd)/Template -zipPods release.json -keepBuildArtifacts true

If you forget about subspecs, code will be missing and you'll get linker errors like the following:

"_OBJC_CLASS_$_GULAppDelegateSwizzler", referenced from:
      objc-class-ref in GoogleAppMeasurement(APMAnalytics_8b8a060bbe8023e2a8dc40576f3452ac.o)

Replaces fat framwork and adds Mac Catalyst support
Note: This removes i386 and armv7 support, as xcframework doesn’t support these
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @steipete ! Just a few quick comments for now.

The PR will definitely be helpful in accelerating xcframework/Catalyst support!

@@ -316,6 +316,7 @@ enum CocoaPodUtils {

// Include the minimum iOS version.
podfile += """
use_frameworks!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use_frameworks! was purposely not there to build static library frameworks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there's a way for xcframework containers to take either, but I didn't get it to work, since that container then expects just a static binar and not a fake framework with header constructs. With dyld3 being there on iOS 13 that shouldn't be a huge impact, and we can delete quite a bit of the script logic.

Folks with CocoaPods can still choose either.

@paulb777 paulb777 added zip-builder Tools related to building the zip file. Catalyst labels Jan 23, 2020
@steipete
Copy link
Contributor Author

This now works and we'll be shipping https://pdfviewer.io/ with it this week. (iOS + Mac Catalyst)

@paulb777 paulb777 self-assigned this Jan 28, 2020
@paulb777
Copy link
Member

paulb777 commented Jan 30, 2020

Thanks again for the PR!

In my testing, I'm seeing issues with library signature problems like opencv/opencv#15645 and finding the workaround for that incompatible with enabling the keychain entitlement necessary for FirebaseAuth.

Here's a plan for next steps to continue collaborating on this:

  1. Merge this PR to a new branch xcframework-master
  2. Fix the CI issues
  3. Add a --dynamic option for dynamic linking
  4. Merge and update the xcframeworks prototype from @ryanwilson in https://github.com/firebase/firebase-ios-sdk/compare/rw-zip-xcframeworks
  5. Stabilize and validate
  6. Merge (without squash) back to master

For Firebase, we'll likely prioritize xcframeworks composed of frameworks with static libraries, but happy to include support here for dynamic linking on option.

return libPath
}
}

// Cries in Google. Why is this not the same?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely because of CocoaPods module_name overrides like https://github.com/protocolbuffers/protobuf/blob/master/Protobuf.podspec#L15

@paulb777 paulb777 changed the base branch from master to xcframework-master January 30, 2020 18:15
@paulb777 paulb777 merged commit 0422c15 into firebase:xcframework-master Jan 30, 2020
paulb777 pushed a commit that referenced this pull request Feb 14, 2020
* Enable Mac Catalyst in support project

* Update builder to output xcframework

Replaces fat framwork and adds Mac Catalyst support
Note: This removes i386 and armv7 support, as xcframework doesn’t support these

* Add Modile and Info.plist fles

* Generate real dynamic frameworks

* Remove unnecessary file

* Preserve symlinks for Mac SDK

* Fix typo

* Re-enable armv7 and i386
@firebase firebase locked and limited conversation to collaborators Mar 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Catalyst cla: yes zip-builder Tools related to building the zip file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants