From 261af04248f211b0546172f116d690bfc1071111 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kat=20March=C3=A1n?= <kzm@sykosomatic.org>
Date: Tue, 14 Feb 2017 00:38:31 -0800
Subject: [PATCH] manifest: standardize manifest format and centralize filling
 in deets

---
 lib/finalize-manifest.js       | 126 +++++++++++++++++++++++++++++++++
 lib/registry/manifest.js       |  50 ++++---------
 lib/registry/tarball.js        |  10 +--
 lib/util/extract-shrinkwrap.js |  66 -----------------
 manifest.js                    |   4 +-
 5 files changed, 148 insertions(+), 108 deletions(-)
 create mode 100644 lib/finalize-manifest.js
 delete mode 100644 lib/util/extract-shrinkwrap.js

diff --git a/lib/finalize-manifest.js b/lib/finalize-manifest.js
new file mode 100644
index 0000000..fda2bef
--- /dev/null
+++ b/lib/finalize-manifest.js
@@ -0,0 +1,126 @@
+'use strict'
+
+var checksumStream = require('./util/checksum-stream')
+var dezalgo = require('dezalgo')
+var finished = require('mississippi').finished
+var gunzip = require('./util/gunzip-maybe')
+var minimatch = require('minimatch')
+var normalize = require('normalize-package-data')
+var optCheck = require('./util/opt-check')
+var path = require('path')
+var pipe = require('mississippi').pipe
+var pipeline = require('mississippi').pipeline
+var tar = require('tar-stream')
+var through = require('mississippi').through
+
+module.exports = finalizeManifest
+function finalizeManifest (pkg, spec, where, opts, cb) {
+  completeFromTarball(pkg, spec, where, opts, function (err) {
+    if (err) { return cb(err) }
+    // normalize should not add any fields, and once
+    // makeManifest completes, it should never be modified.
+    cb(null, new Manifest(pkg, spec, where))
+  })
+}
+
+function Manifest (pkg, spec, where) {
+  this.name = pkg.name
+  this.version = pkg.version
+  this.dependencies = pkg.dependencies || {}
+  this.optionalDependencies = pkg.optionalDependencies || {}
+  this.devDependencies = pkg.devDependencies || {}
+  var bundled = (
+    pkg.bundledDependencies ||
+    pkg.bundleDependencies ||
+    false
+  )
+  this.bundleDependencies = !!bundled.length || false
+  this.peerDependencies = pkg.peerDependencies || {}
+
+  // This one depends entirely on each handler.
+  this._resolved = pkg._resolved
+
+  // Filled in by completeFromTarball as needed.
+  this._shasum = pkg._shasum
+  this._shrinkwrap = pkg._shrinkwrap || null
+  this.bin = pkg.bin || null
+
+  this._id = null // filled in by normalize-package-data, but unnecessary
+
+  Object.preventExtensions(this)
+  normalize(this)
+  Object.freeze(this)
+}
+
+// Some things aren't filled in by standard manifest fetching.
+// If this function needs to do its work, it will grab the
+// package tarball, extract it, and take whatever it needs
+// from the stream.
+function completeFromTarball (pkg, spec, where, opts, cb) {
+  cb = dezalgo(cb)
+  var needsShrinkwrap = (
+    pkg._hasShrinkwrap !== false ||
+    !pkg._shrinkwrap
+  )
+  var needsBin = (
+    !pkg.bin &&
+    pkg.directories &&
+    pkg.directories.bin
+  )
+  var needsShasum = !pkg._shasum
+  if (!needsShrinkwrap && !needsBin && !needsShasum) {
+    return cb(null)
+  } else {
+    opts = optCheck(opts)
+    opts.memoize = false
+    var tarball = require('./handlers/' + spec.type + '/tarball')
+    var tarData = tarball.fromManifest(pkg, spec, opts)
+    var shaStream = null
+    var extractorStream = null
+
+    if (needsShrinkwrap || needsBin) {
+      var dirBin = pkg.directories && pkg.directories.bin
+      pkg.bin = pkg.bin || {}
+      var dataStream = tar.extract()
+      extractorStream = pipeline(gunzip(), dataStream)
+      dataStream.on('entry', function (header, fileStream, next) {
+        var filePath = header.name.replace(/[^/]+\//, '')
+        if (needsShrinkwrap && filePath === 'npm-shrinkwrap.json') {
+          var srData = ''
+          fileStream.on('data', function (d) { srData += d })
+
+          return finished(fileStream, function (err) {
+            if (err) { return dataStream.emit('error', err) }
+            try {
+              pkg._shrinkwrap = JSON.parse(srData)
+              next()
+            } catch (e) {
+              dataStream.emit('error', e)
+            }
+          })
+        } else if (needsBin && minimatch(filePath, dirBin + '/**')) {
+          var relative = path.relative(dirBin, filePath)
+          if (relative && relative[0] !== '.') {
+            pkg.bin[path.basename(relative)] = path.join(dirBin, relative)
+          }
+        }
+        // Drain and get next one
+        fileStream.on('data', function () {})
+        next()
+      })
+    } else {
+      extractorStream = through()
+    }
+    if (needsShasum) {
+      shaStream = checksumStream(null, opts.hashAlgorithm)
+      shaStream.on('digest', function (d) {
+        pkg._shasum = d
+      })
+    } else {
+      shaStream = through()
+    }
+    // Drain the end stream
+    extractorStream.on('data', function () {})
+    return pipe(tarData, shaStream, extractorStream, cb)
+  }
+}
diff --git a/lib/registry/manifest.js b/lib/registry/manifest.js
index 3618f84..f150c2c 100644
--- a/lib/registry/manifest.js
+++ b/lib/registry/manifest.js
@@ -1,10 +1,8 @@
-var extractShrinkwrap = require('../util/extract-shrinkwrap')
 var optCheck = require('../util/opt-check')
 var pickManifest = require('./pick-manifest')
 var pickRegistry = require('./pick-registry')
 var url = require('url')
 var request = require('./request')
-var tarball = require('./tarball')
 
 module.exports = manifest
 function manifest (spec, opts, cb) {
@@ -26,13 +24,21 @@ function manifest (spec, opts, cb) {
       defaultTag: opts.defaultTag
     }, function (err, manifest) {
       if (err) { return cb(err) }
-      opts.log.silly(
-        'registry.manifest',
-        spec.name + '@' + spec.spec,
-        'resolved to',
-        manifest.name + '@' + manifest.version
+      // Done here instead of ./finalize-manifest because these fields
+      // have to be filled in differently depending on type.
+      if (!manifest._shasum) {
+        manifest._shasum = manifest.dist && manifest.dist.shasum
+      }
+      manifest._resolved = (
+        manifest.dist && manifest.dist.tarball
+      ) || url.resolve(
+        registry,
+        '/' + manifest.name +
+        '/-/' + manifest.name +
+        '-' + manifest.version +
+        '.tgz'
       )
-      ensureShrinkwrap(manifest, registry, opts, cb)
+      cb(null, manifest)
     })
   })
 }
@@ -43,31 +49,3 @@ function metadataUrl (registry, name) {
   : registry
   return url.resolve(normalized, name)
 }
-
-function ensureShrinkwrap (manifest, registry, opts, cb) {
-  if (manifest._hasShrinkwrap === false || manifest._shrinkwrap || !manifest.dist) {
-    opts.log.silly('registry.manifest', 'shrinkwrap extraction not needed')
-    cb(null, manifest)
-  } else {
-    opts.log.silly('registry.manifest', 'extracting shrinkwrap')
-    opts.memoize = false
-    var shrinkwrap
-    var tarstream = tarball.fromManifest(manifest, registry, opts)
-    extractShrinkwrap(tarstream, opts, function (err, sr) {
-      if (err) { return cb(err) }
-      shrinkwrap = sr
-    })
-    tarstream.on('data', function () {})
-    tarstream.on('error', cb)
-    tarstream.on('end', function () {
-      manifest._hasShrinkwrap = !!shrinkwrap
-      manifest._shrinkwrap = shrinkwrap
-      if (shrinkwrap) {
-        opts.log.silly('registry.manifest', 'shrinkwrap found')
-      } else {
-        opts.log.silly('registry.manifest', 'no shrinkwrap')
-      }
-      cb(null, manifest)
-    })
-  }
-}
diff --git a/lib/registry/tarball.js b/lib/registry/tarball.js
index 0780d37..d609e61 100644
--- a/lib/registry/tarball.js
+++ b/lib/registry/tarball.js
@@ -8,7 +8,6 @@ var through = require('mississippi').through
 module.exports = tarball
 function tarball (spec, opts) {
   opts = optCheck(opts)
-  var registry = pickRegistry(spec, opts)
   opts.log.verbose(
     'registry.tarball',
     'looking up registry-based metadata for ', spec
@@ -21,7 +20,7 @@ function tarball (spec, opts) {
       'registry metadata found. Downloading ', manifest.name + '@' + manifest.version
     )
     pipe(
-      fromManifest(manifest, registry),
+      fromManifest(manifest, spec, opts),
       stream
     )
   })
@@ -29,9 +28,10 @@ function tarball (spec, opts) {
 }
 
 module.exports.fromManifest = fromManifest
-function fromManifest (manifest, registry, opts) {
+function fromManifest (manifest, spec, opts) {
   opts = optCheck(opts)
-  var uri = manifest.dist && manifest.dist.tarball
-  opts.digest = manifest.dist && manifest.dist.shasum
+  var registry = pickRegistry(spec, opts)
+  var uri = manifest._resolved
+  opts.digest = manifest._shasum
   return request.stream(uri, registry, opts)
 }
diff --git a/lib/util/extract-shrinkwrap.js b/lib/util/extract-shrinkwrap.js
deleted file mode 100644
index 7ec5d10..0000000
--- a/lib/util/extract-shrinkwrap.js
+++ /dev/null
@@ -1,66 +0,0 @@
-var finished = require('mississippi').finished
-var gunzip = require('./gunzip-maybe')
-var pipe = require('mississippi').pipe
-var pipeline = require('mississippi').pipeline
-var through = require('mississippi').through
-
-// `tar-stream` is significantly faster than `node-tar`, but there's
-// a risk that `tar-stream` can cause some incompatibilities.
-//
-// It's worth keeping an eye out for issues caused by this, and
-// swap out the module for node-tar if they rear their head.
-var tar = require('tar-stream')
-
-// Picks out just the `npm-shrinkwrap.json` from a package
-// tarball (coming through `pkgStream`), and stops parsing the
-// tarball as soon as the file is found.
-module.exports = extractShrinkwrap
-function extractShrinkwrap (pkgStream, opts, cb) {
-  var extract = tar.extract()
-
-  // The extra `through` is to compensate for misbehaving `pkgStream`s.
-  // For example, `request` streams are notoriously unreliable.
-  // This is a bit of defensive programming, not a fix for
-  // a specific known example of an issue.
-  var unzipped = pipeline(through(), gunzip(), extract)
-
-  var shrinkwrap = null // we'll pop the data in here if found.
-  extract.on('entry', function onEntry (header, fileStream, next) {
-    if (header.name === 'package/npm-shrinkwrap.json') {
-      opts.log.silly('extract-shrinkwrap', 'found shrinkwrap')
-      // got a shrinkwrap! Now we don't need to look for entries anymore.
-      extract.removeListener('entry', onEntry)
-
-      // Grab all the file data off the entry fileStream.
-      var data = ''
-      fileStream.on('data', function (d) { data += d })
-
-      finished(fileStream, function (err) {
-        if (err) { return extract.emit('error', err) }
-        try {
-          shrinkwrap = JSON.parse(data)
-        } catch (e) {
-          extract.emit('error', e)
-        }
-        // By destroying `unzipped`, this *should* stop `tar-stream`
-        // from continuing to waste resources on tarball parsing.
-        unzipped.unpipe()
-        cb(null, shrinkwrap)
-      })
-    } else {
-      // Not a shrinkwrap. Autodrain this entry, and move on to the next.
-      fileStream.resume()
-      next()
-    }
-  })
-
-  // Any other streams that `pkgStream` is being piped to should
-  // remain unaffected by this, although there might be confusion
-  // around backpressure issues.
-  pipe(pkgStream, unzipped, function (err) {
-    // If we already successfully emitted a shrinkwrap,
-    // we literally don't care about any errors.
-    // Issues with `pkgStream` can be handled elsewhere if needed.
-    if (!shrinkwrap) { cb(err) }
-  })
-}
diff --git a/manifest.js b/manifest.js
index e72d0ae..02dd655 100644
--- a/manifest.js
+++ b/manifest.js
@@ -1,3 +1,4 @@
+var finalizeManifest = require('./lib/finalize-manifest')
 var optCheck = require('./lib/util/opt-check')
 var rps = require('realize-package-specifier')
 
@@ -15,7 +16,8 @@ function manifest (spec, opts, cb) {
     if (err) { return cb(err) }
     var fetcher = handlers[res.type] || (handlers[res.type] = require('./lib/handlers/' + res.type + '/manifest'))
     fetcher(res, opts, function (err, mani) {
-      cb(err, mani)
+      if (err) { return cb(err) }
+      finalizeManifest(mani, res, opts.where, opts, cb)
     })
   })
 }