Skip to content

[Cocoapods] min_ios_version_supported -> min_supported_versions - #39310

Closed
Saadnajmi wants to merge 5 commits into
react:mainfrom
Saadnajmi:pods
Closed

[Cocoapods] min_ios_version_supported -> min_supported_versions#39310
Saadnajmi wants to merge 5 commits into
react:mainfrom
Saadnajmi:pods

Conversation

@Saadnajmi

Copy link
Copy Markdown
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-botfacebook-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
Copy Markdown

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

@Saadnajmi

Saadnajmi commented Sep 6, 2023

Copy link
Copy Markdown
ContributorAuthor

@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
Copy Markdown
Contributor

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

@analysis-bot

analysis-bot commented Sep 6, 2023

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,962,478-43
androidhermesarmeabi-v7an/a--
androidhermesx86n/a--
androidhermesx86_64n/a--
androidjscarm64-v8a9,554,231-29
androidjscarmeabi-v7an/a--
androidjscx86n/a--
androidjscx86_64n/a--

Base commit: ef3e771
Branch: main

@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

Should be fixed now!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@dmytrorykun

Copy link
Copy Markdown

@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
Copy Markdown
ContributorAuthor

@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
Copy Markdown
ContributorAuthor

@dmytrorykun looks like we're good now!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown

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

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

@github-actionsgithub-actionsBot 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: react#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 (react#39570)
Summary:
While merging new commits into React Native macOS, I noticed react#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: react#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 (react#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: react#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
Copy Markdown

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

Saadnajmi commented Feb 5, 2024

Copy link
Copy Markdown
ContributorAuthor

@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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.p: MicrosoftPartner: MicrosoftPartnerShared with MetaApplied 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

@Saadnajmi@dmytrorykun@facebook-github-bot@analysis-bot@bingDBdu