feat(rebuild): add lifecycle scripts dependencies

lib/arborist/rebuild.js

@@ -249,73 +249,214 @@ 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()
+
+        if (!Script.pollRefs)
+          Script.pollRefs = new Map()
+      }
+
+      // releases any refs used in the pool and polling systems
+      static end () {
+        if (Script.runPromises)
+          Script.runPromises.clear()
+
+        if (Script.pollRefs)
+          Script.pollRefs.clear()
+
+        delete Script.runPromises
+        delete Script.pollRefs
+      }
+
+      // make sure there's always just a single, polling system running
+      // at a given time, this method will return a promise that resolves
+      // once the original promise tracked in Script.runPromises is available
+      static waitRunPromise (ref) {
+        const pollAgain = () => setTimeout(poll, 10)
+        const poll = () => {
+          // check if the run-script promise is available for each entry
+          // in the polling queue, if so take it out of the queue
+          // and resolve its pending promise
+          for (const [pollRef, resolve] of Script.pollRefs.entries()) {
+            const p = Script.runPromises.get(pollRef)
+            if (p) {
+              Script.pollRefs.delete(pollRef)
+              resolve(p)
+            }
+          }
+
+          // only poll again if there are still refs left in the queue
+          if (Script.pollRefs.size)
+            pollAgain()
+        }
+
+        // starts the polling system for the first time, given that this
+        // part of the code is still synchronous, it's going to only start
+        // it once the poll refs are empty
+        if (!Script.pollRefs.size)
+          pollAgain()
+
+        // creates a new promise that is going to be returned by this fuction
+        // and also be tracked in the polling system along with its queue ref
+        let resolve
+        const p = new Promise((res) => {
+          resolve = res
+        })
+        Script.pollRefs.set(ref, resolve)
+        return p
+      }
+
+      // 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
+      linkLinkedDepsResolved () {
+        return Promise.all(
+          this.linkLinkedDeps()
+            .map(ref => Script.waitRunPromise(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
+      }
+
+      end () {
+        delete this.node
+        delete this.timer
+        delete this.opts
+        delete this.pkg
+        delete this.path
+        delete this.env
+        delete this.ref
+      }
+    }
+
     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)
+
+        script.end()
+        process.emit('timeEnd', script.timer)
+      }), limit)
 
-      await (this[_doHandleOptionalFailure]
-        ? this[_handleOptionalFailure](node, p)
-        : p)
+    // releases pool refs
+    Script.end()
 
-      process.emit('timeEnd', timer)
-    }), limit)
     process.emit('timeEnd', `build:run:${event}`)
   }
 

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()
+})