diff --git a/lib/arborist/rebuild.js b/lib/arborist/rebuild.js index 8e447bb8f..cf938cc28 100644 --- a/lib/arborist/rebuild.js +++ b/lib/arborist/rebuild.js @@ -249,73 +249,162 @@ module.exports = cls => class Builder extends cls { if (!queue.length) return + // helper class to manage script execution + class Script { + constructor (opts) { + const { + node, + timer, + ...runScriptOpts + } = opts + this.node = node + this.timer = timer + this.opts = runScriptOpts + this.pkg = opts.pkg + this.path = opts.path + this.env = opts.env + this.ref = this.node.target.path + + if (!Script.runPromises) + Script.runPromises = new Map() + } + + // when the script to be run belongs to a link node, this method + // retrieves a list of all other link nodes that are direct dependencies + // of the current node, their scripts also have to be part of the current + // queue, otherwise it just returns an empty list + // this is necessary in order to enable workspaces lifecycle scripts + // that depend on each other + linkLinkedDeps () { + const res = [] + + if (!this.node.isLink) + return res + + for (const edge of this.node.target.edgesOut.values()) { + if (edge.to.isLink && + queue.some(node => node.isLink && node.target.name === edge.name)) + res.push(edge.to.target.path) + } + + return res + } + + // use the list of depended scripts from linkLinkedDeps() + // and returns a Promise that resolves once all scripts + // this current script depends are resolved + async linkLinkedDepsResolved () { + const resolved = (ref) => { + const p = Script.runPromises.get(ref) + // polling mechanism used in case the depended script is not + // yet set in Script.runPromises, this allow us to keep the + // alphabetical order of execution at first and the dependency + // mechanism for linked deps is only used when necessary + if (!p) { + return new Promise(res => + setTimeout(() => res(resolved(ref)), 1)) + } + return p + } + + return await Promise.all( + this.linkLinkedDeps().map(ref => resolved(ref))) + } + + async run () { + // in case the current script belongs to a link node that also has + // linked nodes dependencies (e.g: workspaces that depend on each other) + // then we await for that dependency script to be resolved before + // executing the current script + await this.linkLinkedDepsResolved() + + // executes the current script using @npmcli/run-script + const p = runScript(this.opts) + + // keeps a pool of executing scripts in order to track + // inter-dependencies between these scripts + Script.runPromises.set(this.ref, p) + return p + } + } + process.emit('time', `build:run:${event}`) const stdio = this.options.foregroundScripts ? 'inherit' : 'pipe' const limit = this.options.foregroundScripts ? 1 : undefined - await promiseCallLimit(queue.map(node => async () => { - const { - path, - integrity, - resolved, - optional, - peer, - dev, - devOptional, - package: pkg, - location, - } = node.target - - // skip any that we know we'll be deleting - if (this[_trashList].has(path)) - return - - const timer = `build:run:${event}:${location}` - process.emit('time', timer) - this.log.info('run', pkg._id, event, location, pkg.scripts[event]) - const env = { - npm_package_resolved: resolved, - npm_package_integrity: integrity, - npm_package_json: resolve(path, 'package.json'), - npm_package_optional: boolEnv(optional), - npm_package_dev: boolEnv(dev), - npm_package_peer: boolEnv(peer), - npm_package_dev_optional: - boolEnv(devOptional && !dev && !optional), - } - const runOpts = { - event, - path, - pkg, - stdioString: true, - stdio, - env, - scriptShell: this[_scriptShell], - } - const p = runScript(runOpts).catch(er => { - const { code, signal } = er - this.log.info('run', pkg._id, event, {code, signal}) - throw er - }).then(({args, code, signal, stdout, stderr}) => { - this.scriptsRun.add({ - pkg, + await promiseCallLimit(queue + .map(node => { + const { path, + integrity, + resolved, + optional, + peer, + dev, + devOptional, + package: pkg, + location, + } = node.target + + // skip any that we know we'll be deleting + if (this[_trashList].has(path)) + return + + const timer = `build:run:${event}:${location}` + process.emit('time', timer) + this.log.info('run', pkg._id, event, location, pkg.scripts[event]) + const env = { + npm_package_resolved: resolved, + npm_package_integrity: integrity, + npm_package_json: resolve(path, 'package.json'), + npm_package_optional: boolEnv(optional), + npm_package_dev: boolEnv(dev), + npm_package_peer: boolEnv(peer), + npm_package_dev_optional: + boolEnv(devOptional && !dev && !optional), + } + return new Script({ + node, + timer, event, - cmd: args && args[args.length - 1], + path, + pkg, + stdioString: true, + stdio, env, - code, - signal, - stdout, - stderr, + scriptShell: this[_scriptShell], }) - this.log.info('run', pkg._id, event, {code, signal}) }) + .filter(Boolean) + .map(script => async () => { + const p = script.run().catch(er => { + const { code, signal } = er + this.log.info('run', script.pkg._id, event, {code, signal}) + throw er + }).then(({args, code, signal, stdout, stderr}) => { + this.scriptsRun.add({ + pkg: script.pkg, + path: script.path, + event, + cmd: args && args[args.length - 1], + env: script.env, + code, + signal, + stdout, + stderr, + }) + this.log.info('run', script.pkg._id, event, {code, signal}) + }) + + await (this[_doHandleOptionalFailure] + ? this[_handleOptionalFailure](script.node, p) + : p) + + process.emit('timeEnd', script.timer) + }), limit) - await (this[_doHandleOptionalFailure] - ? this[_handleOptionalFailure](node, p) - : p) + // release pool refs to scripts promises + delete Script.runPromises - process.emit('timeEnd', timer) - }), limit) process.emit('timeEnd', `build:run:${event}`) } diff --git a/test/arborist/rebuild.js b/test/arborist/rebuild.js index f2a3bb192..77c386239 100644 --- a/test/arborist/rebuild.js +++ b/test/arborist/rebuild.js @@ -713,3 +713,112 @@ t.test('only rebuild for workspace', async t => { t.equal(fs.readFileSync(adepTxt, 'utf8'), 'adep', 'adep rebuilt') t.throws(() => fs.readFileSync(bdepTxt, 'utf8'), { code: 'ENOENT' }, 'bdep not rebuilt') }) + +t.test('scripts dependencies between Link nodes', async t => { + // this scenario is laid out as such: the `prepare` script of each linked + // pkg needs the resulting files of its dependency `prepare` script: + // + // prepare: b -> a -> c + // postinstall: e -> a `prepare` + // + // in order to make sure concurrency is handled properly, the `prepare` + // script of "c" takes at least 20ms to complete, while "a" takes at + // least 10ms and the "b" script expects to run synchronously + + const path = t.testdir({ + 'package.json': JSON.stringify({ + dependencies: { + a: 'file:./packages/a', + b: 'file:./packages/b', + c: 'file:./packages/c', + d: 'file:./packages/d', + }, + }), + node_modules: { + a: t.fixture('symlink', '../packages/a'), + b: t.fixture('symlink', '../packages/b'), + c: t.fixture('symlink', '../packages/c'), + d: t.fixture('symlink', '../packages/d'), + abbrev: { + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.1.1', + }), + }, + }, + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + scripts: { + // on prepare script writes a `index.js` file containing: + // module.exports = require("c") + // this is a slow script though that sleeps for + // at least 10ms prior to writing that file + prepare: "node -e \"setTimeout(() => { require('fs').writeFileSync(require('path').resolve('index.js'), 'module.exports = require(function c(){}.name);') }, 10)\"", + }, + dependencies: { + c: '^1.0.0', + }, + }), + }, + b: { + 'package.json': JSON.stringify({ + name: 'b', + version: '1.0.0', + scripts: { + // on prepare script requires `./node_modules/a/index.js` which + // is a dependency of this workspace but with the caveat that this + // file is only build during the `prepare` script of "a" + prepare: "node -p \"require('a')\"", + }, + dependencies: { + a: '^1.0.0', + abbrev: '^1.0.0', + }, + }), + }, + c: { + 'package.json': JSON.stringify({ + name: 'c', + version: '1.0.0', + scripts: { + // on prepare script writes a `index.js` file containing: + // module.exports = "HELLO" + // this is an even slower slower script that sleeps for + // at least 20ms prior to writing that file + prepare: "node -e \"setTimeout(() => { require('fs').writeFileSync(require('path').resolve('index.js'), 'module.exports = function HELLO() {}.name;') }, 20)\"", + }, + }), + }, + d: { + 'package.json': JSON.stringify({ + name: 'd', + version: '1.0.0', + }), + }, + e: { + 'package.json': JSON.stringify({ + name: 'd', + version: '1.0.0', + scripts: { + // on postinstall script requires `./node_modules/a/index.js` which + // is a dependency of this workspace but with the caveat that this + // file is only build during the `prepare` script of "a" + // although different lifecycle scripts do not hold refs to + // depending on each other, all `prepare` scripts should already + // have been resolved by the time we run `postinstall` scripts + postinstall: "node -p \"require('a')\"", + }, + dependencies: { + a: '^1.0.0', + }, + }), + }, + }, + }) + + const arb = newArb({ path }) + await arb.rebuild() +})