fix: load global symlinks implicitly as file: deps

When running `loadActual` or `buildIdealTree()` in the global root, we
read all or some of the top-level installed packages, and create a
"pseudo-package" for the root node which depends on all of them with the
range '*'.

However, for *linked* packages, this is not quite accurate.  In that
case, we really do not want to change them by accident to anything other
than the actual symlink we put there.

Detect this state, and assign the dependency properly in the root level.

Note that this does _not_ fix npm/cli#3457,
but it did contribute to attempting to ending up with a Node rather than
a Link in the top-level of the global install space.  With this patch in
place, the condition described will result in an `ERESOLVE` error rather
than destroying the link target, but that is still arguably preferrable.

Related-to: npm/cli#3457

lib/arborist/build-ideal-tree.js

@@ -9,6 +9,9 @@ const { resolve, dirname } = require('path')
 const { promisify } = require('util')
 const treeCheck = require('../tree-check.js')
 const readdir = promisify(require('readdir-scoped-modules'))
+const fs = require('fs')
+const lstat = promisify(fs.lstat)
+const readlink = promisify(fs.readlink)
 const { depth } = require('treeverse')
 
 const {
@@ -407,7 +410,14 @@ module.exports = cls => class IdealTreeBuilder extends cls {
         if (this[_updateAll] || updateName) {
           if (updateName)
             globalExplicitUpdateNames.push(name)
-          tree.package.dependencies[name] = '*'
+          const dir = resolve(nm, name)
+          const st = await lstat(dir).catch(/* istanbul ignore next */ er => null)
+          if (st && st.isSymbolicLink()) {
+            const target = await readlink(dir)
+            const real = resolve(dirname(dir), target)
+            tree.package.dependencies[name] = `file:${real}`
+          } else
+            tree.package.dependencies[name] = '*'
         }
       }
     }

lib/arborist/load-actual.js

@@ -188,8 +188,10 @@ module.exports = cls => class ActualLoader extends cls {
       const tree = this[_actualTree]
       const actualRoot = tree.isLink ? tree.target : tree
       const { dependencies = {} } = actualRoot.package
-      for (const name of actualRoot.children.keys())
-        dependencies[name] = dependencies[name] || '*'
+      for (const [name, kid] of actualRoot.children.entries()) {
+        const def = kid.isLink ? `file:${kid.realpath}` : '*'
+        dependencies[name] = dependencies[name] || def
+      }
       actualRoot.package = { ...actualRoot.package, dependencies }
     }
     return this[_actualTree]

tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs

@@ -118976,6 +118976,73 @@ ArboristNode {
 }
 `
 
+exports[`test/arborist/build-ideal-tree.js TAP update a global space that contains a link > must match snapshot 1`] = `
+ArboristNode {
+  "children": Map {
+    "abbrev" => ArboristNode {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "abbrev",
+          "spec": "*",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/abbrev",
+      "name": "abbrev",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link/node_modules/abbrev",
+      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+      "version": "1.1.1",
+    },
+    "once" => ArboristLink {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "once",
+          "spec": "file:{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link/target",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/once",
+      "name": "once",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link/node_modules/once",
+      "realpath": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link/target",
+      "resolved": "file:../target",
+      "target": ArboristNode {
+        "location": "target",
+      },
+      "version": "1.0.0-foo",
+    },
+  },
+  "edgesOut": Map {
+    "abbrev" => EdgeOut {
+      "name": "abbrev",
+      "spec": "*",
+      "to": "node_modules/abbrev",
+      "type": "prod",
+    },
+    "once" => EdgeOut {
+      "name": "once",
+      "spec": "file:{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link/target",
+      "to": "node_modules/once",
+      "type": "prod",
+    },
+  },
+  "fsChildren": Set {
+    ArboristNode {
+      "location": "target",
+      "name": "once",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link/target",
+      "version": "1.0.0-foo",
+    },
+  },
+  "isProjectRoot": true,
+  "location": "",
+  "name": "tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link",
+  "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-a-global-space-that-contains-a-link",
+}
+`
+
 exports[`test/arborist/build-ideal-tree.js TAP update flow outdated > update everything 1`] = `
 ArboristNode {
   "children": Map {

test/arborist/build-ideal-tree.js

@@ -2623,3 +2623,26 @@ t.test('inflates old lockfile with hasInstallScript', async t => {
 
   t.equal(tree.children.get('esbuild').hasInstallScript, true)
 })
+
+t.test('update a global space that contains a link', async t => {
+  const path = t.testdir({
+    target: {
+      'package.json': JSON.stringify({
+        name: 'once',
+        version: '1.0.0-foo',
+      }),
+    },
+    node_modules: {
+      abbrev: {
+        'package.json': JSON.stringify({
+          name: 'abbrev',
+          version: '1.0.0',
+        }),
+      },
+      once: t.fixture('symlink', '../target'),
+    },
+  })
+  const tree = await buildIdeal(path, { update: true, global: true })
+  t.matchSnapshot(printTree(tree))
+  t.equal(tree.children.get('once').isLink, true)
+})

test/arborist/load-actual.js

@@ -390,3 +390,30 @@ t.test('recalc dep flags for virtual load actual', async t => {
   const abbrev = tree.children.get('abbrev')
   t.equal(abbrev.extraneous, true, 'abbrev is extraneous')
 })
+
+t.test('load global space with link deps', async t => {
+  const path = t.testdir({
+    target: {
+      'package.json': JSON.stringify({
+        name: 'foo',
+        version: '1.2.3',
+      }),
+    },
+    node_modules: {
+      foo: t.fixture('symlink', '../target'),
+      bar: {
+        'package.json': JSON.stringify({
+          name: 'bar',
+          version: '2.3.4',
+        }),
+      },
+    },
+  })
+  const tree = await loadActual(path, { global: true })
+  t.strictSame(tree.package, {
+    dependencies: {
+      foo: `file:${resolve(path, 'target')}`,
+      bar: '*',
+    },
+  })
+})