From f3299acd0b4249500e940776aca77cc6c0977263 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Fri, 4 Oct 2019 10:23:50 -0400 Subject: [PATCH] Reimplemented using idealTree param PR-URL: https://github.com/npm/cli/pull/257 Credit: @ruyadorno Close: #257 Reviewed-by: @isaacs --- lib/install/actions.js | 2 +- lib/install/deps.js | 2 +- lib/install/validate-args.js | 21 +++++++---- lib/install/validate-tree.js | 12 +------ test/tap/check-engine-reqs.js | 6 ++-- test/tap/validate-tree-check-warnings.js | 44 ------------------------ 6 files changed, 20 insertions(+), 67 deletions(-) delete mode 100644 test/tap/validate-tree-check-warnings.js diff --git a/lib/install/actions.js b/lib/install/actions.js index a34d03ffe2146..e26432b77c86a 100644 --- a/lib/install/actions.js +++ b/lib/install/actions.js @@ -49,7 +49,7 @@ Object.keys(actions).forEach(function (actionName) { if (pkg.knownInstallable) { actionP = runAction(action, staging, pkg, log) } else { - actionP = isInstallable(pkg.package).then(() => { + actionP = isInstallable(null, pkg.package).then(() => { pkg.knownInstallable = true return runAction(action, staging, pkg, log) }) diff --git a/lib/install/deps.js b/lib/install/deps.js index bfc94ae504860..dfe30b6c0f38c 100644 --- a/lib/install/deps.js +++ b/lib/install/deps.js @@ -665,7 +665,7 @@ function resolveWithNewModule (pkg, tree, log, next) { validate('OOOF', arguments) log.silly('resolveWithNewModule', packageId(pkg), 'checking installable status') - return isInstallable(pkg, (err) => { + return isInstallable(tree, pkg, (err) => { let installable = !err addBundled(pkg, (bundleErr) => { var parent = earliestInstallable(tree, tree, pkg, log) || tree diff --git a/lib/install/validate-args.js b/lib/install/validate-args.js index 386cce3ca939c..b680a1b24ba47 100644 --- a/lib/install/validate-args.js +++ b/lib/install/validate-args.js @@ -16,7 +16,7 @@ module.exports = function (idealTree, args, next) { chain([ [hasMinimumFields, pkg], [checkSelf, idealTree, pkg, force], - [isInstallable, pkg] + [isInstallable, idealTree, pkg] ], done) }, next) } @@ -31,17 +31,24 @@ function hasMinimumFields (pkg, cb) { } } -function setWarnings (pkg, warn) { - if (!pkg.warnings) pkg.warnings = [] - if (pkg.warnings.every(i => ( +function setWarnings (idealTree, warn) { + function top (tree) { + if (tree.parent) return top(tree.parent) + return tree + } + + var topTree = top(idealTree) + if (!topTree.warnings) topTree.warnings = [] + + if (topTree.warnings.every(i => ( i.code !== warn.code || i.required !== warn.required || i.pkgid !== warn.pkgid))) { - pkg.warnings.push(warn) + topTree.warnings.push(warn) } } -var isInstallable = module.exports.isInstallable = function (pkg, next) { +var isInstallable = module.exports.isInstallable = function (idealTree, pkg, next) { var force = npm.config.get('force') var nodeVersion = npm.config.get('node-version') if (/-/.test(nodeVersion)) { @@ -52,7 +59,7 @@ var isInstallable = module.exports.isInstallable = function (pkg, next) { var strict = npm.config.get('engine-strict') checkEngine(pkg, npm.version, nodeVersion, force, strict, iferr(next, thenWarnEngineIssues)) function thenWarnEngineIssues (warn) { - if (warn) setWarnings(pkg, warn) + if (idealTree && warn) setWarnings(idealTree, warn) checkPlatform(pkg, force, next) } } diff --git a/lib/install/validate-tree.js b/lib/install/validate-tree.js index d1b40c24c1bcb..24a140171d45c 100644 --- a/lib/install/validate-tree.js +++ b/lib/install/validate-tree.js @@ -22,8 +22,7 @@ module.exports = function (idealTree, log, next) { [asyncMap, modules, function (mod, done) { chain([ mod.parent && !mod.isLink && [checkGit, mod.realpath], - [checkErrors, mod, idealTree], - [checkWarnings, mod, idealTree] + [checkErrors, mod, idealTree] ], done) }], [thenValidateAllPeerDeps, idealTree], @@ -37,15 +36,6 @@ function checkErrors (mod, idealTree, next) { next() } -function checkWarnings (mod, idealTree, next) { - const warnings = mod.package.warnings - if (warnings && (mod.parent || path.resolve(npm.globalDir, '..') !== mod.path)) { - warnings.forEach(warn => idealTree.warnings.push(warn)) - delete mod.package.warnings - } - next() -} - function thenValidateAllPeerDeps (idealTree, next) { validate('OF', arguments) validateAllPeerDeps(idealTree, function (tree, pkgname, version) { diff --git a/test/tap/check-engine-reqs.js b/test/tap/check-engine-reqs.js index 7ebb29484d828..fa25e28dd60ed 100644 --- a/test/tap/check-engine-reqs.js +++ b/test/tap/check-engine-reqs.js @@ -47,9 +47,9 @@ test('warns on bad engine not strict', function (t) { t.ifError(err, 'npm ran without issue') t.is(code, 0, 'result code') var result = JSON.parse(stdout) - t.match(result.warnings[1], /Unsupported engine/, 'reason for optional failure in JSON') - t.match(result.warnings[1], /1.0.0-not-a-real-version/, 'should print mismatch version info') - t.match(result.warnings[1], /Not compatible with your version of node/, 'incompatibility message') + t.match(result.warnings[0], /Unsupported engine/, 'reason for optional failure in JSON') + t.match(result.warnings[0], /1.0.0-not-a-real-version/, 'should print mismatch version info') + t.match(result.warnings[0], /Not compatible with your version of node/, 'incompatibility message') t.done() }) }) diff --git a/test/tap/validate-tree-check-warnings.js b/test/tap/validate-tree-check-warnings.js deleted file mode 100644 index 7b0bd45359d2c..0000000000000 --- a/test/tap/validate-tree-check-warnings.js +++ /dev/null @@ -1,44 +0,0 @@ -'use strict' -var test = require('tap').test -var log = require('npmlog') -var npm = require('../../lib/npm.js') -var checkEngine = require('npm-install-checks').checkEngine - -var idealTree = { - package: { - name: 'a b c', - version: '3.what' - }, - children: [{ - name: 'faulty-engine', - version: '0.0.1', - children: [], - engines: { - node: '>=2.0.0' - }, - package: { - name: 'faulty-engine', - version: '0.0.1' - } - }], - warnings: [] -} - -test('setup', function (t) { - const faultyEnginePkg = idealTree.children[0] - checkEngine(faultyEnginePkg, '1.0.0', '1.0.0', false, false, (err, warn) => { - t.ifError(err, 'check engine ran without issue') - faultyEnginePkg.package.warnings = [warn] - npm.load({}, t.end) - }) -}) - -test('validate-tree should collect warnings from modules', function (t) { - log.disableProgress() - var validateTree = require('../../lib/install/validate-tree.js') - validateTree(idealTree, log.newGroup('validate'), function (er, a, b) { - t.equal(idealTree.warnings[0].code, 'ENOTSUP', 'should have the correct error') - t.match(idealTree.warnings[0].message, /Unsupported engine/, 'reason for optional failure in JSON') - t.end() - }) -})