Fix Build Warnings for SPM with Xcode 12.5 Beta 2#1661
Closed
joesus wants to merge 1 commit intofacebook:masterfrom
Closed
Fix Build Warnings for SPM with Xcode 12.5 Beta 2#1661joesus wants to merge 1 commit intofacebook:masterfrom
joesus wants to merge 1 commit intofacebook:masterfrom
Conversation
joesus
requested review from
jingping2015,
kalendae and
robtimp
as code owners
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D26609865 |
5 tasks
Summary: Pull Request resolved: facebook#1661 Swift Package Manager (SPM) by convention will treat any files in the `include` directory of a target as public. Therefore they need to be present in the umbrella header. Typically this is the header with the target name, ex: `FBSDKCoreKit.h`. For the target `FBSDKCoreKit_Basics` this means removing the internal header `FBSDKUserDataStore+Internal.h` since internal headers should not go in the `include` directory. Some internal methods are used by the meta indexer class but these were already declared in a private extension. Long term we will want to make these methods public but for now it makes sense to avoid adding them to the public interface. For the target `FBSDKCoreKit` we have two types that are only compiled for tvOS. Since we already have them compilation-gated inline, it is not necessary to exclude them from compilation in the umbrella header. It would be better to do this but it is not strictly necessary and this fixes the warning. The part I really don't like about this fix is that it forces us to make those headers public in the Xcode project for targets that they really shouldn't be public for. Differential Revision: D26609865 fbshipit-source-id: 6712ab9e5eb73beb7ae43164cb23b82323244740
joesus
force-pushed
the
export-D26609865
branch
from
937f82f to
b3939e5
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D26609865 |
Contributor
|
This pull request has been merged in 46e385c. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary:
Swift Package Manager (SPM) by convention will treat any files in the
includedirectory of a target as public. Therefore they need to be present in the umbrella header. Typically this is the header with the target name, ex:FBSDKCoreKit.h.For the target
FBSDKCoreKit_Basicsthis means removing the internal headerFBSDKUserDataStore+Internal.hsince internal headers should not go in theincludedirectory. Some internal methods are used by the meta indexer class but these were already declared in a private extension. Long term we will want to make these methods public but for now it makes sense to avoid adding them to the public interface.For the target
FBSDKCoreKitwe have two types that are only compiled for tvOS. Since we already have them compilation-gated inline, it is not necessary to exclude them from compilation in the umbrella header. (It would be better to do this but it is not strictly necessary and this fixes the warning)Differential Revision: D26609865