Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
properly buildIdealTree when root is a symlink
Browse files Browse the repository at this point in the history
  • Loading branch information
@isaacs @nlf
isaacs authored and nlf committed Jan 26, 2021
1 parent 7169e03 commit 0b86538
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 16 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ scripts/benchmark/*/
/test/fixtures/selflink/node_modules/foo/node_modules/selflink
/test/fixtures/symlinked-node-modules/example/node_modules
/test/fixtures/symlinked-node-modules/linked-node-modules/bar
/test/fixtures/testing-peer-deps-link
/test/fixtures/workspace/node_modules/a
/test/fixtures/workspace/node_modules/b
/test/fixtures/workspace/node_modules/c
Expand Down
54 changes: 40 additions & 14 deletions lib/arborist/build-ideal-tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,20 +332,26 @@ module.exports = cls => class IdealTreeBuilder extends cls {
})
}

[_globalRootNode] () {
const root = this[_rootNodeFromPackage]({ dependencies: {} })
async [_globalRootNode] () {
const root = await this[_rootNodeFromPackage]({ dependencies: {} })
// this is a gross kludge to handle the fact that we don't save
// metadata on the root node in global installs, because the "root"
// node is something like /usr/local/lib.
const meta = new Shrinkwrap({ path: this.path })
meta.reset()
root.meta = meta
return Promise.resolve(root)
return root
}

[_rootNodeFromPackage] (pkg) {
return new Node({
async [_rootNodeFromPackage] (pkg) {
// if the path doesn't exist, then assume that it's going to be
// real once we reify it, since we'll create a folder, but not
// a symlink to a folder, by default.
const real = await realpath(this.path, this[_rpcache], this[_stcache])
const Cls = real === this.path ? Node : Link
const root = new Cls({
path: this.path,
realpath: real,
pkg,
extraneous: false,
dev: false,
Expand All @@ -355,12 +361,29 @@ module.exports = cls => class IdealTreeBuilder extends cls {
global: this[_global],
legacyPeerDeps: this.legacyPeerDeps,
})
if (root.isLink) {
root.target = new Node({
path: real,
realpath: real,
pkg,
extraneous: false,
dev: false,
devOptional: false,
peer: false,
optional: false,
global: this[_global],
legacyPeerDeps: this.legacyPeerDeps,
root,
})
}
return root
}

// process the add/rm requests by modifying the root node, and the
// update.names request by queueing nodes dependent on those named.
async [_applyUserRequests] (options) {
process.emit('time', 'idealTree:userRequests')
const tree = this.idealTree.target || this.idealTree
// If we have a list of package names to update, and we know it's
// going to update them wherever they are, add any paths into those
// named nodes to the buildIdealTree queue.
Expand All @@ -373,15 +396,15 @@ module.exports = cls => class IdealTreeBuilder extends cls {
const nm = resolve(this.path, 'node_modules')
for (const name of await readdir(nm)) {
if (this[_updateAll] || this[_updateNames].includes(name))
this.idealTree.package.dependencies[name] = '*'
tree.package.dependencies[name] = '*'
}
}

if (this.auditReport && this.auditReport.size > 0)
this[_queueVulnDependents](options)

if (options.rm && options.rm.length) {
addRmPkgDeps.rm(this.idealTree.package, options.rm)
addRmPkgDeps.rm(tree.package, options.rm)
for (const name of options.rm)
this[_explicitRequests].add(name)
}
Expand All @@ -391,7 +414,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {

// triggers a refresh of all edgesOut
if (options.add && options.add.length || options.rm && options.rm.length)
this.idealTree.package = this.idealTree.package
tree.package = tree.package
process.emit('timeEnd', 'idealTree:userRequests')
}

Expand All @@ -410,8 +433,9 @@ module.exports = cls => class IdealTreeBuilder extends cls {
this[_resolvedAdd] = add
// now add is a list of spec objects with names.
// find a home for each of them!
const tree = this.idealTree.target || this.idealTree
addRmPkgDeps.add({
pkg: this.idealTree.package,
pkg: tree.package,
add,
saveBundle,
saveType,
Expand Down Expand Up @@ -514,7 +538,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
fixAvailable,
} = topVuln
for (const node of topNodes) {
if (node !== this.idealTree) {
if (node !== this.idealTree && node !== this.idealTree.target) {
// not something we're going to fix, sorry. have to cd into
// that directory and fix it yourself.
this.log.warn('audit', 'Manual fix required in linked project ' +
Expand Down Expand Up @@ -646,11 +670,12 @@ This is a one-time fix-up, please be patient...
// at this point we have a virtual tree with the actual root node's
// package deps, which may be partly or entirely incomplete, invalid
// or extraneous.
[_buildDeps] (node) {
[_buildDeps] () {
process.emit('time', 'idealTree:buildDeps')
this[_depsQueue].push(this.idealTree)
const tree = this.idealTree.target || this.idealTree
this[_depsQueue].push(tree)
this.log.silly('idealTree', 'buildDeps')
this.addTracker('idealTree', this.idealTree.name, '')
this.addTracker('idealTree', tree.name, '')
return this[_buildDepStep]()
.then(() => process.emit('timeEnd', 'idealTree:buildDeps'))
}
Expand Down Expand Up @@ -1137,7 +1162,8 @@ This is a one-time fix-up, please be patient...

// when installing globally, or just in global style, we never place
// deps above the first level.
if (this[_globalStyle] && check.resolveParent === this.idealTree)
const tree = this.idealTree && this.idealTree.target || this.idealTree
if (this[_globalStyle] && check.resolveParent === tree)
break
}

Expand Down
5 changes: 3 additions & 2 deletions lib/arborist/reify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,9 @@ module.exports = cls => class Reifier extends cls {
const actualOpt = this[_global] ? {
ignoreMissing: true,
global: true,
filter: (node, kid) => !node.isRoot ? true
: (node.edgesOut.has(kid) || this[_explicitRequests].has(kid)),
filter: (node, kid) => !node.isRoot && node !== node.root.target
? true
: (node.edgesOut.has(kid) || this[_explicitRequests].has(kid)),
} : { ignoreMissing: true }

if (!this[_global]) {
Expand Down
162 changes: 162 additions & 0 deletions tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123635,6 +123635,168 @@ ArboristNode {
}
`

exports[`test/arborist/build-ideal-tree.js TAP testing-peer-deps package with symlinked root > build ideal tree with peer deps 1`] = `
ArboristLink {
"location": "../testing-peer-deps-link",
"name": "testing-peer-deps-link",
"packageName": "@isaacs/testing-peer-deps",
"path": "{CWD}/test/fixtures/testing-peer-deps-link",
"realpath": "{CWD}/test/fixtures/testing-peer-deps",
"resolved": "file:testing-peer-deps",
"target": ArboristNode {
"children": Map {
"@isaacs/testing-peer-deps-b" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "@isaacs/testing-peer-deps-b",
"spec": "1",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-peer-deps-c" => EdgeOut {
"name": "@isaacs/testing-peer-deps-c",
"spec": "1",
"to": "node_modules/@isaacs/testing-peer-deps-c",
"type": "peer",
},
},
"location": "node_modules/@isaacs/testing-peer-deps-b",
"name": "@isaacs/testing-peer-deps-b",
"path": "{CWD}/test/fixtures/testing-peer-deps/node_modules/@isaacs/testing-peer-deps-b",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-b/-/testing-peer-deps-b-1.2.4.tgz",
"version": "1.2.4",
},
"@isaacs/testing-peer-deps-c" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-peer-deps-b",
"name": "@isaacs/testing-peer-deps-c",
"spec": "1",
"type": "peer",
},
},
"location": "node_modules/@isaacs/testing-peer-deps-c",
"name": "@isaacs/testing-peer-deps-c",
"path": "{CWD}/test/fixtures/testing-peer-deps/node_modules/@isaacs/testing-peer-deps-c",
"peer": true,
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-c/-/testing-peer-deps-c-1.2.3.tgz",
"version": "1.2.3",
},
"@isaacs/testing-peer-deps-d" => ArboristNode {
"children": Map {
"@isaacs/testing-peer-deps-a" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-peer-deps-d",
"name": "@isaacs/testing-peer-deps-a",
"spec": "2",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-peer-deps-b" => EdgeOut {
"name": "@isaacs/testing-peer-deps-b",
"spec": "2",
"to": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-b",
"type": "peer",
},
},
"location": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-a",
"name": "@isaacs/testing-peer-deps-a",
"path": "{CWD}/test/fixtures/testing-peer-deps/node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-a",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-a/-/testing-peer-deps-a-2.0.0.tgz",
"version": "2.0.0",
},
"@isaacs/testing-peer-deps-b" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-a",
"name": "@isaacs/testing-peer-deps-b",
"spec": "2",
"type": "peer",
},
},
"edgesOut": Map {
"@isaacs/testing-peer-deps-c" => EdgeOut {
"name": "@isaacs/testing-peer-deps-c",
"spec": "2",
"to": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-c",
"type": "peer",
},
},
"location": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-b",
"name": "@isaacs/testing-peer-deps-b",
"path": "{CWD}/test/fixtures/testing-peer-deps/node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-b",
"peer": true,
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-b/-/testing-peer-deps-b-2.0.1.tgz",
"version": "2.0.1",
},
"@isaacs/testing-peer-deps-c" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-b",
"name": "@isaacs/testing-peer-deps-c",
"spec": "2",
"type": "peer",
},
},
"location": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-c",
"name": "@isaacs/testing-peer-deps-c",
"path": "{CWD}/test/fixtures/testing-peer-deps/node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-c",
"peer": true,
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-c/-/testing-peer-deps-c-2.0.0.tgz",
"version": "2.0.0",
},
},
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "@isaacs/testing-peer-deps-d",
"spec": "2",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-peer-deps-a" => EdgeOut {
"name": "@isaacs/testing-peer-deps-a",
"spec": "2",
"to": "node_modules/@isaacs/testing-peer-deps-d/node_modules/@isaacs/testing-peer-deps-a",
"type": "prod",
},
},
"location": "node_modules/@isaacs/testing-peer-deps-d",
"name": "@isaacs/testing-peer-deps-d",
"path": "{CWD}/test/fixtures/testing-peer-deps/node_modules/@isaacs/testing-peer-deps-d",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-d/-/testing-peer-deps-d-2.0.0.tgz",
"version": "2.0.0",
},
},
"edgesOut": Map {
"@isaacs/testing-peer-deps-b" => EdgeOut {
"name": "@isaacs/testing-peer-deps-b",
"spec": "1",
"to": "node_modules/@isaacs/testing-peer-deps-b",
"type": "prod",
},
"@isaacs/testing-peer-deps-d" => EdgeOut {
"name": "@isaacs/testing-peer-deps-d",
"spec": "2",
"to": "node_modules/@isaacs/testing-peer-deps-d",
"type": "prod",
},
},
"location": "",
"name": "testing-peer-deps",
"packageName": "@isaacs/testing-peer-deps",
"path": "{CWD}/test/fixtures/testing-peer-deps",
"version": "2.0.0",
},
"version": "2.0.0",
}
`

exports[`test/arborist/build-ideal-tree.js TAP update flow outdated > update everything 1`] = `
ArboristNode {
"children": Map {
Expand Down
10 changes: 10 additions & 0 deletions test/arborist/build-ideal-tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,16 @@ t.test('testing-peer-deps package', t => {
.then(() => t.matchSnapshot(printTree(idealTree), 'build ideal tree with peer deps')))
})

t.test('testing-peer-deps package with symlinked root', t => {
const path = resolve(fixtures, 'testing-peer-deps-link')
return buildIdeal(path).then(idealTree => {
t.ok(idealTree.isLink, 'ideal tree is rooted on a Link')
return new Arborist({path, idealTree, ...OPT})
.buildIdealTree().then(tree2 => t.equal(tree2, idealTree))
.then(() => t.matchSnapshot(printTree(idealTree), 'build ideal tree with peer deps'))
})
})

t.test('testing-peer-deps nested', t => {
const path = resolve(fixtures, 'testing-peer-deps-nested')
return t.resolveMatchSnapshot(printIdeal(path), 'build ideal tree')
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const symlinks = {
'deeproot': 'deep',
'badlink/node_modules/foo': 'foo',
'badlink/node_modules/bar': 'baz',
'testing-peer-deps-link': 'testing-peer-deps',

'workspace/node_modules/a': '../packages/a',
'workspace/node_modules/b': '../packages/b',
Expand Down

0 comments on commit 0b86538

Please sign in to comment.