workerLib's spawn should preserve the environment #677
Conversation
| @@ -231,10 +231,16 @@ export class Parent extends RequesterResponder { | |||
| /** start worker */ | |||
| startWorker(childJsPath: string, terminalError: (e: Error) => any, customArguments: string[]) { | |||
| try { | |||
| var spawnEnv = process.env; | |||
| spawnEnv['ATOM_SHELL_INTERNAL_RUN_AS_NODE'] = '1'; | |||
There was a problem hiding this comment.
Issues:
a.) You are mutating process.env here ... that is bad
b.) Using process.env as a whole breaks using atom as node on windows. See : atom/atom#2887 (comment) and independently verified : atom/atom#2887 (comment)
Solutions :
- Use
Object.createaround process.env - only do if platform is
NixOS
Thanks for finding the bug 🌹
There was a problem hiding this comment.
Oh yes, oops! I will fix that.
Do you have an existing way of checking the operating system?
There was a problem hiding this comment.
Do you have an existing way of checking the operating system?
process.platform https://nodejs.org/api/process.html#process_process_platform although I don't know the value for NixOS .... I'd be okay with whatever that is as long as its not mac / windows :)
There was a problem hiding this comment.
It has value 'linux' under NixOS.
I can set it that way, though I am not able to test on many such operating systems.
Its because atom packages are git tags ... and if the |
workerLib's spawn should preserve the environment
|
Released as a minor update : https://github.com/TypeStrong/atom-typescript/releases/tag/v7.5.0 🌹 |
Hello,
Working on NixOS, it is extremely bad that atom-typescript overwrites the environment of the process when calling spawn.
In particular, the child process fails to spawn as it cannot find libgtk-x11.
This patch attempts to solve this issue by only overriding the existing environment.
What is the policy with regards to pull requests? I notice the dist/ repository is tracked, which is weird, but I guess it's useful for bootstrapping?
Best regards,