cherry-pick fix: find-node.sh now respects .nvmrc (#32712) for 0.67 Stable branch #32955
Conversation
This file contains hidden or 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
Summary: React-native Xcode build steps (such as "Build JS Bundle") rely on `find.node-sh` to find the correct node binary, using nvm if present. We do this because Xcode may run build steps in a fresh shell environment, presumably for repeatable builds. This PR fixes `find-node.sh`, to respect any `.nvmrc` file that may be present in the build environment. Today: `find-node.sh` will set the shell environment to the system node version, and ignores any `.nvmrc` the project may provide to pin node for repeatable builds. By ignoring `.nvmrc`, node versions may differ depending on system environment — between developer laptops, or between developer and CI environments. 😞 This problem has been been noticed before in facebook#8887 ### Should this fix happen upstream? Unfortunately this nvm behavior [is intended](nvm-sh/nvm#2053), for backwards compatibility ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [iOS] [Fixed] - find-node.sh now respects .nvmrc Pull Request resolved: facebook#32712 Test Plan: Before: ```bash # nvm isn't loaded $ which nvm # we're on default system node $ which node && node --version /usr/local/bin/node v17.0.1 $ echo v16.13.0 > .nvmrc $ source ./scripts/find-node.sh # Expected: v16.13.0 $ node --version v17.0.1 ``` After: ```bash # we're on default system node $ which node && node --version /usr/local/bin/node v17.0.1 $ echo v16.13.0 > .nvmrc $ source ./scripts/find-node.sh # Expected: v16.13.0 $ node --version v16.13.0 ``` After (no .nvmrc, should preserve previous behavior): ```bash # we're on default system node $ which node && node --version /usr/local/bin/node v17.0.1 $ source ./scripts/find-node.sh $ nvm ls|grep default default -> v14.17.1 # Expected: v14.17.1 $ node --version v14.17.1 ``` Reviewed By: sota000 Differential Revision: D32889629 Pulled By: ShikaSD fbshipit-source-id: 527384055e303a87bad43413fb66a7fd117d1a63
facebook-github-bot
added
the
CLA Signed
|
|
PR build artifact for fb0332b is ready. |
Base commit: 45244eb |
Base commit: 45244eb |
|
PR build artifact for d10f9b2 is ready. |
Contributor
|
Thanks! We have picked the original commit into 0.67.2! |
facebook-github-bot
pushed a commit
that referenced
this pull request
Feb 2, 2022
Summary: Danger 7 -> 11 ## Changelog [Internal] [Changed] - Updates Danger used in PR checking from v7 to v11 Pull Request resolved: #33027 Test Plan: Works when testing locally ``` > DANGER_GITHUB_API_TOKEN=XXYYZZ yarn danger pr #32955 yarn run v1.22.15 $ node ./node_modules/.bin/danger pr #32955 Starting Danger PR on #32955 --- Accurate Error due to not being able to write to labels --- ## Failures `node` failed. ## Messages :clipboard: Missing Summary - <i>Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.</i> - :clipboard: Missing Test Plan - <i>Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.</i> - :clipboard: Missing Changelog - <i>Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`. <details>CATEGORY may be: - General - iOS - Android - JavaScript - Internal (for changes that do not need to be called out in the release notes) TYPE may be: - Added, for new features. - Changed, for changes in existing functionality. - Deprecated, for soon-to-be removed features. - Removed, for now removed features. - Fixed, for any bug fixes. - Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.</details></i> ## Markdowns Danger: ⅹ Failing the build, there is 1 fail. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` Reviewed By: GijsWeterings Differential Revision: D33941271 Pulled By: cortinico fbshipit-source-id: 359c0076a160a8eeac897a2e1556d3e4d3db5e04
amgleitman
pushed a commit
to amgleitman/react-native-macos
that referenced
this pull request
Apr 20, 2022
Summary: Danger 7 -> 11 [Internal] [Changed] - Updates Danger used in PR checking from v7 to v11 Pull Request resolved: facebook#33027 Test Plan: Works when testing locally ``` > DANGER_GITHUB_API_TOKEN=XXYYZZ yarn danger pr facebook#32955 yarn run v1.22.15 $ node ./node_modules/.bin/danger pr facebook#32955 Starting Danger PR on facebook#32955 --- Accurate Error due to not being able to write to labels --- `node` failed. :clipboard: Missing Summary - <i>Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.</i> - :clipboard: Missing Test Plan - <i>Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.</i> - :clipboard: Missing Changelog - <i>Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`. <details>CATEGORY may be: - General - iOS - Android - JavaScript - Internal (for changes that do not need to be called out in the release notes) TYPE may be: - Added, for new features. - Changed, for changes in existing functionality. - Deprecated, for soon-to-be removed features. - Removed, for now removed features. - Fixed, for any bug fixes. - Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.</details></i> Danger: ⅹ Failing the build, there is 1 fail. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` Reviewed By: GijsWeterings Differential Revision: D33941271 Pulled By: cortinico fbshipit-source-id: 359c0076a160a8eeac897a2e1556d3e4d3db5e04
amgleitman
pushed a commit
to amgleitman/react-native-macos
that referenced
this pull request
Apr 20, 2022
Summary: Danger 7 -> 11 [Internal] [Changed] - Updates Danger used in PR checking from v7 to v11 Pull Request resolved: facebook#33027 Test Plan: Works when testing locally ``` > DANGER_GITHUB_API_TOKEN=XXYYZZ yarn danger pr facebook#32955 yarn run v1.22.15 $ node ./node_modules/.bin/danger pr facebook#32955 Starting Danger PR on facebook#32955 --- Accurate Error due to not being able to write to labels --- `node` failed. :clipboard: Missing Summary - <i>Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.</i> - :clipboard: Missing Test Plan - <i>Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.</i> - :clipboard: Missing Changelog - <i>Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`. <details>CATEGORY may be: - General - iOS - Android - JavaScript - Internal (for changes that do not need to be called out in the release notes) TYPE may be: - Added, for new features. - Changed, for changes in existing functionality. - Deprecated, for soon-to-be removed features. - Removed, for now removed features. - Fixed, for any bug fixes. - Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.</details></i> Danger: ⅹ Failing the build, there is 1 fail. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` Reviewed By: GijsWeterings Differential Revision: D33941271 Pulled By: cortinico fbshipit-source-id: 359c0076a160a8eeac897a2e1556d3e4d3db5e04
shwanton
pushed a commit
to shwanton/react-native-macos
that referenced
this pull request
Feb 13, 2023
Summary: Danger 7 -> 11 ## Changelog [Internal] [Changed] - Updates Danger used in PR checking from v7 to v11 Pull Request resolved: facebook#33027 Test Plan: Works when testing locally ``` > DANGER_GITHUB_API_TOKEN=XXYYZZ yarn danger pr facebook#32955 yarn run v1.22.15 $ node ./node_modules/.bin/danger pr facebook#32955 Starting Danger PR on facebook#32955 --- Accurate Error due to not being able to write to labels --- ## Failures `node` failed. ## Messages :clipboard: Missing Summary - <i>Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.</i> - :clipboard: Missing Test Plan - <i>Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.</i> - :clipboard: Missing Changelog - <i>Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`. <details>CATEGORY may be: - General - iOS - Android - JavaScript - Internal (for changes that do not need to be called out in the release notes) TYPE may be: - Added, for new features. - Changed, for changes in existing functionality. - Deprecated, for soon-to-be removed features. - Removed, for now removed features. - Fixed, for any bug fixes. - Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.</details></i> ## Markdowns Danger: ⅹ Failing the build, there is 1 fail. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` Reviewed By: GijsWeterings Differential Revision: D33941271 Pulled By: cortinico fbshipit-source-id: 359c0076a160a8eeac897a2e1556d3e4d3db5e04
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Helps bring 35bcf93 to 0.67 stable branch