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

RUMM-1145 Carthage XCFrameworks support #439

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

buranmert
Copy link
Contributor

@buranmert buranmert commented Mar 10, 2021

Problem

The main thing of carthage is building projects for every possible arch-platform so that they don't need to be rebuilt when the developer changes the target arch-platform.
It was achieving this by lipoing BuildProducts for different arch-platforms and giving one single .framework containing multiple slices.

This model stopped working as arm64-simulator arch-platform came along with Apple Silicon; it duplicated arm64-ios and lipo doesn't merge duplicate slices.

Apple's solution

Apple introduced XCFrameworks to solve this problem: this format can contain multiple .frameworks for different arch-platforms.

Example.xcframework:
- ios-arm64_armv7/Example.framework
- ios-arm64_i386_x86_64-simulator/Example.framework
- macos-arm64_x86_64/Example.framework
- tvos-arm64/Example.framework
- tvos-arm64_x86_64-simulator/Example.framework
Info.plist

Carthage's solution

carthage started supporting this new format starting from 0.37.
carthage also maintains backward compatibility by not breaking projects that are configured for .framework dependencies.
For some reason (this looks like a bug in carthage to me), dd-sdk-ios works only in this backward-compatibility mode although it is configured with .xcframework

Backward compatibility

If the target has FRAMEWORK_SEARCH_PATH set to Carthage/Build/, carthage assumes that it searches for Example.framework whereas there is Example.xcframework instead.
In that case, carthage extracts the content of Example.xcframework and inject its location as an extra FRAMEWORK_SEARCH_PATH to xcodebuild

guard let platformTripleOS = settings.platformTripleOS.value,
  let frameworkSearchPaths = settings.frameworkSearchPaths.value,
  frameworkSearchPaths.contains(where: isRelativeToBuildDirectory) else {
  // don't extract
  return
}
extractXCFramework()
//

Setting such a FRAMEWORK_SEARCH_PATH value triggers XCFramework extraction and compilation succeeds ✅

ℹ️ Note that this build setting is not supposed to be needed for compilation.
ℹ️ Based on my trials, always the second target fails at building for iphonesimulator

Build settings of Kronos

Kronos had VALID_ARCHS in its base xcconfig file and it was not updated after the introduction of arm64-simulator.
I fixed it with MobileNativeFoundation/Kronos#76
I also asked for a hotfix version and I'm waiting for a response.

If Kronos ships a hotfix...

We can update to this version and dd-sdk-ios can be used by Apple Silicon owner carthage users ✅
Sooner or later, we will need to support this arch-platform.

Review checklist

  • If Kronos ships a hotfix, remove the content of Base.xcconfig
  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@buranmert buranmert requested a review from a team as a code owner March 10, 2021 17:30
@buranmert buranmert self-assigned this Mar 10, 2021
@buranmert buranmert force-pushed the buranmert/RUMM-1145-carthage-fix branch from 77e38dd to 9d84210 Compare March 10, 2021 17:31
@buranmert
Copy link
Contributor Author

there is still something that i don't understand:
Datadog target links against Kronos.xcframework (not Kronos.framework)
i don't understand why it still requires extractXCFrameworks
once there is Carthage/Build/Kronos.xcframework in place, it builds without problem from Xcode

@buranmert buranmert force-pushed the buranmert/RUMM-1145-carthage-fix branch from 9d84210 to b1bbbfe Compare March 10, 2021 17:48
1. .frameworks replaced with .xcframeworks
2. FRAMEWORK_SEARCH_PATH is set for both Datadog and DatadogObjc
Otherwise `carthage build` fails with no module found errors
Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Looks good 👌. I'm assuming something extra would have to be done for the crash-reporting branch once it's merged, but that's another work. Let's ship this as a hotfix?

PWD := $(shell pwd)
# TODO: RUMM-760 Remove this workaround once Carthage fixes their Xcode 12 issue
# https://github.com/Carthage/Carthage/issues/3019
export XCODE_XCCONFIG_FILE := $(PWD)/tmp.xcconfig
Copy link
Member

Choose a reason for hiding this comment

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

Great to see it gone 💪

@buranmert
Copy link
Contributor Author

for the records:
this PR is a workaround, carthage is supposed to succeed without FRAMEWORK_SEARCH_PATHS
we can revert this workaround once there is news from Carthage/Carthage#3145

@buranmert buranmert merged commit 97b8747 into master Mar 11, 2021
@buranmert buranmert deleted the buranmert/RUMM-1145-carthage-fix branch March 11, 2021 16:10
buranmert added a commit that referenced this pull request Mar 11, 2021
@buranmert buranmert mentioned this pull request Mar 11, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants