-
Notifications
You must be signed in to change notification settings - Fork 24.9k
[ios][precompile] Fix copy symbol files in RNDeps precompile #53353
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
Conversation
Symbol files wasn't copied correctly when building - as with bundles we did overwrite the files and ended up with only the last symbol file. This commit fixes this by mapping the framework build folder architecture type to the xcframework slices creating the correct file structure under the Symbols folder. - Each slice gets a folder with the architecture name under Symbols containing the dSym folder for that slice - Refactored getting correct architecture folder into a separate function. - Refactored target folder lookup in copyBundles - Removed unused async modifier on function
After fixing an isssue with ReactnativeDependencies and how it built symbols (#53353) this commit will align the output of the Symbols folder for the two frameworks. Previously we had an output in the Symbols folder that looked like this (from a local build on my machine) - catalyst - iphone - iphonesimulator After this we now have the more correct arcitecture names on these folders: - ios-arm64 - ios-arm64_x86_64-simulator - ios-arm64_x86_64-maccatalyst This is in line with how the ReactNativeDependencies Symbol folder is set up.
cipolleschi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. I left a minor nit on the naming
Co-authored-by: Riccardo Cipolleschi <[email protected]>
Co-authored-by: Riccardo Cipolleschi <[email protected]>
|
Now returns sorted results.
Now we have multiple symbol files so the old tar command failed.
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D80692019. |
Summary: After fixing an isssue with ReactnativeDependencies and how it built symbols (#53353) this commit will align the output of the Symbols folder for the two frameworks. Previously we had an output in the Symbols folder that looked like this (from a local build on my machine) - catalyst - iphone - iphonesimulator After this we now have the more correct arcitecture names on these folders: - ios-arm64 - ios-arm64_x86_64-simulator - ios-arm64_x86_64-maccatalyst This is in line with how the ReactNativeDependencies Symbol folder is set up. ## Changelog: [IOS] [FIXED] - Aligned Symbols folder in React.xcframework symbols with ReactNativeDependencies.xcframework symbols. Pull Request resolved: #53354 Test Plan: Nightlies Reviewed By: cortinico Differential Revision: D80692098 Pulled By: cipolleschi fbshipit-source-id: e952b087d5dbdeb929b45d9e6d3d7e077c9d05cc
|
@cipolleschi merged this pull request in a843119. |
Summary: After fixing an isssue with ReactnativeDependencies and how it built symbols (facebook#53353) this commit will align the output of the Symbols folder for the two frameworks. Previously we had an output in the Symbols folder that looked like this (from a local build on my machine) - catalyst - iphone - iphonesimulator After this we now have the more correct arcitecture names on these folders: - ios-arm64 - ios-arm64_x86_64-simulator - ios-arm64_x86_64-maccatalyst This is in line with how the ReactNativeDependencies Symbol folder is set up. ## Changelog: [IOS] [FIXED] - Aligned Symbols folder in React.xcframework symbols with ReactNativeDependencies.xcframework symbols. Pull Request resolved: facebook#53354 Test Plan: Nightlies Reviewed By: cortinico Differential Revision: D80692098 Pulled By: cipolleschi fbshipit-source-id: e952b087d5dbdeb929b45d9e6d3d7e077c9d05cc
Summary: Symbol files wasn't copied correctly when building - as with bundles we did overwrite the files and ended up with only the last symbol file. This commit fixes this by mapping the framework build folder architecture type to the xcframework slices creating the correct file structure under the Symbols folder. - Each slice gets a folder with the architecture name under Symbols containing the dSym folder for that slice - Refactored getting correct architecture folder into a separate function. - Refactored target folder lookup in copyBundles - Removed unused async modifier on function ## Changelog: [IOS] [FIXED] - Fixed how we copy and build the Symbols folder when precompiling ReactNativeDependencies Pull Request resolved: facebook#53353 Test Plan: Run nightlies and verify that ReactNativeDependencies.framework.dSym files contains symbol files for all architectures. Reviewed By: cortinico Differential Revision: D80692019 Pulled By: cipolleschi fbshipit-source-id: 77983bc29d1965edf3bc0fcbd9cb3177071991d3
Summary: After fixing an isssue with ReactnativeDependencies and how it built symbols (#53353) this commit will align the output of the Symbols folder for the two frameworks. Previously we had an output in the Symbols folder that looked like this (from a local build on my machine) - catalyst - iphone - iphonesimulator After this we now have the more correct arcitecture names on these folders: - ios-arm64 - ios-arm64_x86_64-simulator - ios-arm64_x86_64-maccatalyst This is in line with how the ReactNativeDependencies Symbol folder is set up. ## Changelog: [IOS] [FIXED] - Aligned Symbols folder in React.xcframework symbols with ReactNativeDependencies.xcframework symbols. Pull Request resolved: #53354 Test Plan: Nightlies Reviewed By: cortinico Differential Revision: D80692098 Pulled By: cipolleschi fbshipit-source-id: e952b087d5dbdeb929b45d9e6d3d7e077c9d05cc
Summary: Symbol files wasn't copied correctly when building - as with bundles we did overwrite the files and ended up with only the last symbol file. This commit fixes this by mapping the framework build folder architecture type to the xcframework slices creating the correct file structure under the Symbols folder. - Each slice gets a folder with the architecture name under Symbols containing the dSym folder for that slice - Refactored getting correct architecture folder into a separate function. - Refactored target folder lookup in copyBundles - Removed unused async modifier on function ## Changelog: [IOS] [FIXED] - Fixed how we copy and build the Symbols folder when precompiling ReactNativeDependencies Pull Request resolved: #53353 Test Plan: Run nightlies and verify that ReactNativeDependencies.framework.dSym files contains symbol files for all architectures. Reviewed By: cortinico Differential Revision: D80692019 Pulled By: cipolleschi fbshipit-source-id: 77983bc29d1965edf3bc0fcbd9cb3177071991d3
|
This pull request was successfully merged by @chrfalch in ba221e1 When will my fix make it into a release? | How to file a pick request? |
Summary:
Symbol files wasn't copied correctly when building - as with bundles we did overwrite the files and ended up with only the last symbol file.
This commit fixes this by mapping the framework build folder architecture type to the xcframework slices creating the correct file structure under the Symbols folder.
Changelog:
[IOS] [FIXED] - Fixed how we copy and build the Symbols folder when precompiling ReactNativeDependencies
Test Plan:
Run nightlies and verify that ReactNativeDependencies.framework.dSym files contains symbol files for all architectures.