allow local .npmrc to override internal env variables set by the runn…

…ing npm process, fixes #81

package.json

@@ -49,7 +49,7 @@
     "minimal-request-promise": "^1.1.0",
     "minimist": "^1.2.0",
     "oh-no-i-insist": "^1.0.0",
-    "shelljs": "^0.5.3",
+    "shelljs": "^0.7.5",
     "tar-fs": "^1.14.0",
     "uuid": "^2.0.1"
   },

spec/clean-up-package-spec.js

@@ -0,0 +1,76 @@
+/*global describe, it, beforeEach, afterEach, require, it, expect */
+var underTest = require('../src/tasks/clean-up-package'),
+	shell = require('shelljs'),
+	fs = require('fs'),
+	ArrayLogger = require('../src/util/array-logger'),
+	tmppath = require('../src/util/tmppath'),
+	runNpm = require('../src/util/run-npm'),
+	path = require('path');
+describe('cleanUpPackage', function () {
+	'use strict';
+	var sourcedir, pwd,
+		logger,
+		configurePackage = function (packageConf) {
+			fs.writeFileSync(path.join(sourcedir, 'package.json'), JSON.stringify(packageConf), 'utf8');
+		};
+	beforeEach(function (done) {
+		sourcedir = tmppath();
+		shell.mkdir(sourcedir);
+		logger = new ArrayLogger();
+		pwd = shell.pwd();
+		configurePackage({
+			dependencies: {
+				'uuid': '^2.0.0'
+			},
+			optionalDependencies: {
+				'minimist': '^1.2.0'
+			}
+		});
+		runNpm(sourcedir, 'install', logger).then(done, done.fail);
+	});
+	afterEach(function () {
+		shell.cd(pwd);
+		if (sourcedir) {
+			shell.rm('-rf', sourcedir);
+		}
+	});
+	it('returns the directory path', function (done) {
+		underTest(sourcedir, {}, logger).then(function (result) {
+			expect(result).toEqual(sourcedir);
+		}).then(done, done.fail);
+	});
+	it('does not clean up optional dependencies if not requested', function (done) {
+		underTest(sourcedir, {}, logger).then(function (result) {
+			expect(result).toEqual(sourcedir);
+			expect(shell.test('-e', path.join(sourcedir, 'node_modules', 'uuid'))).toBeTruthy();
+			expect(shell.test('-e', path.join(sourcedir, 'node_modules', 'minimist'))).toBeTruthy();
+		}).then(done, done.fail);
+	});
+	it('cleans up optional dependencies if requested', function (done) {
+		underTest(sourcedir, {'optional-dependencies': false}, logger).then(function (result) {
+			expect(result).toEqual(sourcedir);
+			expect(shell.test('-e', path.join(sourcedir, 'node_modules', 'uuid'))).toBeTruthy();
+			expect(shell.test('-e', path.join(sourcedir, 'node_modules', 'minimist'))).toBeFalsy();
+		}).then(done, done.fail);
+	});
+	it('removes .npmrc if exists', function (done) {
+		fs.writeFileSync(path.join(sourcedir, '.npmrc'), 'optional = false', 'utf8');
+		underTest(sourcedir, {}, logger).then(function (result) {
+			expect(result).toEqual(sourcedir);
+			expect(shell.test('-e', path.join(sourcedir, '.npmrc'))).toBeFalsy();
+		}).then(done, done.fail);
+
+	});
+	it('fails if npm install fails', function (done) {
+		configurePackage({
+			files: ['root.txt'],
+			dependencies: {
+				'non-existing-package': '2.0.0'
+			}
+		});
+		underTest(sourcedir, {'optional-dependencies': false}, logger).then(done.fail, function (reason) {
+			expect(reason).toMatch(/npm install --production --no-optional failed/);
+			done();
+		});
+	});
+});

spec/collect-files-spec.js

@@ -213,6 +213,25 @@ describe('collectFiles', function () {
 				done();
 			}, done.fail);
 		});
+		it('uses the local .npmrc file if it exists', function (done) {
+			configurePackage({
+				files: ['root.txt'],
+				dependencies: {
+					'uuid': '^2.0.0'
+				},
+				optionalDependencies: {
+					'minimist': '^1.2.0'
+				}
+			});
+			fs.writeFileSync(path.join(sourcedir, '.npmrc'), 'optional = false', 'utf8');
+			underTest(sourcedir).then(function (packagePath) {
+				destdir = packagePath;
+				expect(shell.test('-e', path.join(packagePath, 'node_modules', 'uuid'))).toBeTruthy();
+				expect(shell.test('-e', path.join(packagePath, 'node_modules', 'minimist'))).toBeFalsy();
+				expect(shell.test('-e', path.join(packagePath, 'node_modules', 'old-mod'))).toBeFalsy();
+				done();
+			}, done.fail);
+		});
 		it('uses local node_modules instead of running npm install if localDependencies is set to true', function (done) {
 			configurePackage({
 				dependencies: {

spec/create-spec.js

@@ -19,7 +19,10 @@ describe('create', function () {
 			if (!shell.test('-e', workingdir)) {
 				shell.mkdir('-p', workingdir);
 			}
-			shell.cp('-r', path.join(__dirname, 'test-projects/', (dir || 'hello-world')) + '/*', workingdir);
+			shell.cp('-r',
+				path.join(__dirname, 'test-projects/', (dir || 'hello-world')) + '/*',
+				path.join(__dirname, 'test-projects/', (dir || 'hello-world')) + '/.*',
+				workingdir);
 			return underTest(config, logger).then(function (result) {
 				newObjects.lambdaRole = result.lambda && result.lambda.role;
 				newObjects.lambdaFunction = result.lambda && result.lambda.name;
@@ -534,6 +537,14 @@ describe('create', function () {
 				expect(lambdaResult.Payload).toEqual('{"endpoint":"https://s3.amazonaws.com/","modules":[".bin","huh"]}');
 			}).then(done, done.fail);
 		});
+		it('removes .npmrc from the package', function (done) {
+			createFromDir('ls-dir').then(function () {
+				return lambda.invokePromise({FunctionName: testRunName});
+			}).then(function (lambdaResult) {
+				expect(lambdaResult.StatusCode).toEqual(200);
+				expect(lambdaResult.Payload).toEqual('{"files":["main.js","node_modules","package.json"]}');
+			}).then(done, done.fail);
+		});
 		it('keeps the archive on the disk if --keep is specified', function (done) {
 			config.keep = true;
 			createFromDir('hello-world').then(function (result) {

spec/localize-dependencies-spec.js

@@ -13,11 +13,12 @@ describe('localizeDependencies', function () {
 	var workdir, referencedir;
 	beforeEach(function () {
 		workdir = path.join(os.tmpdir(), uuid.v4());
-		referencedir = '/abc/def';
+		referencedir = path.join(os.tmpdir(), uuid.v4());
 		shell.mkdir(workdir);
+		shell.mkdir(referencedir);
 	});
 	afterEach(function () {
-		shell.rm('-rf', workdir);
+		shell.rm('-rf', workdir, referencedir);
 	});
 	it('does not modify package properties that have nothing to do with dependencies', function (done) {
 		var referenceJSON;
@@ -112,5 +113,20 @@ describe('localizeDependencies', function () {
 			}).then(done, done.fail);
 		});
 	});
+	it('does not create .npmrc if the original directory does not have one', function (done) {
+		shell.cp(path.join(__dirname, '..', 'package.json'), workdir);
+		localizeDependencies(workdir, referencedir).then(function () {
+			expect(shell.test('-e', path.join(workdir, '.npmrc'))).toBeFalsy();
+		}).then(done, done.fail);
+	});
+	it('copies .npmrc if the original directory contains it', function (done) {
+		fs.writeFileSync(path.join(referencedir, '.npmrc'), 'optional = false', 'utf8');
+		shell.cp(path.join(__dirname, '..', 'package.json'), workdir);
+		localizeDependencies(workdir, referencedir).then(function () {
+			var npmRcPath = path.join(workdir, '.npmrc');
+			expect(shell.test('-e', npmRcPath)).toBeTruthy();
+			expect(fs.readFileSync(npmRcPath, 'utf8')).toEqual('optional = false');
+		}).then(done, done.fail);
+	});
 
 });

spec/run-npm-spec.js

@@ -41,6 +41,25 @@ describe('runNpm', function () {
 			done();
 		}, done.fail);
 	});
+	it('uses local .npmrc if exists', function (done) {
+		configurePackage({
+			dependencies: {
+				'uuid': '^2.0.0'
+			},
+			optionalDependencies: {
+				'minimist': '^1.2.0'
+			}
+		});
+		fs.writeFileSync(path.join(sourcedir, '.npmrc'), 'optional = false', 'utf8');
+		underTest(sourcedir, 'install --production', logger).then(function (packagePath) {
+			expect(packagePath).toEqual(sourcedir);
+			expect(shell.pwd()).toEqual(pwd);
+			expect(shell.test('-e', path.join(sourcedir, 'node_modules', 'uuid'))).toBeTruthy();
+			expect(shell.test('-e', path.join(sourcedir, 'node_modules', 'minimist'))).toBeFalsy();
+			done();
+		}, done.fail);
+
+	});
 	it('fails if npm install fails', function (done) {
 		configurePackage({
 			files: ['root.txt'],

spec/test-projects/ls-dir/.npmrc

@@ -0,0 +1 @@
+optional = false

spec/test-projects/ls-dir/main.js

@@ -0,0 +1,8 @@
+/*global exports, require*/
+var fs = require('fs');
+exports.handler = function (event, context) {
+	'use strict';
+	context.succeed({
+		files: fs.readdirSync('.')
+	});
+};

spec/test-projects/ls-dir/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "echo",
+  "version": "1.0.0",
+  "private": true,
+  "files": [
+    "main.js"
+  ]
+}

src/commands/create.js

@@ -5,6 +5,7 @@ var Promise = require('bluebird'),
 	aws = require('aws-sdk'),
 	zipdir = require('../tasks/zipdir'),
 	collectFiles = require('../tasks/collect-files'),
+	cleanUpPackage = require('../tasks/clean-up-package'),
 	addPolicy = require('../tasks/add-policy'),
 	markAlias = require('../tasks/mark-alias'),
 	templateFile = require('../util/template-file'),
@@ -13,7 +14,6 @@ var Promise = require('bluebird'),
 	rebuildWebApi = require('../tasks/rebuild-web-api'),
 	readjson = require('../util/readjson'),
 	apiGWUrl = require('../util/apigw-url'),
-	cleanOptionalDependencies = require('../tasks/clean-optional-dependencies'),
 	promiseWrap = require('../util/promise-wrap'),
 	retry = require('oh-no-i-insist'),
 	fs = Promise.promisifyAll(require('fs')),
@@ -333,11 +333,7 @@ module.exports = function create(options, optionalLogger) {
 		return validatePackage(dir, options.handler, options['api-module']);
 	}).then(function (dir) {
 		packageFileDir = dir;
-		if (options['optional-dependencies'] === false) {
-			return cleanOptionalDependencies(dir, logger);
-		} else {
-			return dir;
-		}
+		return cleanUpPackage(dir, options, logger);
 	}).then(function (dir) {
 		logger.logStage('zipping package');
 		return zipdir(dir);

src/commands/update.js

@@ -4,7 +4,7 @@ var Promise = require('bluebird'),
 	collectFiles = require('../tasks/collect-files'),
 	os = require('os'),
 	path = require('path'),
-	cleanOptionalDependencies = require('../tasks/clean-optional-dependencies'),
+	cleanUpPackage = require('../tasks/clean-up-package'),
 	aws = require('aws-sdk'),
 	allowApiInvocation = require('../tasks/allow-api-invocation'),
 	lambdaCode = require('../tasks/lambda-code'),
@@ -141,9 +141,7 @@ module.exports = function update(options, optionalLogger) {
 		return validatePackage(dir, functionConfig.Handler, apiConfig && apiConfig.module);
 	}).then(function (dir) {
 		packageDir = dir;
-		if (options['optional-dependencies'] === false) {
-			return cleanOptionalDependencies(dir, logger);
-		}
+		return cleanUpPackage(dir, options, logger);
 	}).then(function () {
 		if (requiresHandlerUpdate) {
 			return lambda.updateFunctionConfigurationPromise({FunctionName: lambdaConfig.name, Handler: functionConfig.Handler});

src/tasks/clean-up-package.js

@@ -0,0 +1,20 @@
+/*global module, require, Promise */
+var shell = require('shelljs'),
+	path = require('path'),
+	runNpm = require('../util/run-npm');
+module.exports = function cleanUpPackage(packageDir, options, logger) {
+	'use strict';
+	var cleanUpDependencies = function () {
+		if (options['optional-dependencies'] === false) {
+			logger.logApiCall('removing optional dependencies');
+			shell.rm('-rf', path.join(packageDir, 'node_modules'));
+			return runNpm(packageDir, 'install --production --no-optional', logger);
+		} else {
+			return Promise.resolve();
+		}
+	};
+	return cleanUpDependencies().then(function () {
+		shell.rm('-rf', path.join(packageDir, '.npmrc'));
+		return packageDir;
+	});
+};

src/tasks/localize-dependencies.js

@@ -3,6 +3,7 @@ var Promise = require('bluebird'),
 	fs = require('fs'),
 	path = require('path'),
 	Promise = require('bluebird'),
+	shell = require('shelljs'),
 	writeFile = Promise.promisify(fs.writeFile),
 	readjson = require('../util/readjson');
 
@@ -30,5 +31,10 @@ module.exports = function (workdir, referencedir) {
 		return content;
 	}).then(function (content) {
 		return writeFile(packagePath, JSON.stringify(content), {encoding: 'utf8'});
+	}).then(function () {
+		var npmRcPath = path.join(referencedir, '.npmrc');
+		if (shell.test('-e', npmRcPath)) {
+			shell.cp(npmRcPath, workdir);
+		}
 	});
 };

src/util/run-npm.js

@@ -5,10 +5,14 @@ module.exports = function runNpm(dir, options, logger) {
 	'use strict';
 	var cwd = shell.pwd(),
 		npmlog = tmppath(),
-		command = 'npm ' + options;
+		command = shell.which('npm') + ' ' + options,
+		env = shell.env;
 	logger.logApiCall(command);
 	shell.cd(dir);
-	if (shell.exec(command + ' > ' + npmlog + ' 2>&1').code !== 0) {
+	if (shell.test('-e', '.npmrc')) {
+		env = {};
+	}
+	if (shell.exec(command + ' > ' + npmlog + ' 2>&1', {env: env}).code !== 0) {
 		shell.cd(cwd);
 		return Promise.reject(command + ' failed. Check ' + npmlog);
 	}