fix: saveSpec should follow symlinks

lib/arborist/reify.js

@@ -6,6 +6,7 @@ const rpj = require('read-package-json-fast')
 const {checkEngine, checkPlatform} = require('npm-install-checks')
 const updateDepSpec = require('../update-dep-spec.js')
 const AuditReport = require('../audit-report.js')
+const realpath = require('../realpath.js')
 
 const {dirname, resolve, relative} = require('path')
 const {depth: dfwalk} = require('treeverse')
@@ -75,6 +76,10 @@ const _omitPeer = Symbol('omitPeer')
 const _global = Symbol.for('global')
 const _scriptShell = Symbol('scriptShell')
 
+const _rpcache = Symbol('realpathCache')
+const _stcache = Symbol('statCache')
+const _replaceSymlinkRealpath = Symbol('replaceSymlinkRealpath')
+
 // defined by Ideal mixin
 const _force = Symbol.for('force')
 const _pruneBundledMetadeps = Symbol.for('pruneBundledMetadeps')
@@ -104,6 +109,12 @@ module.exports = cls => class Reifier extends cls {
     this[_retiredUnchanged] = {}
     this[_sparseTreeDirs] = new Set()
     this[_trashList] = new Set()
+
+    // caches for cached realpath calls
+    const cwd = process.cwd()
+    // assume that the cwd is real enough for our purposes
+    this[_rpcache] = new Map([[ cwd, cwd ]])
+    this[_stcache] = new Map()
   }
 
   // public method
@@ -809,7 +820,7 @@ module.exports = cls => class Reifier extends cls {
 
   // last but not least, we save the ideal tree metadata to the package-lock
   // or shrinkwrap file, and any additions or removals to package.json
-  [_saveIdealTree] (options) {
+  async [_saveIdealTree] (options) {
     // the ideal tree is actualized now, hooray!
     // it still contains all the references to optional nodes that were removed
     // for install failures.  Those still end up in the shrinkwrap, so we
@@ -829,6 +840,8 @@ module.exports = cls => class Reifier extends cls {
         const child = root.children.get(name)
         const res = npa(child.resolved)
 
+        await this[_replaceSymlinkRealpath](req)
+
         if (req.registry) {
           const version = child.version
           const range = this[_savePrefix] + version
@@ -862,10 +875,27 @@ module.exports = cls => class Reifier extends cls {
       .replace(/\n/g, eol)
 
     const saveOpt = { format: this[_formatPackageLock] }
-    return Promise.all([
+    await Promise.all([
       this[_usePackageLock] && this.idealTree.meta.save(saveOpt),
       writeFile(pj, json),
-    ]).then(() => process.emit('timeEnd', 'reify:save'))
+    ])
+
+    process.emit('timeEnd', 'reify:save')
+  }
+
+  async [_replaceSymlinkRealpath] (req) {
+    if (req.type === 'directory') {
+      // Follow symlinks realpaths and update saveSpec
+      // in order to store that on package.json
+      const child = this.idealTree.children.get(req.name)
+      const real = await (
+        realpath(child.path, this[_rpcache], this[_stcache])
+          .catch(() => null)
+      )
+
+      if (real)
+        req.saveSpec = `file:${relpath(this.path, real)}`
+    }
   }
 
   [_copyIdealToActual] () {

tap-snapshots/test-arborist-reify.js-TAP.test.js

@@ -27030,9 +27030,17 @@ Object {
       "resolved": "https://registry.npmjs.org/c/-/c-1.2.3.tgz",
       "version": "npm:[email protected]",
     },
+    "e": Object {
+      "extraneous": true,
+      "version": "1.0.0",
+    },
+    "f": Object {
+      "extraneous": true,
+      "version": "1.0.0",
+    },
   },
   "lockfileVersion": 2,
-  "name": "reify-saving-the-ideal-tree-save-some-stuff",
+  "name": "project",
   "packages": Object {
     "": Object {
       "bundleDependencies": Array [
@@ -27044,11 +27052,18 @@ Object {
         "a": "github:foo/bar#baz",
         "b": "^1.2.3",
         "d": "npm:c@^1.2.3",
+        "e": "file:../linked-pkg",
+        "f": "file:../foo",
       },
       "devDependencies": Object {
         "c": "git+ssh://[email protected]:a/b/c.git#master",
       },
     },
+    "../linked-pkg": Object {
+      "extraneous": true,
+      "name": "e",
+      "version": "1.0.0",
+    },
     "node_modules/a": Object {
       "extraneous": true,
       "inBundle": true,
@@ -27071,6 +27086,16 @@ Object {
       "resolved": "https://registry.npmjs.org/c/-/c-1.2.3.tgz",
       "version": "1.2.3",
     },
+    "node_modules/e": Object {
+      "extraneous": true,
+      "resolved": "file:node_modules/global-prefix/lib/node_modules/e",
+      "version": "1.0.0",
+    },
+    "node_modules/f": Object {
+      "extraneous": true,
+      "resolved": "file:node_modules/foo",
+      "version": "1.0.0",
+    },
   },
   "requires": true,
 }

test/arborist/reify.js

@@ -821,13 +821,14 @@ t.test('rollbacks', { buffered: false }, t => {
 
 t.test('saving the ideal tree', t => {
   const kSaveIdealTree = Symbol.for('saveIdealTree')
-  t.test('save=false', t => {
+  t.test('save=false', async t => {
     // doesn't actually do anything, just for coverage.
     // if it wasn't an early exit, it'd blow up and throw
     // an error though.
     const path = t.testdir()
     const a = newArb({ path })
-    t.notOk(a[kSaveIdealTree]({ save: false }))
+    const res = await a[kSaveIdealTree]({ save: false })
+    t.notOk(res)
     t.end()
   })
 
@@ -846,9 +847,30 @@ t.test('saving the ideal tree', t => {
 
     const npa = require('npm-package-arg')
     const kResolvedAdd = Symbol.for('resolvedAdd')
-    const path = t.testdir({
-      'package.json': JSON.stringify(pkg)
+    const testdir = t.testdir({
+      'global-prefix': {
+        lib: {
+          node_modules: {
+            'e': t.fixture('symlink', '../../../linked-pkg')
+          }
+        }
+      },
+      project: {
+        // have node_modules/e here to simulate reify having put
+        // symlinks in their right place so that realpath can follow them
+        node_modules: {
+          e: t.fixture('symlink', '../../global-prefix/lib/node_modules/e')
+        },
+        'package.json': JSON.stringify(pkg),
+      },
+      'linked-pkg': {
+        'package.json': JSON.stringify({
+          name: 'e',
+          version: '1.0.0'
+        })
+      }
     })
+    const path = resolve(testdir, 'project')
     const a = newArb({ path })
     const hash = '71f3ccfefba85d2048484569dba8c1829f6f41d7'
     return a.loadActual().then(tree => Shrinkwrap.load({path}).then(meta => {
@@ -887,11 +909,29 @@ t.test('saving the ideal tree', t => {
         },
         parent: tree,
       })
+      new Node({
+        name: 'e',
+        path: `${path}/project/node_modules/e`,
+        resolved: 'file:../global-prefix/lib/node_modules/e',
+        parent: tree,
+        pkg: { name: 'e', version: '1.0.0' },
+      })
+      // non-existent link, simulates the situation in which
+      // realpath can not properly follow the symlink
+      new Node({
+        name: 'f',
+        path: `${path}/foo`,
+        resolved: 'file:../foo',
+        parent: tree,
+        pkg: { name: 'f', version: '1.0.0' },
+      })
 
       a[kResolvedAdd] = [
         npa('a@git+ssh://[email protected]:foo/bar#baz'),
         npa('b'),
         npa('d@npm:[email protected]'),
+        npa('e@file:../global-prefix/lib/node_modules/e'),
+        npa('f@file:../foo'),
         npa(`c@git+ssh://[email protected]:a/b/c.git#master`),
       ]
       return a[kSaveIdealTree]({
@@ -905,6 +945,8 @@ t.test('saving the ideal tree', t => {
           a: 'github:foo/bar#baz',
           b: '^1.2.3',
           d: 'npm:c@^1.2.3',
+          e: 'file:../linked-pkg',
+          f: 'file:../foo',
         },
         devDependencies: {
           c: 'git+ssh://[email protected]:a/b/c.git#master',