From cc0c6e127876c277d201b5f400e2adfbca312987 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 9 Nov 2020 23:36:51 -0800 Subject: [PATCH] Support update options in global package space This adjusts the portion of the global actualTree that gets loaded when doing an update, and sets up the virtual global package manifest to depend on '*' for all the packages being updated at the top global level. Fixes: https://github.com/npm/cli/issues/1962 --- lib/arborist/build-ideal-tree.js | 27 ++- lib/arborist/reify.js | 2 +- ...t-arborist-build-ideal-tree.js-TAP.test.js | 179 ++++++++++++++++++ test/arborist/build-ideal-tree.js | 67 ++++++- 4 files changed, 265 insertions(+), 10 deletions(-) diff --git a/lib/arborist/build-ideal-tree.js b/lib/arborist/build-ideal-tree.js index 6fbf2e55c..5fa8023bd 100644 --- a/lib/arborist/build-ideal-tree.js +++ b/lib/arborist/build-ideal-tree.js @@ -8,7 +8,9 @@ const promiseCallLimit = require('promise-call-limit') const getPeerSet = require('../peer-set.js') const realpath = require('../../lib/realpath.js') const walkUpPath = require('walk-up-path') -const { dirname } = require('path') +const { dirname, resolve } = require('path') +const { promisify } = require('util') +const readdir = promisify(require('readdir-scoped-modules')) const debug = require('../debug.js') const fromPath = require('../from-path.js') @@ -184,8 +186,10 @@ module.exports = cls => class IdealTreeBuilder extends cls { process.emit('time', 'idealTree') - if (!options.add && !options.rm && this[_global]) - return Promise.reject(new Error('global requires an add or rm option')) + if (!options.add && !options.rm && !options.update && this[_global]) { + const er = new Error('global requires add, rm, or update option') + return Promise.reject(er) + } // first get the virtual tree, if possible. If there's a lockfile, then // that defines the ideal tree, unless the root package.json is not @@ -307,7 +311,6 @@ module.exports = cls => class IdealTreeBuilder extends cls { // cases we don't use a lockfile anyway. // Load on a new Arborist object, so the Nodes aren't the same, // or else it'll get super confusing when we change them! - // Only have to mapWorkspaces if we didn't get it from actual or virtual .then(async root => { if (!this[_updateAll] && !this[_global] && !root.meta.loadedFromDisk) await new this.constructor(this.options).loadActual({ root }) @@ -324,10 +327,10 @@ module.exports = cls => class IdealTreeBuilder extends cls { } [_globalRootNode] () { - const root = this[_rootNodeFromPackage]({}) + const root = this[_rootNodeFromPackage]({ dependencies: {} }) // this is a gross kludge to handle the fact that we don't save // metadata on the root node in global installs, because the "root" - // node is something like /usr/local/lib/node_modules. + // node is something like /usr/local/lib. const meta = new Shrinkwrap({ path: this.path }) meta.reset() root.meta = meta @@ -355,9 +358,19 @@ module.exports = cls => class IdealTreeBuilder extends cls { // If we have a list of package names to update, and we know it's // going to update them wherever they are, add any paths into those // named nodes to the buildIdealTree queue. - if (this[_updateNames].length) + if (!this[_global] && this[_updateNames].length) this[_queueNamedUpdates]() + // global updates only update the globalTop nodes, but we need to know + // that they're there, and not reinstall the world unnecessarily. + if (this[_global] && (this[_updateAll] || this[_updateNames].length)) { + const nm = resolve(this.path, 'node_modules') + for (const name of await readdir(nm)) { + if (this[_updateAll] || this[_updateNames].includes(name)) + this.idealTree.package.dependencies[name] = '*' + } + } + if (this.auditReport && this.auditReport.size > 0) this[_queueVulnDependents](options) diff --git a/lib/arborist/reify.js b/lib/arborist/reify.js index 61ab94e70..b10637bd0 100644 --- a/lib/arborist/reify.js +++ b/lib/arborist/reify.js @@ -172,7 +172,7 @@ module.exports = cls => class Reifier extends cls { ignoreMissing: true, global: true, filter: (node, kid) => !node.isRoot ? true - : this[_explicitRequests].has(kid), + : (node.edgesOut.has(kid) || this[_explicitRequests].has(kid)), } : { ignoreMissing: true } if (!this[_global]) { diff --git a/tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js b/tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js index aecd08ecd..9d8e79546 100644 --- a/tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js +++ b/tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js @@ -91620,6 +91620,185 @@ Node { } ` +exports[`test/arborist/build-ideal-tree.js TAP update global > update a single dep 1`] = ` +Node { + "children": Map { + "once" => Node { + "children": Map { + "wrappy" => Node { + "edgesIn": Set { + Edge { + "from": "node_modules/once", + "name": "wrappy", + "spec": "1", + "type": "prod", + }, + }, + "location": "node_modules/once/node_modules/wrappy", + "name": "wrappy", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + }, + }, + "edgesIn": Set { + Edge { + "from": "", + "name": "once", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "wrappy" => Edge { + "name": "wrappy", + "spec": "1", + "to": "node_modules/once/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/once", + "name": "once", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + }, + }, + "edgesOut": Map { + "once" => Edge { + "name": "once", + "spec": "*", + "to": "node_modules/once", + "type": "prod", + }, + }, + "location": "", + "name": "build-ideal-tree-update-global", + "resolved": null, +} +` + +exports[`test/arborist/build-ideal-tree.js TAP update global > update all the deps 1`] = ` +Node { + "children": Map { + "@isaacs/testing-dev-optional-flags" => Node { + "children": Map { + "own-or" => Node { + "edgesIn": Set { + Edge { + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "own-or", + "spec": "^1.0.0", + "type": "optional", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "name": "own-or", + "optional": true, + "resolved": "https://registry.npmjs.org/own-or/-/own-or-1.0.0.tgz", + }, + "wrappy" => Node { + "edgesIn": Set { + Edge { + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "wrappy", + "spec": "^1.0.2", + "type": "optional", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", + "name": "wrappy", + "optional": true, + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + }, + }, + "edgesIn": Set { + Edge { + "from": "", + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "own-or" => Edge { + "name": "own-or", + "spec": "^1.0.0", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "type": "optional", + }, + "wrappy" => Edge { + "name": "wrappy", + "spec": "^1.0.2", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", + "type": "optional", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "@isaacs/testing-dev-optional-flags", + "resolved": "https://registry.npmjs.org/@isaacs/testing-dev-optional-flags/-/testing-dev-optional-flags-1.0.0.tgz", + }, + "once" => Node { + "children": Map { + "wrappy" => Node { + "edgesIn": Set { + Edge { + "from": "node_modules/once", + "name": "wrappy", + "spec": "1", + "type": "prod", + }, + }, + "location": "node_modules/once/node_modules/wrappy", + "name": "wrappy", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + }, + }, + "edgesIn": Set { + Edge { + "from": "", + "name": "once", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "wrappy" => Edge { + "name": "wrappy", + "spec": "1", + "to": "node_modules/once/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/once", + "name": "once", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + }, + }, + "edgesOut": Map { + "@isaacs/testing-dev-optional-flags" => Edge { + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "to": "node_modules/@isaacs/testing-dev-optional-flags", + "type": "prod", + }, + "once" => Edge { + "name": "once", + "spec": "*", + "to": "node_modules/once", + "type": "prod", + }, + }, + "location": "", + "name": "build-ideal-tree-update-global", + "resolved": null, +} +` + +exports[`test/arborist/build-ideal-tree.js TAP update global > updating sub-dep has no effect 1`] = ` +Node { + "location": "", + "name": "build-ideal-tree-update-global", + "resolved": null, +} +` + exports[`test/arborist/build-ideal-tree.js TAP update mkdirp to non-minimist-using version > must match snapshot 1`] = ` Node { "children": Map { diff --git a/test/arborist/build-ideal-tree.js b/test/arborist/build-ideal-tree.js index 4aeb6d85b..35c68119a 100644 --- a/test/arborist/build-ideal-tree.js +++ b/test/arborist/build-ideal-tree.js @@ -2154,7 +2154,7 @@ t.test('always prefer deduping peer deps', async t => { 'package.json': JSON.stringify({ dependencies: { '@pmmmwh/react-refresh-webpack-plugin': '0.4.2', - 'ink': '3.0.8', + ink: '3.0.8', 'react-reconciler': '0.25.0', }, }), @@ -2170,7 +2170,7 @@ t.test('do not ever nest peer deps underneath their dependent ever', async t => const path = t.testdir({ 'package.json': JSON.stringify({ dependencies: { - 'ink': '3.0.8', + ink: '3.0.8', // this peer depends on react 17 'react-reconciler': '0.26.0', }, @@ -2212,3 +2212,66 @@ t.test('properly assign fsParent when paths have .. in them', async t => { t.equal(tree.inventory.has(target.fsParent), true, 'other targets have fsParent') } }) + +t.test('update global', async t => { + // global root + // ├─┬ @isaacs/testing-dev-optional-flags@1.0.0 + // │ ├── own-or@1.0.0 + // │ └── wrappy@1.0.2 + // └─┬ once@1.3.1 + // └── wrappy@1.0.1 + + const path = t.testdir({ + node_modules: { + '@isaacs': { + 'testing-dev-optional-flags': { + 'package.json': JSON.stringify({ + name: '@isaacs/testing-dev-optional-flags', + version: '1.0.0', + dependencies: { + wrappy: '^1.0.2', + 'own-or': '^1.0.0', + }, + }), + node_modules: { + 'own-or': { + 'package.json': JSON.stringify({ + name: 'own-or', + version: '1.0.0', + }), + }, + wrappy: { + 'package.json': JSON.stringify({ + name: 'wrappy', + version: '1.0.0', + }), + }, + }, + }, + }, + once: { + 'package.json': JSON.stringify({ + name: 'once', + version: '1.3.1', + dependencies: { + wrappy: '1', + }, + }), + node_modules: { + wrappy: { + 'package.json': JSON.stringify({ + name: 'wrappy', + version: '1.0.1', + }), + }, + }, + }, + }, + }) + t.matchSnapshot(await printIdeal(path, { global: true, update: ['wrappy'] }), + 'updating sub-dep has no effect') + t.matchSnapshot(await printIdeal(path, { global: true, update: ['once'] }), + 'update a single dep') + t.matchSnapshot(await printIdeal(path, { global: true, update: true }), + 'update all the deps') +})