New api for child_process.spawn; ability to set cwd for spawn()ed process

Tests for child_process.spawn() use new API

Test for deprecated child_process.spawn() API

doc/api.markdown

@@ -938,11 +938,22 @@ Example:
     grep.stdin.end();
 
 
-### child_process.spawn(command, args=[], env=process.env)
+### child_process.spawn(command, args=[], [options])
 
-Launches a new process with the given `command`, command line arguments, and
+Launches a new process with the given `command`, with  command line arguments in `args`.
-environment variables.  If omitted, `args` defaults to an empty Array, and `env`
+If omitted, `args` defaults to an empty Array.
-defaults to `process.env`.
+
+The third argument is used to specify additional options, which defaults to:
+
+    { cwd: undefined
+    , env: process.env,
+    , customFds: [-1, -1, -1]
+    }
+
+`cwd` allows you to specify the working directory from which the process is spawned.
+Use `env` to specify environment variables that will be visible to the new process.
+With `customFds` it is possible to hook up the new process' [stdin, stout, stderr] to
+existing streams; `-1` means that a new stream should be created.
 
 Example of running `ls -lh /usr`, capturing `stdout`, `stderr`, and the exit code:
 

lib/child_process.js

@@ -4,9 +4,9 @@ var Stream = require('net').Stream;
 var InternalChildProcess = process.binding('child_process').ChildProcess;
 
 
-var spawn = exports.spawn = function (path, args, env, customFds) {
+var spawn = exports.spawn = function (path, args /*, options OR env, customFds */) {
   var child = new ChildProcess();
-  child.spawn(path, args, env, customFds);
+  child.spawn.apply(child, arguments);
   return child;
 };
 
@@ -55,7 +55,7 @@ exports.execFile = function (file /* args, options, callback */) {
     }
   }
 
-  var child = spawn(file, args, options.env);
+  var child = spawn(file, args, {env: options.env});
   var stdout = "";
   var stderr = "";
   var killed = false;
@@ -161,9 +161,24 @@ ChildProcess.prototype.kill = function (sig) {
 };
 
 
-ChildProcess.prototype.spawn = function (path, args, env, customFds) {
+ChildProcess.prototype.spawn = function (path, args, options, customFds) {
   args = args || [];
-  env = env || process.env;
+  options = options || {};
+
+  var cwd, env;
+  if (options.cwd === undefined && options.env === undefined && options.customFds === undefined) {
+    // Deprecated API: (path, args, options, env, customFds)
+    cwd = "";
+    env = options || process.env;
+    customFds = customFds || [-1, -1, -1];
+  }
+  else {
+    // Recommended API: (path, args, options)
+    cwd = options.cwd || "";
+    env = options.env || process.env;
+    customFds = options.customFds || [-1, -1, -1];
+  }
+
   var envPairs = [];
   var keys = Object.keys(env);
   for (var index = 0, keysLength = keys.length; index < keysLength; index++) {
@@ -171,8 +186,7 @@ ChildProcess.prototype.spawn = function (path, args, env, customFds) {
     envPairs.push(key + "=" + env[key]);
   }
 
-  customFds = customFds || [-1, -1, -1];
+  var fds = this.fds = this._internal.spawn(path, args, cwd, envPairs, customFds);
-  var fds = this.fds = this._internal.spawn(path, args, envPairs, customFds);
 
   if (customFds[0] === -1 || customFds[0] === undefined) {
     this.stdin.open(fds[0]);

src/node_child_process.cc

@@ -75,7 +75,8 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
   if (args.Length() < 3 ||
       !args[0]->IsString() ||
       !args[1]->IsArray() ||
-      !args[2]->IsArray()) {
+      !args[2]->IsString() ||
+      !args[3]->IsArray()) {
     return ThrowException(Exception::Error(String::New("Bad argument.")));
   }
 
@@ -99,8 +100,12 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
     argv[i+1] = strdup(*arg);
   }
 
-  // Copy third argument, args[2], into a c-string array called env.
+  // Copy third argument, args[2], into a c-string called cwd.
-  Local<Array> env_handle = Local<Array>::Cast(args[2]);
+  String::Utf8Value arg(args[2]->ToString());
+  char *cwd = strdup(*arg);
+
+  // Copy fourth argument, args[3], into a c-string array called env.
+  Local<Array> env_handle = Local<Array>::Cast(args[3]);
   int envc = env_handle->Length();
   char **env = new char*[envc+1]; // heap allocated to detect errors
   env[envc] = NULL;
@@ -110,9 +115,9 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
   }
 
   int custom_fds[3] = { -1, -1, -1 };
-  if (args[3]->IsArray()) {
+  if (args[4]->IsArray()) {
     // Set the custom file descriptor values (if any) for the child process
-    Local<Array> custom_fds_handle = Local<Array>::Cast(args[3]);
+    Local<Array> custom_fds_handle = Local<Array>::Cast(args[4]);
     int custom_fds_len = custom_fds_handle->Length();
     for (int i = 0; i < custom_fds_len; i++) {
       if (custom_fds_handle->Get(i)->IsUndefined()) continue;
@@ -123,7 +128,7 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
 
   int fds[3];
 
-  int r = child->Spawn(argv[0], argv, env, fds, custom_fds);
+  int r = child->Spawn(argv[0], argv, cwd, env, fds, custom_fds);
 
   for (i = 0; i < argv_length; i++) free(argv[i]);
   delete [] argv;
@@ -200,6 +205,7 @@ void ChildProcess::Stop() {
 //
 int ChildProcess::Spawn(const char *file,
                         char *const args[],
+                        const char *cwd,
                         char **env,
                         int stdio_fds[3],
                         int custom_fds[3]) {
@@ -251,6 +257,11 @@ int ChildProcess::Spawn(const char *file,
         dup2(custom_fds[2], STDERR_FILENO);
       }
 
+      if (strlen(cwd) && chdir(cwd)) {
+        perror("chdir()");
+        _exit(127);
+      }
+
       environ = env;
 
       execvp(file, args);

src/node_child_process.h

@@ -42,7 +42,7 @@ class ChildProcess : ObjectWrap {
   // are readable.
   // The user of this class has responsibility to close these pipes after
   // the child process exits.
-  int Spawn(const char *file, char *const argv[], char **env, int stdio_fds[3], int custom_fds[3]);
+  int Spawn(const char *file, char *const argv[], const char *cwd, char **env, int stdio_fds[3], int custom_fds[3]);
 
   // Simple syscall wrapper. Does not disable the watcher. onexit will be
   // called still.

test/simple/test-child-process-custom-fds.js

@@ -21,7 +21,7 @@ function test1(next) {
   console.log("Test 1...");
   fs.open(helloPath, 'w', 400, function (err, fd) {
     if (err) throw err;
-    var child = spawn('/bin/echo', [expected], undefined, [-1, fd] );
+    var child = spawn('/bin/echo', [expected], {customFds: [-1, fd]});
 
     assert.notEqual(child.stdin, null);
     assert.equal(child.stdout, null);
@@ -50,7 +50,7 @@ function test2(next) {
   fs.open(helloPath, 'r', undefined, function (err, fd) {
     var child = spawn(process.argv[0]
                      , [fixtPath('stdio-filter.js'), 'o', 'a']
-                     , undefined, [fd, -1, -1]);
+                     , {customFds: [fd, -1, -1]});
 
     assert.equal(child.stdin, null);
     var actualData = '';
@@ -74,7 +74,7 @@ function test3(next) {
   console.log("Test 3...");
   var filter = spawn(process.argv[0]
                    , [fixtPath('stdio-filter.js'), 'o', 'a']);
-  var echo = spawn('/bin/echo', [expected], undefined, [-1, filter.fds[0]]);
+  var echo = spawn('/bin/echo', [expected], {customFds: [-1, filter.fds[0]]});
   var actualData = '';
   filter.stdout.addListener('data', function(data) {
     console.log("  Got data --> " + data);

test/simple/test-child-process-cwd.js

@@ -0,0 +1,51 @@
+common = require("../common");
+assert = common.assert
+spawn = require('child_process').spawn,
+path = require('path');
+
+var returns = 0;
+
+/*
+  Spawns 'pwd' with given options, then test 
+  - whether the exit code equals forCode,
+  - optionally whether the stdout result (after removing traling whitespace) matches forData
+*/
+function testCwd(options, forCode, forData) {
+  var data = "";
+  
+  var child = spawn('pwd', [], options);
+  child.stdout.setEncoding('utf8');
+
+  child.stdout.addListener('data', function(chunk) {
+    data += chunk;
+  });
+  
+  child.addListener('exit', function(code, signal) {
+    forData && assert.strictEqual(forData, data.replace(/[\s\r\n]+$/, ''))
+    assert.strictEqual(forCode, code);
+    returns--;
+  });
+  
+  returns++;
+}
+
+// Assume these exist, and 'pwd' gives us the right directory back
+testCwd( { cwd: '/bin'    }, 0, '/bin' );
+testCwd( { cwd: '/dev'    }, 0, '/dev' );
+testCwd( { cwd: '/'       }, 0, '/'    );
+
+// Assume this doesn't exist, we expect exitcode=127
+testCwd( { cwd: 'does-not-exist' }, 127 );
+
+// Spawn() shouldn't try to chdir() so this should just work
+testCwd( undefined,          0 );
+testCwd( {                }, 0 );
+testCwd( { cwd: ''        }, 0 );
+testCwd( { cwd: undefined }, 0 );
+testCwd( { cwd: null      }, 0 );
+
+// Check whether all tests actually returned
+assert.notEqual(0, returns);
+process.addListener('exit', function () {
+  assert.equal(0, returns);
+});

test/simple/test-child-process-deprecated-api.js

@@ -0,0 +1,59 @@
+var common = require("../common");
+var assert = common.assert;
+var spawn  = require('child_process').spawn;
+var path   = require('path');
+var fs     = require('fs');
+var exits  = 0;
+
+// Test `env` parameter for child_process.spawn(path, args, env, customFds) deprecated api
+(function() {
+  var response = "";
+  var child = spawn('/usr/bin/env', [], {'HELLO' : 'WORLD'});
+
+  child.stdout.setEncoding('utf8');
+
+  child.stdout.addListener("data", function (chunk) {
+    response += chunk;
+  });
+
+  process.addListener('exit', function () {
+   assert.ok(response.indexOf('HELLO=WORLD') >= 0);
+   exits++;
+  });
+})();
+
+// Test `customFds` parameter for child_process.spawn(path, args, env, customFds) deprecated api
+(function() {
+  var expected = "hello world";
+  var helloPath = path.join(common.fixturesDir, "hello.txt");
+
+  fs.open(helloPath, 'w', 400, function (err, fd) {
+    if (err) throw err;
+
+    var child = spawn('/bin/echo', [expected], undefined, [-1, fd]);
+
+    assert.notEqual(child.stdin, null);
+    assert.equal(child.stdout, null);
+    assert.notEqual(child.stderr, null);
+
+    child.addListener('exit', function (err) {
+      if (err) throw err;
+
+      fs.close(fd, function (error) {
+        if (error) throw error;
+
+        fs.readFile(helloPath, function (err, data) {
+          if (err) throw err;
+
+          assert.equal(data.toString(), expected + "\n");
+          exits++;
+        });
+      });
+    });
+  });
+})();
+
+// Check if all child processes exited
+process.addListener('exit', function () {
+  assert.equal(2, exits);
+});

test/simple/test-child-process-env.js

@@ -2,7 +2,7 @@ common = require("../common");
 assert = common.assert
 
 var spawn = require('child_process').spawn;
-child = spawn('/usr/bin/env', [], {'HELLO' : 'WORLD'});
+child = spawn('/usr/bin/env', [], {env: {'HELLO' : 'WORLD'}});
 
 response = "";