-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix static linking on iOS #6614
Merged
Merged
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
bartlomiejbloniarz
approved these changes
Oct 18, 2024
3 tasks
tomekzaw
added a commit
that referenced
this pull request
Oct 21, 2024
Fixes #6607. This is more of a workaround to fix issue with missing headers when building Reanimated with static linkage for iOS. I spent several hours on trying to make it right but it would work correctly only partially. I will post my findings here or internally later on and I hope to find a better solution sometime in the future. This workaround adds paths to `Common/cpp` and `apple/` directories to `HEADER_SEARCH_PATHS` – the preprocessor looks for headers in these locations. I can't use an absolute path (calculated in Ruby) because it would make the output setup-dependent and thus affect checksums in `Podfile.lock` which is unwanted as some apps check against checksum changes on CI. Tested on fabric-example app with `USE_FRAMEWORKS=static` and without. --- When installing pods in `fabric-example` with `cd ios && bundle install && bundle exec pod install`, the headers are visible in `ios/Pods` directory: ``` $ find . -name "REAUIKit.h" ./Pods/Headers/Public/RNReanimated/reanimated/apple/REAUIKit.h ./Pods/Headers/Private/RNReanimated/reanimated/apple/REAUIKit.h ``` However, after running `USE_FRAMEWORKS=static bundle exec pod install`, the headers are no longer present: ``` $ find . -name "REAUIKit.h" <no output> ``` Because of this, `#import <reanimated/apple/REAUIKit.h>` doesn't work, but when changed back to `#import <RNReanimated/REAUIKit.h>` it works fine (jump to file also works in Xcode). This made me wonder what's the location of `REAUIKit.h`. Obviously, the file is located in `react-native-reanimated/packages/react-native-reanimated/apple/reanimated/apple/REAUIKit.h` (using symlinks as a Development Pod), but the path is not included in header search paths. Hence, I decided to add `react-native-reanimated/packages/react-native-reanimated/apple/` to header search paths. However, this can't be done using absolute paths because they are likely to be machine-specific (e.g. containing the home directory name) which also affects checksum in Podfile.lock which is inconvenient since some setups assume the checksum to be stable (unless the version of the library is changed) due to security concerns. A better idea would be to take inspiration from react-native itself. I've noticed that `ReactCommon` has a similar feature – the headers are in nested subdirectories (e.g. `react/renderer/core/ShadowNode.h`) and the imports don't assume flat structure (e.g. it's `#import <react/renderer/core/ShadowNode.h>` instead of `#import <ReactCommon/ShadowNode.h>`). I remembered that frameworks create `FrameworkName.framework` directories. I finally found `RNReanimated.framework` in Xcode build folder that you can open from menu bar: <img width="312" alt="Screenshot 2024-10-18 at 18 04 45" src="https://github.com/user-attachments/assets/1db95b44-d21c-4e9a-a1d5-19674093094b"> Taking a quick look inside and comparing `ReactCommon` and `RNReanimated`, I've noticed that `RNReanimated.framework` doesn't contain any header files (except for umbrella header) while `ReactCommon` does: <img width="1178" alt="Screenshot 2024-10-18 at 18 05 33" src="https://github.com/user-attachments/assets/2633ab64-a85e-4a04-b2b3-97a11e576d6c"> When I commented out all 3 occurrences of `header_mappings_dir` in RNReanimated.podspec and run the Xcode build (first build lasts until the first error, if you hit the play button once again then, I assume it runs the remaining tasks), the headers were finally there but the directory structure was flattened: ```diff -ss.header_mappings_dir = "Common/cpp/reanimated" +#ss.header_mappings_dir = "Common/cpp/reanimated" ``` <img width="1178" alt="Screenshot 2024-10-18 at 18 14 52" src="https://github.com/user-attachments/assets/c25c1dfe-ce45-45ff-856f-be005907ffe2"> Then I tried passing an absolute path to appropriate directories to see if this would fix the structure: ```diff -ss.header_mappings_dir = "Common/cpp/reanimated" +ss.header_mappings_dir = "/Users/tomekzaw/RNOS/react-native-reanimated/packages/react-native-reanimated/Common/cpp" -sss.header_mappings_dir = "apple/reanimated" +sss.header_mappings_dir = "/Users/tomekzaw/RNOS/react-native-reanimated/packages/react-native-reanimated/ -ss.header_mappings_dir = "Common/cpp/worklets" +ss.header_mappings_dir = "/Users/tomekzaw/RNOS/react-native-reanimated/packages/react-native-reanimated/Common/cpp" ``` Then I investigated `ReactCommon.podspec` inside react-native repository and found several interesting lines of code: ```rb s.header_dir = "ReactCommon" # Use global header_dir for all subspecs for use_frameworks! compatibility ``` It looks like `header_dir` must contain `header_mappings_dir` for the latter to work properly when `use_frameworks!` is enabled. ```rb if ENV['USE_FRAMEWORKS'] s.header_mappings_dir = './' end ``` For some reason, `s.header_mappings_dir` is set to the current directory only if `USE_FRAMEWORKS` is set. ... ``` HEADER_SEARCH_PATHS = ( "$(inherited)", "${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers", "${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core", "${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers", "${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers/platform/ios", "${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/view/platform/cxx", "${PODS_CONFIGURATION_BUILD_DIR}/React-NativeModulesApple/React_NativeModulesApple.framework/Headers", "${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers", "${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios", ); ```
@tomekzaw hi, quick question: when |
@efstathiosntonas It's definitely recommended but not sure if required, I usually run |
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.
Summary
Fixes #6607.
This is more of a workaround to fix issue with missing headers when building Reanimated with static linkage for iOS. I spent several hours on trying to make it right but it would work correctly only partially. I will post my findings here or internally later on and I hope to find a better solution sometime in the future.
This workaround adds paths to
Common/cpp
andapple/
directories toHEADER_SEARCH_PATHS
– the preprocessor looks for headers in these locations. I can't use an absolute path (calculated in Ruby) because it would make the output setup-dependent and thus affect checksums inPodfile.lock
which is unwanted as some apps check against checksum changes on CI.Test plan
Tested on fabric-example app with
USE_FRAMEWORKS=static
and without.My findings
When installing pods in
fabric-example
withcd ios && bundle install && bundle exec pod install
, the headers are visible inios/Pods
directory:However, after running
USE_FRAMEWORKS=static bundle exec pod install
, the headers are no longer present:Because of this,
#import <reanimated/apple/REAUIKit.h>
doesn't work, but when changed back to#import <RNReanimated/REAUIKit.h>
it works fine (jump to file also works in Xcode).This made me wonder what's the location of
REAUIKit.h
. Obviously, the file is located inreact-native-reanimated/packages/react-native-reanimated/apple/reanimated/apple/REAUIKit.h
(using symlinks as a Development Pod), but the path is not included in header search paths. Hence, I decided to addreact-native-reanimated/packages/react-native-reanimated/apple/
to header search paths. However, this can't be done using absolute paths because they are likely to be machine-specific (e.g. containing the home directory name) which also affects checksum in Podfile.lock which is inconvenient since some setups assume the checksum to be stable (unless the version of the library is changed) due to security concerns.A better idea would be to take inspiration from react-native itself. I've noticed that
ReactCommon
has a similar feature – the headers are in nested subdirectories (e.g.react/renderer/core/ShadowNode.h
) and the imports don't assume flat structure (e.g. it's#import <react/renderer/core/ShadowNode.h>
instead of#import <ReactCommon/ShadowNode.h>
).I remembered that frameworks create
FrameworkName.framework
directories. I finally foundRNReanimated.framework
in Xcode build folder that you can open from menu bar:Taking a quick look inside and comparing
ReactCommon
andRNReanimated
, I've noticed thatRNReanimated.framework
doesn't contain any header files (except for umbrella header) whileReactCommon
does:When I commented out all 3 occurrences of
header_mappings_dir
in RNReanimated.podspec and run the Xcode build (first build lasts until the first error, if you hit the play button once again then, I assume it runs the remaining tasks), the headers were finally there but the directory structure was flattened:Then I tried passing an absolute path to appropriate directories to see if this would fix the structure:
After this change, the headers were present and in correct tree structure:
However, the headers were still not found by Xcode when building the project.
While reviewing the changes, I found the following diff in
project.pbxproj
(file is generated by CocoaPods):It looks like react-native overrides
HEADER_SEARCH_PATHS
whenuse_frameworks! :static
is used. So I tried doing the same inRNReanimated.podspec
:Now the headers were correctly found but I got an error about redefinitions:
Then I investigated
ReactCommon.podspec
inside react-native repository and found several interesting lines of code:It looks like
header_dir
must containheader_mappings_dir
for the latter to work properly whenuse_frameworks!
is enabled.For some reason,
s.header_mappings_dir
is set to the current directory only ifUSE_FRAMEWORKS
is set.