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

[Cocoapods] min_ios_version_supported -> min_supported_versions #39310

Closed
wants to merge 5 commits into from

Conversation

Saadnajmi
Copy link
Contributor

Summary:

One of the most common diffs we have in React Native macOS is simply extending the platforms key Inside every pod spec to include macOS. React Native tvOS does the same to add tvOS. In the future, React Native may support visionOS, at which point we do the same thing again. Let's define a min_supported_versions hash that can be overridden at one place that is extensible to more platforms, instead of just specifying min_ios_version_supported.

Note: In doing this change, I have set it that React-Hermes.podspec doesn't build for macOS anymore. I think this is safe, since anyone using Hermes on macOS was probably using React Native macOS where we already have a diff to add macOS back?

Changelog:

[IOS] [CHANGED] - Add min_supported_versions helper to cocoa pods scripts

Test Plan:

CI should pass.

@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: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Sep 6, 2023
@dmytrorykun
Copy link
Contributor

@Saadnajmi could you please rebase on the latest main, as there were unrelated CircleCI failures recently.

@Saadnajmi
Copy link
Contributor Author

Saadnajmi commented Sep 6, 2023

@Saadnajmi could you please rebase on the latest main, as there were unrelated CircleCI failures recently.

Uuuh, I seem to have messed up the branch a bit with the rebase, I'll fix shortly

@facebook-github-bot
Copy link
Contributor

@dmytrorykun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@analysis-bot
Copy link

analysis-bot commented Sep 6, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,962,478 -43
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,554,231 -29
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: ef3e771
Branch: main

@Saadnajmi
Copy link
Contributor Author

Should be fixed now!

@facebook-github-bot
Copy link
Contributor

@dmytrorykun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@dmytrorykun
Copy link
Contributor

@Saadnajmi there are some Ruby Test failures. Reproducible locally with

cd packages/react-native/scripts
sh run_ruby_tests.sh

Could you please fix them?

@Saadnajmi
Copy link
Contributor Author

@Saadnajmi there are some Ruby Test failures. Reproducible locally with

cd packages/react-native/scripts
sh run_ruby_tests.sh

Could you please fix them?

Sorry, I think this will still fail actually (seeing that locally). Give me a bit while I learn more about ruby :D

@Saadnajmi
Copy link
Contributor Author

@dmytrorykun looks like we're good now!

@facebook-github-bot
Copy link
Contributor

@dmytrorykun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@github-actions
Copy link

github-actions bot commented Sep 7, 2023

This pull request was successfully merged by @Saadnajmi in 1b78da8.

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

@github-actions github-actions bot added the Merged This PR has been merged. label Sep 7, 2023
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 19, 2024
Summary:

One of the most common diffs we have in React Native macOS is simply extending the `platforms` key Inside every pod spec to include macOS. React Native tvOS does the same to add tvOS. In the future, React Native may support visionOS, at which point we do the same thing again. Let's define a `min_supported_versions` hash that can be overridden at one place that is extensible to more platforms, instead of just specifying `min_ios_version_supported`.

Note: In doing this change, I have set it that `React-Hermes.podspec` doesn't build for macOS anymore. I think this is safe, since anyone using Hermes on macOS was probably using React Native macOS where we already have a diff to add macOS back?

[IOS] [CHANGED] - Add min_supported_versions helper to cocoa pods scripts

Pull Request resolved: facebook#39310

Test Plan: CI should pass.

Reviewed By: NickGerleman

Differential Revision: D49014109

Pulled By: dmytrorykun

fbshipit-source-id: d44fc7b750c70cc263a2c89502c022a0db9a4771
Saadnajmi added a commit to microsoft/react-native-macos that referenced this pull request Jan 20, 2024
)

* updateIphoneOSDeploymentTarget -> updateOSDeploymentTarget (facebook#39570)

Summary:
While merging new commits into React Native macOS, I noticed facebook#39478

I would like to also set `MACOS_DEPLOYMENT_TARGET` in our fork, and thought this slight rename would be something I can do upstream

[Internal] - updateIphoneOSDeploymentTarget -> updateOSDeploymentTarget

Pull Request resolved: facebook#39570

Test Plan: CI should pass

Reviewed By: NickGerleman

Differential Revision: D49514693

Pulled By: ryancat

fbshipit-source-id: b4dafb1f9736d2977510712652cb8097263c489d

* min_ios_version_supported -> min_supported_versions (facebook#39310)

Summary:

One of the most common diffs we have in React Native macOS is simply extending the `platforms` key Inside every pod spec to include macOS. React Native tvOS does the same to add tvOS. In the future, React Native may support visionOS, at which point we do the same thing again. Let's define a `min_supported_versions` hash that can be overridden at one place that is extensible to more platforms, instead of just specifying `min_ios_version_supported`.

Note: In doing this change, I have set it that `React-Hermes.podspec` doesn't build for macOS anymore. I think this is safe, since anyone using Hermes on macOS was probably using React Native macOS where we already have a diff to add macOS back?

[IOS] [CHANGED] - Add min_supported_versions helper to cocoa pods scripts

Pull Request resolved: facebook#39310

Test Plan: CI should pass.

Reviewed By: NickGerleman

Differential Revision: D49014109

Pulled By: dmytrorykun

fbshipit-source-id: d44fc7b750c70cc263a2c89502c022a0db9a4771

* Set macOS deployment target
@bingDBdu
Copy link

bingDBdu commented Feb 5, 2024

image
image
I don't think it can find the min_supported_versions

What should I do more fetch the value from cocoapods helper?

@Saadnajmi
Copy link
Contributor Author

Saadnajmi commented Feb 5, 2024

@bingDBdu Without knowing much of your setup, I see you're using cocoapods 1.15.0, which I think is actually not compatible with React Native at the moment. Also are you on RN 0.73?

@Saadnajmi Saadnajmi deleted the pods branch February 5, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants