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

Set C++ version for libs #35160

Closed
wants to merge 1 commit into from

Conversation

cipolleschi
Copy link
Contributor

Summary:
With the simplified migration to the new arch, we are offering a function in cocoapods that takes care of configuring all the dependencies for libraries.

That function was not setting the proper version of C++ for the library. This could lead to build problems.

This Diff make sure that the libraries that are created with this function call have the proper C++ version.

NOTE: we already have a post install hook that was setting the the proper C++ version, but that was only working for the project. Plus, we can't read that version from the React-Core podspec because at the podspec definition time, cocoapods has not read that yet. Therefore, I just hardcoded the C++ version on top of the file, so it will be easier to update if we decide to change it.

Changelog

[iOS][Fixed] - Make sure that libraries created with install_modules_dependencies has the right C++ version.

Differential Revision: D40894561

Summary:
With the simplified migration to the new arch, we are offering a function in cocoapods that takes care of configuring all the dependencies for libraries.

That function was not setting the proper version of C++ for the library. This could lead to build problems.

This Diff make sure that the libraries that are created with this function call have the proper C++ version.

NOTE: we already have a post install hook that was setting the the proper C++ version, but that was only working for the project. Plus, we can't read that version from the React-Core podspec because at the podspec definition time, cocoapods has not read that yet. Therefore, I just hardcoded the C++ version on top of the file, so it will be easier to update if we decide to change it.

## Changelog
[iOS][Fixed] - Make sure that libraries created with `install_modules_dependencies` has the right C++ version.

Differential Revision: D40894561

fbshipit-source-id: edb376f2dcf48bdaea39da94ba076f664d77b66d
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Nov 1, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40894561

@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Nov 1, 2022
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 6,991,560 +0
android hermes armeabi-v7a 6,368,096 +0
android hermes x86 7,404,245 +0
android hermes x86_64 7,268,256 +0
android jsc arm64-v8a 8,855,317 +0
android jsc armeabi-v7a 7,593,825 +0
android jsc x86 8,913,067 +0
android jsc x86_64 9,396,414 +0

Base commit: 1e6945e
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 1e6945e
Branch: main

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @cipolleschi in 40ad31e.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 1, 2022
@pull-bot
Copy link

pull-bot commented Nov 1, 2022

PR build artifact for 115e864 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

pull-bot commented Nov 1, 2022

PR build artifact for 115e864 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Pull Request resolved: facebook#35160

With the simplified migration to the new arch, we are offering a function in cocoapods that takes care of configuring all the dependencies for libraries.

That function was not setting the proper version of C++ for the library. This could lead to build problems.

This Diff make sure that the libraries that are created with this function call have the proper C++ version.

NOTE: we already have a post install hook that was setting the the proper C++ version, but that was only working for the project. Plus, we can't read that version from the React-Core podspec because at the podspec definition time, cocoapods has not read that yet. Therefore, I just hardcoded the C++ version on top of the file, so it will be easier to update if we decide to change it.

## Changelog
[iOS][Fixed] - Make sure that libraries created with `install_modules_dependencies` has the right C++ version.

Reviewed By: dmytrorykun

Differential Revision: D40894561

fbshipit-source-id: a5187be2d85888a335d4c033f16fdacaf2c945f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants