From 68ecfac8b9b9a0b81d73759a11483863c32cfcae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Sat, 7 Oct 2017 18:58:14 -0700 Subject: [PATCH] feat(lifecycle): run scripts in dep order (#23) Fixes: #16 --- index.js | 108 ++++++++++++++++++++++++-------------------- package-lock.json | 5 ++ package.json | 1 + test/specs/index.js | 30 +++++++++--- 4 files changed, 88 insertions(+), 56 deletions(-) diff --git a/index.js b/index.js index dc546b5..2c15330 100644 --- a/index.js +++ b/index.js @@ -9,6 +9,7 @@ const getPrefix = require('./lib/get-prefix.js') const lifecycle = require('npm-lifecycle') const lockVerify = require('lock-verify') const path = require('path') +const logi = require('npm-logical-tree') const rimraf = BB.promisify(require('rimraf')) const readFileAsync = BB.promisify(fs.readFile) @@ -19,22 +20,38 @@ class Installer { // Stats this.startTime = Date.now() - this.runTime = null + this.runTime = 0 this.pkgCount = 0 // Misc this.pkg = null - this.scriptQ = [] } run () { + return this.prepare() + .then(() => this.runScript('preinstall', this.pkg, this.prefix)) + .then(() => this.extractTree(this.logicalTree)) + .then(() => this.runScript('install', this.pkg, this.prefix)) + .then(() => this.runScript('postinstall', this.pkg, this.prefix)) + .then(() => { + extract.stopWorkers() + this.runTime = Date.now() - this.startTime + return this + }, e => { + extract.stopWorkers() + throw e + }) + } + + prepare () { extract.startWorkers() return ( this.opts.prefix ? BB.resolve(this.opts.prefix) : getPrefix(process.cwd()) - ).then(prefix => { + ) + .then(prefix => { this.prefix = prefix return BB.join( readJson(prefix, 'package.json'), @@ -45,32 +62,17 @@ class Installer { this.pkg = pkg } ) - }).then(() => { - return config(this.prefix, process.argv, this.pkg) - }).then(conf => { + }) + .then(() => config(this.prefix, process.argv, this.pkg)) + .then(conf => { this.config = conf return BB.join( this.checkLock(), rimraf(path.join(this.prefix, 'node_modules')) ) }).then(() => { - return this.runScript('preinstall', this.pkg, this.prefix) - }).then(() => { - return this.extractDeps( - path.join(this.prefix, 'node_modules'), - this.pkg._shrinkwrap.dependencies - ) - }).then(() => { - return this.runScript('install', this.pkg, this.prefix) - }).then(() => { - return this.runScript('postinstall', this.pkg, this.prefix) - }).then(() => { - extract.stopWorkers() - this.runTime = Date.now() - this.startTime - return this - }, e => { - extract.stopWorkers() - throw e + // This needs to happen -after- we've done checkLock() + this.logicalTree = logi(this.pkg, this.pkg._shrinkwrap) }) } @@ -97,36 +99,31 @@ class Installer { }) } - extractDeps (modPath, deps) { - return BB.map(Object.keys(deps || {}), name => { - const child = deps[name] - const childPath = path.join(modPath, name) - return extract.child(name, child, childPath, this.config).then(() => { - return readJson(childPath, 'package.json') - }).tap(pkg => { + extractTree (tree) { + const deps = tree.dependencies.values() + return BB.map(deps, child => { + if (child.pending) { return hasCycle(child) || child.pending } + if (child.dev && this.config.config.production) { return } + const childPath = path.join( + this.prefix, + 'node_modules', + child.address.replace(/:/g, '/node_modules/') + ) + child.pending = BB.resolve() + .then(() => extract.child(child.name, child, childPath, this.config)) + .then(() => readJson(childPath, 'package.json')) + .then(pkg => { return this.runScript('preinstall', pkg, childPath) - }).then(pkg => { - return this.extractDeps( - path.join(childPath, 'node_modules'), child.dependencies - ).then(dependencies => { - return { - name, - package: pkg, - child, - childPath, - dependencies: dependencies.reduce((acc, dep) => { - acc[dep.name] = dep - return acc - }, {}) - } + .then(() => this.extractTree(child)) + .then(() => this.runScript('install', pkg, childPath)) + .then(() => this.runScript('postinstall', pkg, childPath)) + .then(() => { + this.pkgCount++ + return this }) - }).tap(full => { - this.pkgCount++ - return this.runScript('install', full.package, childPath) - }).tap(full => { - return this.runScript('postinstall', full.package, childPath) }) - }, {concurrency: 50}) + return child.pending + }, { concurrency: 50 }) } runScript (stage, pkg, pkgPath) { @@ -154,3 +151,14 @@ function readJson (jsonPath, name, ignoreMissing) { function stripBOM (str) { return str.replace(/^\uFEFF/, '') } + +function hasCycle (child, seen) { + seen = seen || new Set() + if (seen.has(child.address)) { + return true + } else { + seen.add(child.address) + const deps = Array.from(child.dependencies.values()) + return deps.some(dep => hasCycle(dep, seen)) + } +} diff --git a/package-lock.json b/package-lock.json index 4529c9e..2386892 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2883,6 +2883,11 @@ } } }, + "npm-logical-tree": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/npm-logical-tree/-/npm-logical-tree-1.0.0.tgz", + "integrity": "sha512-BoF9a9GNQgTJ6xbovo3wBmmVkDRdzqOkWfdUn5PDTdkiYcB4GXBk5inN/csKQMcXd4Z8zZs6Lqke6J+LK1iFCQ==" + }, "npm-package-arg": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/npm-package-arg/-/npm-package-arg-5.1.2.tgz", diff --git a/package.json b/package.json index 1ab81d8..129eb69 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "graceful-fs": "^4.1.11", "lock-verify": "^1.1.0", "npm-lifecycle": "^1.0.3", + "npm-logical-tree": "^1.0.0", "npm-package-arg": "^5.1.2", "npmlog": "^4.1.2", "pacote": "^6.0.2", diff --git a/test/specs/index.js b/test/specs/index.js index 61a5411..aadd8b0 100644 --- a/test/specs/index.js +++ b/test/specs/index.js @@ -115,11 +115,16 @@ test('handles dependency list with only shallow subdeps', t => { const prefix = fixtureHelper.write(pkgName, { 'package.json': { name: pkgName, - version: pkgVersion + version: pkgVersion, + dependencies: { + 'a': '^1' + } }, 'package-lock.json': { dependencies: { - a: {} + a: { + version: '1.1.1' + } }, lockfileVersion: 1 } @@ -150,13 +155,22 @@ test('handles dependency list with only deep subdeps', t => { const prefix = fixtureHelper.write(pkgName, { 'package.json': { name: pkgName, - version: pkgVersion + version: pkgVersion, + dependencies: { + a: '^1' + } }, 'package-lock.json': { dependencies: { a: { + version: '1.1.1', + requires: { + b: '2.2.2' + }, dependencies: { - b: {} + b: { + version: '2.2.2' + } } } }, @@ -206,11 +220,14 @@ test('runs lifecycle hooks of packages with env variables', t => { preinstall: writeEnvScript, install: writeEnvScript, postinstall: writeEnvScript + }, + dependencies: { + a: '^1' } }, 'package-lock.json': { dependencies: { - a: {} + a: { version: '1.0.0' } }, lockfileVersion: 1 } @@ -252,6 +269,7 @@ test('skips lifecycle scripts with ignoreScripts is set', t => { 'package.json': { name: pkgName, version: pkgVersion, + dependencies: { a: '^1' }, scripts: { preinstall: writeEnvScript, install: writeEnvScript, @@ -260,7 +278,7 @@ test('skips lifecycle scripts with ignoreScripts is set', t => { }, 'package-lock.json': { dependencies: { - a: {} + a: { version: '1.0.0' } }, lockfileVersion: 1 }