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

[constants] Fix build phase error in xcode for nodejs possibly not found in nvm #14047

Merged
merged 3 commits into from
Aug 17, 2021

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Aug 13, 2021

Why

if user does not install nodejs by homebrew but something like nvm or volta. typically these will setup shell $PATH. however, building in xcode does not honor login shell rc file and getting errors for nodejs not found.

How

execute the script with bash -l that will honor login shell settings.

Test Plan

pass building bare-expo and expo-go in xcode.

Checklist

@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Aug 13, 2021
@Kudo Kudo changed the title @kudo/bare expo shell fix [constants] Fix build phase error in xcode for nodejs possibly not found in nvm Aug 13, 2021
@Kudo Kudo requested a review from brentvatne August 13, 2021 15:42
@Kudo Kudo marked this pull request as ready for review August 13, 2021 15:42
@Kudo Kudo requested a review from bbarthec as a code owner August 13, 2021 15:42
@Kudo Kudo removed the request for review from bbarthec August 13, 2021 15:42
Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if node isn't set up to work without the login shell, i imagine builds won't work anyways, right? because react-native's bundle script will fail. that said, this seems reasonable in order to avoid being the cause of the failure.

@Kudo
Copy link
Contributor Author

Kudo commented Aug 15, 2021

if node isn't set up to work without the login shell, i imagine builds won't work anyways, right? because react-native's bundle script will fail. that said, this seems reasonable in order to avoid being the cause of the failure.

exactly. react-native expects to setup $NODE_BINARY in react-native-xcode.sh and will show error if node not found. however, in codegen script, there is also a bash -l -c. i would say the bash -c -l strategy isn't a 100% working solution but may reduce failure rate and no hurt to do so.

@Kudo Kudo force-pushed the @kudo/bare-expo-shell-fix branch from 44d393f to b0a11c7 Compare August 17, 2021 11:28
@Kudo Kudo force-pushed the @kudo/bare-expo-shell-fix branch from b0a11c7 to f0070e0 Compare August 17, 2021 13:33
@Kudo Kudo merged commit 7c591bb into master Aug 17, 2021
@Kudo Kudo deleted the @kudo/bare-expo-shell-fix branch August 17, 2021 13:33
FelipeACP pushed a commit to FelipeACP/expo that referenced this pull request Sep 18, 2021
…und in nvm (expo#14047)

# Why

if user does not install nodejs by homebrew but something like nvm or volta. typically these will setup shell `$PATH`. however, building in [xcode does not honor login shell rc file](https://mgrebenets.github.io/xcode/2019/04/04/xcode-build-phases-and-environment) and getting errors for nodejs not found.

# How

execute the script with `bash -l` that will honor login shell settings.

# Test Plan

pass building bare-expo and expo-go in xcode.

# Checklist

- [x] Documentation is up to date to reflect these changes (eg: https://docs.expo.io and README.md).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants