From 60426a958d7c15a59426a50b958d2131cd0902bf Mon Sep 17 00:00:00 2001 From: "Rico Sta. Cruz" Date: Sat, 30 Jan 2016 14:03:22 +0800 Subject: [PATCH 1/5] Separate build and fetch steps --- lib/install.js | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/install.js b/lib/install.js index 2214f91ef9..849654b22f 100644 --- a/lib/install.js +++ b/lib/install.js @@ -116,7 +116,10 @@ function buildToStoreCached (ctx, paths, pkg, log) { } else { return make(paths.target, ctx.builds[pkg.fullname], _ => memoize(ctx.builds, pkg.fullname, _ => - buildToStore(ctx, paths, pkg, log))) + Promise.resolve() + .then(_ => fetchToStore(ctx, paths, pkg, log)) + .then(_ => buildInStore(ctx, paths, pkg, log)) + )) } } @@ -125,9 +128,7 @@ function buildToStoreCached (ctx, paths, pkg, log) { * Fetches from npm, recurses to dependencies, runs lifecycle scripts, etc */ -function buildToStore (ctx, paths, pkg, log) { - var installAll = require('./install_multiple') - +function fetchToStore (ctx, paths, pkg, log) { return Promise.resolve() // symlink .tmp/0a1b2c3d -> .store/lodash@4.0.0 // so that when any other module requires it, it's available even @@ -142,7 +143,15 @@ function buildToStore (ctx, paths, pkg, log) { .then(_ => fs.writeFile(join(paths.tmp, '.pnpm_inprogress'), '', 'utf-8')) .then(_ => fetch(paths.tmp, pkg.dist.tarball, pkg.dist.shasum, log)) - // update pkg.fulldata; to be used later + // TODO: this is the point it becomes partially useable. + // ie, it can now be symlinked into .store/foo@1.0.0. + // it is only here that it should be available for ciruclar dependencies. +} + +function buildInStore (ctx, paths, pkg, log) { + var installAll = require('./install_multiple') + + return Promise.resolve() .then(_ => { pkg.fulldata = require(abspath(join(paths.tmp, 'package.json'))) }) // link node_modules/.bin @@ -203,6 +212,9 @@ function symlinkSelf (target, pkg, depth) { */ function symlinkToModules (target, pkg, modules) { + // TODO: uncomment to make things fail + // require(join(target, 'package.json')) + // lodash -> .store/lodash@4.0.0 // .store/foo@1.0.0/node_modules/lodash -> ../../../.store/lodash@4.0.0 // .tmp/01234567890/node_modules/lodash -> ../../../.store/lodash@4.0.0 From 2c8c48c0c55ccb1c50290cab97ed9aafeb73c9b7 Mon Sep 17 00:00:00 2001 From: "Rico Sta. Cruz" Date: Sat, 30 Jan 2016 14:16:47 +0800 Subject: [PATCH 2/5] Cache fetches --- lib/install.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/install.js b/lib/install.js index 849654b22f..e1a8c40f39 100644 --- a/lib/install.js +++ b/lib/install.js @@ -42,6 +42,7 @@ var obliterate = require('./fs/obliterate') module.exports = function install (ctx, pkgSpec, modules, options) { debug('installing ' + pkgSpec) if (!ctx.builds) ctx.builds = {} + if (!ctx.fetches) ctx.fetches = {} var pkg = { // Preliminary spec data @@ -111,13 +112,16 @@ module.exports = function install (ctx, pkgSpec, modules, options) { */ function buildToStoreCached (ctx, paths, pkg, log) { + if (ctx.builds[pkg.fullname]) return ctx.fetches[pkg.fullname] + if (isCircular(pkg)) { return Promise.resolve() } else { return make(paths.target, ctx.builds[pkg.fullname], _ => memoize(ctx.builds, pkg.fullname, _ => Promise.resolve() - .then(_ => fetchToStore(ctx, paths, pkg, log)) + .then(_ => memoize(ctx.fetches, pkg.fullname, _ => + fetchToStore(ctx, paths, pkg, log))) .then(_ => buildInStore(ctx, paths, pkg, log)) )) } @@ -246,7 +250,9 @@ function make (path, isWorking, fn) { return fs.stat(path) .then(_ => { return fs.stat(join(path, '.pnpm_inprogress')) - .then(_ => { if (!isWorking) return obliterate(path).then(fn) }) + .then(_ => { + if (!isWorking) return obliterate(path).then(fn) + }) .catch(err => { if (err.code !== 'ENOENT') throw err }) }) .catch(err => { From a45c3d96678bee5332fc591bce8e1b9c713872d6 Mon Sep 17 00:00:00 2001 From: "Rico Sta. Cruz" Date: Sat, 30 Jan 2016 14:29:37 +0800 Subject: [PATCH 3/5] Symlink based on package.json name --- lib/fs/require_json.js | 12 ++++++++++++ lib/install.js | 31 +++++++++++++------------------ 2 files changed, 25 insertions(+), 18 deletions(-) create mode 100644 lib/fs/require_json.js diff --git a/lib/fs/require_json.js b/lib/fs/require_json.js new file mode 100644 index 0000000000..fe3bf6b51f --- /dev/null +++ b/lib/fs/require_json.js @@ -0,0 +1,12 @@ +var cache = {} + +/* + * Works identically to require('/path/to/file.json'), but safer. + */ + +module.exports = function requireJson (path) { + path = require('path').resolve(path) + if (cache[path]) return cache[path] + cache[path] = JSON.parse(require('fs').readFileSync(path, 'utf-8')) + return cache[path] +} diff --git a/lib/install.js b/lib/install.js index e1a8c40f39..7627a71ed9 100644 --- a/lib/install.js +++ b/lib/install.js @@ -15,6 +15,7 @@ var linkBundledDeps = require('./install/link_bundled_deps') var postInstall = require('./install/post_install') var fs = require('mz/fs') var obliterate = require('./fs/obliterate') +var requireJson = require('./fs/require_json') /* * Installs a package. @@ -60,10 +61,7 @@ module.exports = function install (ctx, pkgSpec, modules, options) { dist: undefined, // package.json data as retrieved from resolve() => { name, version, ... } - data: undefined, - - // package.json data as retrieved from actual package - fulldata: undefined + data: undefined } if (!pkg.spec.name) { @@ -89,7 +87,7 @@ module.exports = function install (ctx, pkgSpec, modules, options) { .then(_ => log('resolved', pkg.data)) .then(_ => buildToStoreCached(ctx, paths, pkg, log)) .then(_ => mkdirp(paths.modules)) - .then(_ => symlinkToModules(paths.target, pkg.spec, paths.modules))) + .then(_ => symlinkToModules(paths.target, paths.modules))) .then(_ => log('done')) .catch(err => { log('error', err) @@ -154,9 +152,10 @@ function fetchToStore (ctx, paths, pkg, log) { function buildInStore (ctx, paths, pkg, log) { var installAll = require('./install_multiple') + var fulldata return Promise.resolve() - .then(_ => { pkg.fulldata = require(abspath(join(paths.tmp, 'package.json'))) }) + .then(_ => { fulldata = requireJson(abspath(join(paths.tmp, 'package.json'))) }) // link node_modules/.bin .then(_ => linkBins(paths.modules, paths.tmp, paths.target)) @@ -174,7 +173,7 @@ function buildInStore (ctx, paths, pkg, log) { .then(_ => symlinkSelf(paths.tmp, pkg.data, pkg.keypath.length)) // postinstall hooks - .then(_ => postInstall(paths.tmp, pkg.fulldata, installLogger(log, pkg))) + .then(_ => postInstall(paths.tmp, fulldata, installLogger(log, pkg))) // move to .store/lodash@4.0.0; remove the stub done earlier .then(_ => fs.unlink(join(paths.tmp, '.pnpm_inprogress'))) @@ -210,25 +209,21 @@ function symlinkSelf (target, pkg, depth) { * Perform the final symlinking of ./.store/x@1.0.0 -> ./x. * * target = '/node_modules/.store/lodash@4.0.0' - * name = 'lodash' * modules = './node_modules' - * symlinkToModules(fullname, name, modules, 0) + * symlinkToModules(fullname, modules) */ -function symlinkToModules (target, pkg, modules) { +function symlinkToModules (target, modules) { // TODO: uncomment to make things fail - // require(join(target, 'package.json')) + var pkgData = requireJson(join(target, 'package.json')) + if (!pkgData.name) { throw new Error('Invalid package.json for ' + target) } // lodash -> .store/lodash@4.0.0 // .store/foo@1.0.0/node_modules/lodash -> ../../../.store/lodash@4.0.0 // .tmp/01234567890/node_modules/lodash -> ../../../.store/lodash@4.0.0 - if (pkg.scope) { - debug('make scope dir', pkg.scope) - return mkdirp(join(modules, pkg.scope)) - .then(_ => relSymlink(target, join(modules, pkg.name))) - } - - return relSymlink(target, join(modules, pkg.name)) + var out = join(modules, pkgData.name) + return mkdirp(dirname(out)) + .then(_ => relSymlink(target, out)) } /* From 2196e6c5f4869e232ab0c5dc89bf579c2b35014b Mon Sep 17 00:00:00 2001 From: "Rico Sta. Cruz" Date: Sat, 30 Jan 2016 14:33:38 +0800 Subject: [PATCH 4/5] Document pkg.fullname --- lib/install.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/install.js b/lib/install.js index 7627a71ed9..6e95a0b6d9 100644 --- a/lib/install.js +++ b/lib/install.js @@ -54,7 +54,12 @@ module.exports = function install (ctx, pkgSpec, modules, options) { // => ['babel-core@6.4.5', 'babylon@6.4.5', 'babel-runtime@5.8.35'] keypath: (options && options.keypath || []), - // Full name of package => 'lodash@4.0.0' + // Full name of package as it should be put in the store. Aim to make + // this as friendly as possible as this will appear in stack traces. + // => 'lodash@4.0.0' + // => '@rstacruz!tap-spec@4.1.1' + // => 'rstacruz!pnpm.js@0a1b382da' + // => 'foobar@9a3b283ac' fullname: undefined, // Distribution data from resolve() => { shasum, tarball } From 087cfe912718b2b414a87c222ec1b552c166469a Mon Sep 17 00:00:00 2001 From: "Rico Sta. Cruz" Date: Sat, 30 Jan 2016 14:37:07 +0800 Subject: [PATCH 5/5] Remove circular dependency management --- lib/install.js | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/lib/install.js b/lib/install.js index 6e95a0b6d9..1643a0825f 100644 --- a/lib/install.js +++ b/lib/install.js @@ -50,7 +50,8 @@ module.exports = function install (ctx, pkgSpec, modules, options) { // => { raw, name, scope, type, spec, rawSpec } spec: npa(pkgSpec), - // Dependency path to the current package + // Dependency path to the current package. Not actually needed anmyore + // outside getting its length // => ['babel-core@6.4.5', 'babylon@6.4.5', 'babel-runtime@5.8.35'] keypath: (options && options.keypath || []), @@ -115,19 +116,17 @@ module.exports = function install (ctx, pkgSpec, modules, options) { */ function buildToStoreCached (ctx, paths, pkg, log) { + // If a package is requested for a second time (usually when many packages depend + // on the same thing), only resolve until it's fetched (not built). if (ctx.builds[pkg.fullname]) return ctx.fetches[pkg.fullname] - if (isCircular(pkg)) { - return Promise.resolve() - } else { - return make(paths.target, ctx.builds[pkg.fullname], _ => - memoize(ctx.builds, pkg.fullname, _ => - Promise.resolve() - .then(_ => memoize(ctx.fetches, pkg.fullname, _ => - fetchToStore(ctx, paths, pkg, log))) - .then(_ => buildInStore(ctx, paths, pkg, log)) - )) - } + return make(paths.target, ctx.builds[pkg.fullname], _ => + memoize(ctx.builds, pkg.fullname, _ => + Promise.resolve() + .then(_ => memoize(ctx.fetches, pkg.fullname, _ => + fetchToStore(ctx, paths, pkg, log))) + .then(_ => buildInStore(ctx, paths, pkg, log)) + )) } /* @@ -231,14 +230,6 @@ function symlinkToModules (target, modules) { .then(_ => relSymlink(target, out)) } -/* - * Checks if the current package is a circular dependency. - */ - -function isCircular (pkg) { - return pkg.keypath.indexOf(pkg.fullname) > -1 -} - /* * If `path` doesn't exist, run `fn()`. * If it exists and is not in progress, don't do anything.