diff --git a/smoke-tests/test/install-links.js b/smoke-tests/test/install-links.js new file mode 100644 index 0000000000000..4ae659f51ea66 --- /dev/null +++ b/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') +}) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index afcb67903da6d..6c4401f8fd2c8 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/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) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index c3cbf02b31080..526d84d2c2e4c 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/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), }) } } diff --git a/workspaces/arborist/lib/dep-valid.js b/workspaces/arborist/lib/dep-valid.js index 9c1bc7d3f23e9..4d2d9d0fe8d5f 100644 --- a/workspaces/arborist/lib/dep-valid.js +++ b/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) { diff --git a/workspaces/arborist/lib/diff.js b/workspaces/arborist/lib/diff.js index 0387773c29754..8a597f2c9e531 100644 --- a/workspaces/arborist/lib/diff.js +++ b/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