-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Fix path to node in .xcode.env.local #43333
Conversation
Summary: This change fixes facebook#43285. Basically, when using a `yarn` alias to install pods, yarn creates a copy of the `node` and `yarn` executables and the `command -v node` command will return the path to that executable. Differential Revision: D54542774
This pull request was exported from Phabricator. Differential Revision: D54542774 |
For the sake of fully talking this out: I think there are drawbacks to hardcoding a path for node. This breaks the model of many version managers; for example, if i'm using nvm, this is going to store I don't fully understand the original problem that this was trying to solve, so it's hard to suggest other fixes. I see on #38879 you mention
If it's 1, can we evaluate |
Hi @lindboe in this change we are not hardcoding the path to node. We are asking to the command So, this should work for all the other package managers as well.The problem here is when the The only way to work around that is by using the absolute paths to the executable, which might be an absolute path provided by As of priorities,
No, this will leave you in the same state as not having the
We are already generating it at the earliest time: at |
Thanks @cipolleschi my understanding is improved. I'm still not clear about a couple of things, though, will break that into smaller parts: Hardcoding path to node
I am confused by this statement, when I pull down this patch and test it, the contents of my Auto-generating .xcode.env.localI am still somewhat confused, if we can auto-generate the correct contents for Making version managers work in XCode scriptsTools like nvm not working in XCode shell scripts is maybe an issue with their setup instructions, and I even see on the react native setup docs is mentions this: https://reactnative.dev/docs/environment-setup#optional-configuring-your-environment. If you move setup to SummaryI think it might be preferable to always dynamically evaluate the path to node on each script run, and instead help anyone with a version manager understand that they can either:
|
Sorry for the long delay. I was on PTO for a few weeks. |
@cipolleschi any updates here? Thanks in advance! |
Sorry, I had to work on higher priority tasks and then React Conf trampled all my plans. I need a couple of extra weeks as we are in planning session + AppJS is coming up this week. :( |
It'd be nice to have a flag to opt-out of this behaviour when the update goes in (for now we're using We use volta for our node version management, our With a hardcoded path being written to |
This doesn't seem to fix it for me in an environment managed with
The generated |
This pull request has been merged in 8408b8b. |
This pull request was successfully merged by @cipolleschi in 8408b8b When will my fix make it into a release? | How to file a pick request? |
Summary: Pull Request resolved: #43333 This change fixes #43285. Basically, when using a `yarn` alias to install pods, yarn creates a copy of the `node` and `yarn` executables and the `command -v node` command will return the path to that executable. ## Changelog [iOS][Fixed] - Do not use temporary node when creating the .xcode.env.local Reviewed By: dmytrorykun Differential Revision: D54542774 fbshipit-source-id: 3ab0d0bb441988026feff9d5390dcfd10869a1b5
Summary: Pull Request resolved: #43333 This change fixes #43285. Basically, when using a `yarn` alias to install pods, yarn creates a copy of the `node` and `yarn` executables and the `command -v node` command will return the path to that executable. ## Changelog [iOS][Fixed] - Do not use temporary node when creating the .xcode.env.local Reviewed By: dmytrorykun Differential Revision: D54542774 fbshipit-source-id: 3ab0d0bb441988026feff9d5390dcfd10869a1b5
Any plan to backport the changes to v0.73? |
This is where backports to previous react-native releases can be requested: |
Summary: Pull Request resolved: #43333 This change fixes #43285. Basically, when using a `yarn` alias to install pods, yarn creates a copy of the `node` and `yarn` executables and the `command -v node` command will return the path to that executable. ## Changelog [iOS][Fixed] - Do not use temporary node when creating the .xcode.env.local Reviewed By: dmytrorykun Differential Revision: D54542774 fbshipit-source-id: 3ab0d0bb441988026feff9d5390dcfd10869a1b5
Summary:
This change fixes #43285.
Basically, when using a
yarn
alias to install pods, yarn creates a copy of thenode
andyarn
executables and thecommand -v node
command will return the path to that executable.Differential Revision: D54542774