wip: treat links with installLinks set as Links not Nodes

smoke-tests/test/install-links.js

@@ -0,0 +1,52 @@
+const t = require('tap')
+const { join } = require('path')
+const setup = require('./fixtures/setup.js')
+
+t.test('workspaces + file deps + non-deduping abbrev', async t => {
+ const abbrevs = ['1.0.3', '1.0.4', '1.0.5', '1.0.6']
+
+ const { npm, registry, paths } = await setup(t, {
+ debug: true,
+ testdir: {
+ a: {
+ 'package.json': { name: 'a', dependencies: { abbrev: abbrevs[0], b: 'file:./b' } },
+ },
+ b: {
+ 'package.json': { name: 'b', dependencies: { abbrev: abbrevs[1] } },
+ },
+ packages: abbrevs.reduce((acc, v) => {
+ acc[`abbrev-${v}`] = { 'package.json': { name: 'abbrev', version: v } }
+ return acc
+ }, {}),
+ },
+ })
+
+ await npm('init', '-y')
+ await npm('init', '-y', '--workspace=workspaces/ws-a')
+ await npm('init', '-y', '--workspace=workspaces/ws-b')
+
+ // set root deps
+ await npm('pkg', 'set', `dependencies.ws-a=1.0.0`)
+ await npm('pkg', 'set', `dependencies.a=file:../a`)
+
+ // set workspace a deps
+ await npm('pkg', 'set', `dependencies.abbrev=${abbrevs[2]}`, '--workspace=ws-a')
+ await npm('pkg', 'set', `dependencies.ws-b=1.0.0`, '--workspace=ws-a')
+
+ // set workspace b deps
+ await npm('pkg', 'set', `dependencies.abbrev=${abbrevs[3]}`, '--workspace=ws-b')
+
+ await registry.package({
+ manifest: registry.manifest({ name: 'abbrev', versions: abbrevs }),
+ tarballs: abbrevs.reduce((acc, v) => {
+ acc[v] = join(paths.root, 'packages', `abbrev-${v}`)
+ return acc
+ }, {}),
+ times: 2,
+
+ })
+
+ // this should not fail and all 4 abbrevs should get installed
+ // with no mocks remaining
+ await npm('install', '--install-links=true')
+})

workspaces/arborist/lib/arborist/build-ideal-tree.js

@@ -28,6 +28,7 @@ const Shrinkwrap = require('../shrinkwrap.js')
 const { defaultLockfileVersion } = Shrinkwrap
 const Node = require('../node.js')
 const Link = require('../link.js')
+const Edge = require('../edge.js')
 const addRmPkgDeps = require('../add-rm-pkg-deps.js')
 const optionalSet = require('../optional-set.js')
 const { checkEngine, checkPlatform } = require('npm-install-checks')
@@ -1045,14 +1046,18 @@ This is a one-time fix-up, please be patient...
  // loads a node from an edge, and then loads its peer deps (and their
  // peer deps, on down the line) into a virtual root parent.
  async [_nodeFromEdge] (edge, parent_, secondEdge, required) {
+ let from = edge.from
+ if (from.installLinks && from.linksIn.size) {
+ from = from.linksIn.values().next().value
+ }
  // create a virtual root node with the same deps as the node that
  // is requesting this one, so that we can get all the peer deps in
  // a context where they're likely to be resolvable.
  // Note that the virtual root will also have virtual copies of the
  // targets of any child Links, so that they resolve appropriately.
- const parent = parent_ || this[_virtualRoot](edge.from)
+ const parent = parent_ || this[_virtualRoot](from)
 
- const spec = npa.resolve(edge.name, edge.spec, edge.from.path)
+ const spec = npa.resolve(edge.name, edge.spec, from.path)
  const first = await this[_nodeFromSpec](edge.name, spec, parent, edge)
 
  // we might have a case where the parent has a peer dependency on
@@ -1110,11 +1115,10 @@ This is a one-time fix-up, please be patient...
  legacyPeerDeps: this.legacyPeerDeps,
  overrides: node.overrides,
  })
-
  // also need to set up any targets from any link deps, so that
  // they are properly reflected in the virtual environment
  for (const child of node.children.values()) {
- if (child.isLink) {
+ if (child.isLink && (child.isWorkspace || !child.installLinks)) {
  new Node({
  path: child.realpath,
  sourceReference: child.target,
@@ -1228,8 +1232,8 @@ This is a one-time fix-up, please be patient...
  const isWorkspace = this.idealTree.workspaces && this.idealTree.workspaces.has(spec.name)
 
  // spec is a directory, link it unless installLinks is set or it's a workspace
- if (spec.type === 'directory' && (isWorkspace || !installLinks)) {
- return this[_linkFromSpec](name, spec, parent, edge)
+ if (spec.type === 'directory') {
+ return this[_linkFromSpec](name, spec, parent)
  }
 
  // if the spec matches a workspace name, then see if the workspace node will
@@ -1263,7 +1267,7 @@ This is a one-time fix-up, please be patient...
  })
  }
 
- [_linkFromSpec] (name, spec, parent, edge) {
+ [_linkFromSpec] (name, spec, parent) {
  const realpath = spec.fetchSpec
  const { installLinks, legacyPeerDeps } = this
  return rpj(realpath + '/package.json').catch(() => ({})).then(pkg => {
@@ -1410,6 +1414,29 @@ This is a one-time fix-up, please be patient...
  continue
  }
 
+ // XXX: maybe we need to replace the link with its target
+ // at this stage so it gets treated as a regular node at
+ // reified correctly inside node_modules?
+ // if (link.installLinks && !link.isWorkspace) {
+ // // console.log(link)edg
+ // // console.log(link.name)
+ // // console.log(link.parent)
+ // // console.log(link.path)
+ // // console.log(link.realpath)
+ // // console.log(link.pkg)
+ // const target = link.target
+ // link.replaceWith(target)
+ // this.addTracker('idealTree', target.name, target.location)
+ // this[_depsQueue].push(target)
+ // continue
+ // }
+ // if installLinks is set then we want to install deps no matter what
+ if (link.installLinks) {
+ this.addTracker('idealTree', link.target.name, link.target.location)
+ this[_depsQueue].push(link.target)
+ continue
+ }
+
  const tree = this.idealTree.target
  const external = !link.target.isDescendantOf(tree)
 

workspaces/arborist/lib/arborist/reify.js

@@ -668,7 +668,7 @@ module.exports = cls => class Reifier extends cls {
  const nm = resolve(node.parent.path, 'node_modules')
  await this[_validateNodeModules](nm)
 
- if (node.isLink) {
+ if (node.isLink && (node.isWorkspace || !node.installLinks)) {
  await rm(node.path, { recursive: true, force: true })
  await this[_symlink](node)
  } else {
@@ -687,6 +687,7 @@ module.exports = cls => class Reifier extends cls {
  Arborist: this.constructor,
  resolved: node.resolved,
  integrity: node.integrity,
+ where: dirname(node.path),
  })
  }
  }

workspaces/arborist/lib/dep-valid.js

@@ -108,6 +108,7 @@ const depValid = (child, requested, requestor) => {
 
 const linkValid = (child, requested, requestor) => {
  const isLink = !!child.isLink
+
  // if we're installing links and the node is a link, then it's invalid because we want
  // a real node to be there. Except for workspaces. They are always links.
  if (requestor.installLinks && !child.isWorkspace) {

workspaces/arborist/lib/diff.js

@@ -6,7 +6,7 @@
 // for a given branch of the tree being mutated.
 
 const { depth } = require('treeverse')
-const { existsSync } = require('fs')
+const { existsSync, lstatSync } = require('fs')
 
 const ssri = require('ssri')
 
@@ -114,6 +114,21 @@ const getAction = ({ actual, ideal }) => {
  return ideal.inDepBundle ? null : 'ADD'
  }
 
+ if (actual.isLink) {
+ let stat
+ try {
+ stat = lstatSync(actual.path)
+ } catch {}
+
+ if (stat) {
+ if (ideal.installLinks && stat.isSymbolicLink()) {
+ return 'CHANGE'
+ } else if (!ideal.installLinks && !stat.isSymbolicLink()) {
+ return 'CHANGE'
+ }
+ }
+ }
+
  // always ignore the root node
  if (ideal.isRoot && actual.isRoot) {
  return null