From f748501d545eeaef45d35dd9c3ea1e527ce87281 Mon Sep 17 00:00:00 2001 From: Gojko Adzic Date: Tue, 8 Nov 2016 22:25:31 +0100 Subject: [PATCH] allow local .npmrc to override internal env variables set by the running npm process, fixes https://github.com/claudiajs/claudia/issues/81 --- package.json | 2 +- spec/clean-up-package-spec.js | 76 ++++++++++++++++++++++++ spec/collect-files-spec.js | 19 ++++++ spec/create-spec.js | 13 +++- spec/localize-dependencies-spec.js | 20 ++++++- spec/run-npm-spec.js | 19 ++++++ spec/test-projects/ls-dir/.npmrc | 1 + spec/test-projects/ls-dir/main.js | 8 +++ spec/test-projects/ls-dir/package.json | 8 +++ src/commands/create.js | 8 +-- src/commands/update.js | 6 +- src/tasks/clean-optional-dependencies.js | 10 ---- src/tasks/clean-up-package.js | 20 +++++++ src/tasks/localize-dependencies.js | 6 ++ src/util/run-npm.js | 8 ++- 15 files changed, 198 insertions(+), 26 deletions(-) create mode 100644 spec/clean-up-package-spec.js create mode 100644 spec/test-projects/ls-dir/.npmrc create mode 100644 spec/test-projects/ls-dir/main.js create mode 100644 spec/test-projects/ls-dir/package.json delete mode 100644 src/tasks/clean-optional-dependencies.js create mode 100644 src/tasks/clean-up-package.js diff --git a/package.json b/package.json index e8532dc6..ba3a3a63 100644 --- a/package.json +++ b/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" }, diff --git a/spec/clean-up-package-spec.js b/spec/clean-up-package-spec.js new file mode 100644 index 00000000..4a6b61d8 --- /dev/null +++ b/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(); + }); + }); +}); diff --git a/spec/collect-files-spec.js b/spec/collect-files-spec.js index 61d3f79f..1f71a050 100644 --- a/spec/collect-files-spec.js +++ b/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: { diff --git a/spec/create-spec.js b/spec/create-spec.js index 0f4130e0..d02dc4b2 100644 --- a/spec/create-spec.js +++ b/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) { diff --git a/spec/localize-dependencies-spec.js b/spec/localize-dependencies-spec.js index 8a2e6642..fc2c22d3 100644 --- a/spec/localize-dependencies-spec.js +++ b/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); + }); }); diff --git a/spec/run-npm-spec.js b/spec/run-npm-spec.js index 1ff87bbc..90ebdffb 100644 --- a/spec/run-npm-spec.js +++ b/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'], diff --git a/spec/test-projects/ls-dir/.npmrc b/spec/test-projects/ls-dir/.npmrc new file mode 100644 index 00000000..6ad0abd2 --- /dev/null +++ b/spec/test-projects/ls-dir/.npmrc @@ -0,0 +1 @@ +optional = false diff --git a/spec/test-projects/ls-dir/main.js b/spec/test-projects/ls-dir/main.js new file mode 100644 index 00000000..59d0602b --- /dev/null +++ b/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('.') + }); +}; diff --git a/spec/test-projects/ls-dir/package.json b/spec/test-projects/ls-dir/package.json new file mode 100644 index 00000000..81cc2df6 --- /dev/null +++ b/spec/test-projects/ls-dir/package.json @@ -0,0 +1,8 @@ +{ + "name": "echo", + "version": "1.0.0", + "private": true, + "files": [ + "main.js" + ] +} diff --git a/src/commands/create.js b/src/commands/create.js index a5bc0e58..87f2e9bb 100644 --- a/src/commands/create.js +++ b/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); diff --git a/src/commands/update.js b/src/commands/update.js index 9a544f69..4f5d0e15 100644 --- a/src/commands/update.js +++ b/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}); diff --git a/src/tasks/clean-optional-dependencies.js b/src/tasks/clean-optional-dependencies.js deleted file mode 100644 index 1a4377f6..00000000 --- a/src/tasks/clean-optional-dependencies.js +++ /dev/null @@ -1,10 +0,0 @@ -/*global module, require */ -var shell = require('shelljs'), - path = require('path'), - runNpm = require('../util/run-npm'); -module.exports = function cleanOptionalDependencies(packageDir, logger) { - 'use strict'; - logger.logApiCall('removing optional dependencies'); - shell.rm('-rf', path.join(packageDir, 'node_modules')); - return runNpm(packageDir, 'install --production --no-optional', logger); -}; diff --git a/src/tasks/clean-up-package.js b/src/tasks/clean-up-package.js new file mode 100644 index 00000000..ab80af4d --- /dev/null +++ b/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; + }); +}; diff --git a/src/tasks/localize-dependencies.js b/src/tasks/localize-dependencies.js index 6a49045a..6bb73d57 100644 --- a/src/tasks/localize-dependencies.js +++ b/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); + } }); }; diff --git a/src/util/run-npm.js b/src/util/run-npm.js index 29036d50..ae124c90 100644 --- a/src/util/run-npm.js +++ b/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); }