child_process.fork: allow execPath to be specified. #3248

Closed
wants to merge 1 commit into
from

Projects

None yet

6 participants

@lwille
lwille commented May 10, 2012

When running coffee-script files with dev tools like node-dev or testing tools like mocha, execPath is always "node" as these tools are plain js.

To be able to fork a coffee-script child process, I have to modify process.execPath to the path of "coffee":
[process.oldExecPath, process.execPath] = [process.execPath,'/usr/local/bin/coffee']
child = fork modulePath, [], options
process.execPath = process.oldExecPath if process.oldExecPath

This pull request provides child_process.fork with an optional execPath option.

@bnoordhuis
Member

Can you add documentation and one or two tests?

@TooTallNate

-1

Compile your "compile to js" languages to js, then execute that

@lwille
lwille commented May 10, 2012

@TooTallNate: I rather enjoy my setup the way it is. Didn't find any problem in using coffee as an interpreter, except for the forking of child processes. Also I really like this node way of doing IPC, so I would use it for other JSON-capable languages as well.

@bnoordhuis I'll add tests and docs when the addition gets more appreciation.

@isaacs
Collaborator
isaacs commented May 11, 2012

This has come up several times. The answer is no. child_process.fork is intended to be used to run a copy of node. It is quite easy to write a JavaScript program that loads coffee-script, and then uses that to load your coffee-script program, or to compile your program to JS before running it.

However, in v0.8, you will be able to specify an arbitrary stdio file descriptor as ipc and then do the messaging stuff with it, and even send handles across it, so the need to do this with the fork sugar method will be greatly reduced.

@isaacs isaacs closed this May 11, 2012
@bnoordhuis
Member

I can see this being useful in some cases. Say you have a shell script wrapper that sets up the environment in some way. It's not possible right now to make fork() invoke the wrapper.

For example, on some runs I want to log the stderr of each worker in a cluster to a separate log file. With this patch, I can do that in three lines of shell instead of thirty lines of javascript.

Not having this functionality is no great loss though, it's just convenient.

@isaacs
Collaborator
isaacs commented May 11, 2012

@bnoordhuis If you could direct spawn/fork'ed workers' stdio to arbitrary file descriptors or 'ipc', would that still be a pressing need?

@bnoordhuis
Member

It's not quite the same. The benefit of a wrapper script is that you can tweak node's behavior from the outside (ex. logging of stderr) with minimal changes to your code. Not having such functionality is no great hardship though, I'll manage. :-)

@isaacs
Collaborator
isaacs commented May 11, 2012

Hm. That's true, and a good point. I could see this also being in the "api consistency" category.

Let's revisit for 0.9. The door is closed for new 0.8 features at this point.

@lwille Please go ahead and add some docs and tests. The final shape of the API might change slightly, but at least we'll have a base to start with.

@isaacs isaacs reopened this May 11, 2012
@bmeck
Collaborator
bmeck commented Jun 15, 2012

workaround for now is to set process.execPath before invoking (can wrap and reset child_process.fork). We may end up doing this for our bootstrap script and the cluster module as well.

@mmalecki

Shim:

var spawn = require('child_process').spawn;
var child = spawn(process.argv[1], process.argv.slice(2), {
  customFds: [0, 1, 2]
});
child.on('exit', process.exit);

You can fork that shim and pass executable as the first parameter (since you can fork with parameters).

@isaacs
Collaborator
isaacs commented Dec 30, 2012

Anyone care to revive this? All it needs is tests and docs.

@bmeck
Collaborator
bmeck commented Dec 30, 2012

I'll find some time in next couple of days. @isaacs , testing via a shell piping to node is fine correct?

@isaacs
Collaborator
isaacs commented Dec 30, 2012

@bmeck The test should be entirely included in one file, if possible, or use a file in test/fixtures/ if absolutely necessary. It must also run on Windows, so no bash/sh scripts.

@isaacs
Collaborator
isaacs commented Dec 30, 2012

@bmeck If necessary, I guess you could invoke cmd on windows, and sh on Unix.

@bmeck
Collaborator
bmeck commented Jan 4, 2013

@isaacs generating a symlink at test time and testing argv[0] seems the way to go, is generating the link at test time fine?

@bnoordhuis
Member

is generating the link at test time fine?

Yes, but it should go into common.tmpDir, i.e. test/tmp, not test/fixtures.

@bnoordhuis bnoordhuis added a commit that closed this pull request Jan 6, 2013
@bmeck bmeck child_process: make fork() execPath configurable
Allows for arbitrary path to executable spawned using `fork`. This
fixes some issues around running multiple versions of node with workers
and allows arbitrary IPC with compatible executables.

Fixes #3248.
70ad9bb
@bnoordhuis bnoordhuis closed this in 70ad9bb Jan 6, 2013
@bnoordhuis
Member

Fixed in 70ad9bb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment