diff --git a/lib/main.js b/lib/main.js index 91251ba7..fe6551ec 100644 --- a/lib/main.js +++ b/lib/main.js @@ -3,6 +3,7 @@ var path = require('path') var aws = require('aws-sdk') var exec = require('child_process').exec +var execFile = require('child_process').execFile var fs = require('fs-extra') var packageJson = require(path.join(__dirname, '..', 'package.json')) var minimatch = require('minimatch') @@ -285,18 +286,55 @@ Lambda.prototype._rsync = function (program, src, dest, excludeNodeModules, call }) } -Lambda.prototype._npmInstall = function (program, codeDirectory, callback) { - const installOptions = [ - `--prefix ${codeDirectory}`, - process.platform === 'win32' ? `--cwd ${codeDirectory}` : null - ].join(' ') - var command = program.dockerImage - ? 'docker run --rm -v ' + codeDirectory + ':/var/task ' + program.dockerImage + ' npm -s install --production' - : `npm -s install --production ${installOptions}` - exec(command, { +Lambda.prototype._npmInstall = (program, codeDirectory, callback) => { + const dockerBaseOptions = [ + 'run', '--rm', '-v', `${codeDirectory}:/var/task`, + program.dockerImage, + 'npm', '-s', 'install', '--production' + ] + const npmInstallBaseOptions = [ + '-s', + 'install', + '--production', + '--prefix', codeDirectory + ] + + const params = (() => { + // reference: https://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows + + // with docker + if (program.dockerImage) { + if (process.platform === 'win32') { + return { + command: 'cmd.exe', + options: ['/c', 'docker'].concat(dockerBaseOptions) + } + } + return { + command: 'docker', + options: dockerBaseOptions + } + } + + // simple npm install + if (process.platform === 'win32') { + return { + command: 'cmd.exe', + options: ['/c', 'npm'] + .concat(npmInstallBaseOptions) + .concat(['--cwd', codeDirectory]) + } + } + return { + command: 'npm', + options: npmInstallBaseOptions + } + })() + + execFile(params.command, params.options, { maxBuffer: maxBufferSize, env: process.env - }, function (err) { + }, (err) => { if (err) { return callback(err) } diff --git a/test/main.js b/test/main.js index 83347fbe..c5ea073d 100644 --- a/test/main.js +++ b/test/main.js @@ -321,14 +321,14 @@ describe('lib/main', function () { describe('_rsync', function () { rsyncTests('_rsync') }) } - describe('_npmInstall', function () { - beforeEach(function (done) { - lambda._cleanDirectory(codeDirectory, function (err) { + describe('_npmInstall', () => { + beforeEach((done) => { + lambda._cleanDirectory(codeDirectory, (err) => { if (err) { return done(err) } - lambda._fileCopy(program, '.', codeDirectory, true, function (err) { + lambda._fileCopy(program, '.', codeDirectory, true, (err) => { if (err) { return done(err) } @@ -340,9 +340,47 @@ describe('lib/main', function () { it('_npm adds node_modules', function (done) { _timeout({ this: this, sec: 30 }) // give it time to build the node modules - lambda._npmInstall(program, codeDirectory, function (err, result) { + lambda._npmInstall(program, codeDirectory, (err, result) => { assert.isNull(err) - var contents = fs.readdirSync(codeDirectory) + const contents = fs.readdirSync(codeDirectory) + assert.include(contents, 'node_modules') + done() + }) + }) + }) + + describe('_npmInstall (When codeDirectory contains characters to be escaped)', () => { + beforeEach((done) => { + // Since '\' can not be included in the file or directory name in Windows + const directoryName = process.platform === 'win32' + ? 'hoge fuga\' piyo' + : 'hoge "fuga\' \\piyo' + codeDirectory = path.join(os.tmpdir(), directoryName) + lambda._cleanDirectory(codeDirectory, (err) => { + if (err) { + return done(err) + } + + lambda._fileCopy(program, '.', codeDirectory, true, (err) => { + if (err) { + return done(err) + } + done() + }) + }) + }) + + afterEach(() => { + fs.removeSync(codeDirectory) + codeDirectory = lambda._codeDirectory() + }) + + it('_npm adds node_modules', function (done) { + _timeout({ this: this, sec: 30 }) // give it time to build the node modules + + lambda._npmInstall(program, codeDirectory, (err, result) => { + assert.isNull(err) + const contents = fs.readdirSync(codeDirectory) assert.include(contents, 'node_modules') done() })