Merge pull request #29 from rstacruz/separate-build-and-fetch

Separate build and fetch steps

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]
+}

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.
@@ -42,27 +43,31 @@ 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
     // => { 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 || []),
 
-    // 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 }
     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) {
@@ -88,7 +93,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)
@@ -111,13 +116,17 @@ module.exports = function install (ctx, pkgSpec, modules, options) {
  */
 
 function buildToStoreCached (ctx, paths, pkg, log) {
-  if (isCircular(pkg)) {
-    return Promise.resolve()
-  } else {
-    return make(paths.target, ctx.builds[pkg.fullname], _ =>
-      memoize(ctx.builds, pkg.fullname, _ =>
-        buildToStore(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]
+
+  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))
+    ))
 }
 
 /*
@@ -125,9 +134,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,8 +149,17 @@ 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
-    .then(_ => { pkg.fulldata = require(abspath(join(paths.tmp, 'package.json'))) })
+    // 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')
+  var fulldata
+
+  return Promise.resolve()
+    .then(_ => { fulldata = requireJson(abspath(join(paths.tmp, 'package.json'))) })
 
     // link node_modules/.bin
     .then(_ => linkBins(paths.modules, paths.tmp, paths.target))
@@ -161,7 +177,7 @@ function buildToStore (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')))
@@ -197,30 +213,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
+  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))
-}
-
-/*
- * Checks if the current package is a circular dependency.
- */
-
-function isCircular (pkg) {
-  return pkg.keypath.indexOf(pkg.fullname) > -1
+  var out = join(modules, pkgData.name)
+  return mkdirp(dirname(out))
+    .then(_ => relSymlink(target, out))
 }
 
 /*
@@ -234,7 +241,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 => {