fix: removes [_complete] check to apply correct metadata

Closes #2606

PR-URL: #287
Credit: @gimli01
Close: #287
Reviewed-by: @nlf

lib/arborist/build-ideal-tree.js

@@ -7,7 +7,7 @@ const semver = require('semver')
 const promiseCallLimit = require('promise-call-limit')
 const getPeerSet = require('../peer-set.js')
 const realpath = require('../../lib/realpath.js')
-const { resolve } = require('path')
+const { resolve, dirname } = require('path')
 const { promisify } = require('util')
 const treeCheck = require('../tree-check.js')
 const readdir = promisify(require('readdir-scoped-modules'))
@@ -661,7 +661,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
     const ancient = meta.ancientLockfile
     const old = meta.loadedFromDisk && !(meta.originalLockfileVersion >= 2)
 
-    if (inventory.size === 0 || !ancient && !(old && this[_complete]))
+    if (inventory.size === 0 || !ancient && !old)
       return
 
     // if the lockfile is from node v5 or earlier, then we'll have to reload
@@ -688,10 +688,12 @@ This is a one-time fix-up, please be patient...
         this.log.silly('inflate', node.location)
         const { resolved, version, path, name, location, integrity } = node
         // don't try to hit the registry for linked deps
-        const useResolved = !version ||
-          resolved && resolved.startsWith('file:')
-        const id = useResolved ? resolved : version
-        const spec = npa.resolve(name, id, path)
+        const useResolved = resolved && (
+          !version || resolved.startsWith('file:')
+        )
+        const id = useResolved ? resolved
+          : version || `file:${node.path}`
+        const spec = npa.resolve(name, id, dirname(path))
         const sloc = location.substr('node_modules/'.length)
         const t = `idealTree:inflate:${sloc}`
         this.addTracker(t)

lib/arborist/reify.js

@@ -2,7 +2,6 @@
 
 const onExit = require('../signal-handling.js')
 const pacote = require('pacote')
-const rpj = require('read-package-json-fast')
 const AuditReport = require('../audit-report.js')
 const {subset, intersects} = require('semver')
 const npa = require('npm-package-arg')
@@ -57,7 +56,6 @@ const _extractOrLink = Symbol('extractOrLink')
 const _checkBins = Symbol.for('checkBins')
 const _symlink = Symbol('symlink')
 const _warnDeprecated = Symbol('warnDeprecated')
-const _loadAncientPackageDetails = Symbol('loadAncientPackageDetails')
 const _loadBundlesAndUpdateTrees = Symbol.for('loadBundlesAndUpdateTrees')
 const _submitQuickAudit = Symbol('submitQuickAudit')
 const _awaitQuickAudit = Symbol('awaitQuickAudit')
@@ -522,7 +520,6 @@ module.exports = cls => class Reifier extends cls {
         await this[_checkBins](node)
         await this[_extractOrLink](node)
         await this[_warnDeprecated](node)
-        await this[_loadAncientPackageDetails](node)
       })
 
     return this[_handleOptionalFailure](node, p)
@@ -583,32 +580,6 @@ module.exports = cls => class Reifier extends cls {
       this.log.warn('deprecated', `${_id}: ${deprecated}`)
   }
 
-  async [_loadAncientPackageDetails] (node, forceReload = false) {
-    // If we're loading from a v1 lockfile, load details from the package.json
-    // that weren't recorded in the old format.
-    const {meta} = this.idealTree
-    const ancient = meta.ancientLockfile
-    const old = meta.loadedFromDisk && !(meta.originalLockfileVersion >= 2)
-
-    // already replaced with the manifest if it's truly ancient
-    if (node.path && (forceReload || (old && !ancient))) {
-      // XXX should have a shared location where package.json is read,
-      // so we don't ever read the same pj more than necessary.
-      let pkg
-      try {
-        pkg = await rpj(node.path + '/package.json')
-      } catch (err) {}
-
-      if (pkg) {
-        node.package.bin = pkg.bin
-        node.package.os = pkg.os
-        node.package.cpu = pkg.cpu
-        node.package.engines = pkg.engines
-        meta.add(node)
-      }
-    }
-  }
-
   // if the node is optional, then the failure of the promise is nonfatal
   // just add it and its optional set to the trash list.
   [_handleOptionalFailure] (node, p) {
@@ -1079,12 +1050,6 @@ module.exports = cls => class Reifier extends cls {
 
     const { meta } = this.idealTree
 
-    // might have to update metadata for bins and stuff that gets lost
-    if (meta.loadedFromDisk && !(meta.originalLockfileVersion >= 2)) {
-      for (const node of this.idealTree.inventory.values())
-        await this[_loadAncientPackageDetails](node, true)
-    }
-
     return meta.save(saveOpt)
   }