Revert "Change FRAMEWORK_SEARCH_PATH for xcframeworks (#1015)" #1081
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 7b8f5a1.
|
@FranzBusch as the original PR author what are your thoughts on this? |
|
Thanks for opening this up @elliottwilliams. When I created the previous PR, we saw some very weird build errors in Xcode when integrating xcframeworks via XcodeGen. The only solution we found was to introduce this frame work search paths setting. However, we just tested our project based on this PR and the latest Xcode version and saw that these build errors are gone now. Probably Xcode fixed them somewhere between 12.3-12.5. So from my side, we can go forward and revert this @yonaskolb |
yonaskolb
approved these changes
May 21, 2021
|
Cool, thanks both. @elliottwilliams, if you could just rebase/merge in master, for the conflicts and to fix Linux |
|
Done. Thank you both! |
yonaskolb
approved these changes
May 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey! Sorry if there's a better way to propose this, but I think there are some issues with #1015 that slipped into production:
As implemented, if you declare a dependency on an XCFramework, then XcodeGen will add every framework bundle inside the XCFramework to the target's FRAMEWORK_SEARCH_PATHS. For example, if I declare
the resulting compile command looks like
This is suboptimal for a number of reasons:
Xcode doesn't need any of these search paths, and if you add an xcframework to a project manually, these search paths are not present. When Xcode builds a target that links an XCFramework, it extracts the appropriate framework from the .xcframework bundle into BUILT_PRODUCTS_DIR. This shows up in build logs as a "ProcessXCFramework" step. So the framework we actually want the compiler to use is not in any of these paths, it's in DerivedData.
If for some reason the extracted framework in DerivedData is missing, these search paths do not provide a useful fallback, because the compiler will attempt to link the first framework it finds, and the framework inside the .xcframework bundle are target-specific. So if I were building for a simulator (target:
arm64-apple-ios13.0-simulator), in the above scenario the compiler would find the framework atDwifft.xcframework/ios-arm64_armv7/Dwifft.framework˙(target:arm64-apple-ios8.0), and emit an error:The extra search paths amount to ~1.5KB of arguments per xcframework in many circumstances. This is a bit of a scalability issue, since the compile command is limited by ARG_MAX (1MB on most macs) and the length of the command scales with the number of xcframeworks.
This change reverts that PR and clarifies how to integrate XCFrameworks in the docs.
I think a preferable solution to the issue described in #1015 would have been to reconfigure the target to reference the xcframework directly (not indirectly via something like
OTHER_LDFLAGS), and to make sure that target was building to the same BUILT_PRODUCTS_DIR as the rest of the project.