diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 0e98ed6fc533c..945bae56b63de 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -1076,9 +1076,8 @@ This is a one-time fix-up, please be patient... // if it fails at this point, though, dont' worry because it // may well be an optional dep that has gone missing. it'll // fail later anyway. - const from = fromPath(placed) promises.push(...this[_problemEdges](placed).map(e => - this[_fetchManifest](npa.resolve(e.name, e.spec, from)) + this[_fetchManifest](npa.resolve(e.name, e.spec, fromPath(placed, e))) .catch(er => null))) }, }) diff --git a/workspaces/arborist/lib/dep-valid.js b/workspaces/arborist/lib/dep-valid.js index c69ab557ae491..d02f397fcdf17 100644 --- a/workspaces/arborist/lib/dep-valid.js +++ b/workspaces/arborist/lib/dep-valid.js @@ -20,7 +20,7 @@ const depValid = (child, requested, requestor) => { // file: deps that depend on other files/dirs, we must resolve the // location based on the *requestor* file/dir, not where it ends up. // '' is equivalent to '*' - requested = npa.resolve(child.name, requested || '*', fromPath(requestor)) + requested = npa.resolve(child.name, requested || '*', fromPath(requestor, requestor.edgesOut.get(child.name))) } catch (er) { // Not invalid because the child doesn't match, but because // the spec itself is not supported. Nothing would match, diff --git a/workspaces/arborist/lib/from-path.js b/workspaces/arborist/lib/from-path.js index 2a3617844c001..1006f73af3d07 100644 --- a/workspaces/arborist/lib/from-path.js +++ b/workspaces/arborist/lib/from-path.js @@ -6,8 +6,19 @@ const { dirname } = require('path') const npa = require('npm-package-arg') -const fromPath = (node, spec) => - spec && spec.type === 'file' ? dirname(spec.fetchSpec) - : node.realpath +const fromPath = (node, spec, edge) => { + if (edge && edge.overrides && edge.overrides.name === edge.name && edge.overrides.value) { + // fromPath could be called with a node that has a virtual root, if that happens + // we want to make sure we get the real root node when overrides are in use. this + // is to allow things like overriding a dependency with a tarball file that's a + // relative path from the project root + return node.sourceReference + ? node.sourceReference.root.realpath + : node.root.realpath + } -module.exports = node => fromPath(node, node.resolved && npa(node.resolved)) + return spec && spec.type === 'file' ? dirname(spec.fetchSpec) + : node.realpath +} + +module.exports = (node, edge) => fromPath(node, node.resolved && npa(node.resolved), edge) diff --git a/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs b/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs index 6feae28b570d6..17dc0b0c9fb0b 100644 --- a/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs @@ -88,6 +88,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "invalid": false, @@ -188,6 +209,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "invalid": true, @@ -345,6 +387,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "resolve": Function resolve(n), @@ -414,6 +477,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "resolve": Function resolve(n), @@ -614,6 +698,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "resolve": Function resolve(n), @@ -697,6 +802,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "a" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "type": "peer", @@ -748,6 +874,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "invalid": false, @@ -814,6 +961,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "resolve": Function resolve(n), @@ -868,6 +1036,27 @@ Edge { "version": "1.2.3", }, "resolve": Function resolve(n), + "root": Object { + "addEdgeIn": Function addEdgeIn(edge), + "addEdgeOut": Function addEdgeOut(edge), + "edgesIn": Set {}, + "edgesOut": Map { + "missing" => Edge { + "peerConflicted": false, + }, + }, + "explain": Function explain(), + "isTop": true, + "name": "top", + "package": Object { + "name": "top", + "version": "1.2.3", + }, + "packageName": "top", + "parent": null, + "resolve": Function resolve(n), + "version": "1.2.3", + }, "version": "1.2.3", }, "invalid": false, diff --git a/workspaces/arborist/test/dep-valid.js b/workspaces/arborist/test/dep-valid.js index 901325b2390b9..8c0e761658873 100644 --- a/workspaces/arborist/test/dep-valid.js +++ b/workspaces/arborist/test/dep-valid.js @@ -4,7 +4,12 @@ const npa = require('npm-package-arg') const { normalizePaths } = require('./fixtures/utils.js') const { resolve } = require('path') -t.ok(depValid({}, '', null, {}), '* is always ok') +// dep-valid reads from requestor.edgesOut so we use this instead of {} in these tests +const emptyRequestor = { + edgesOut: new Map(), +} + +t.ok(depValid({}, '', null, emptyRequestor), '* is always ok') t.ok(depValid({ package: { @@ -13,7 +18,7 @@ t.ok(depValid({ get version () { return this.package.version }, -}, '1.x', null, {}), 'range that is satisfied') +}, '1.x', null, emptyRequestor), 'range that is satisfied') t.ok(depValid({ package: { @@ -22,21 +27,21 @@ t.ok(depValid({ get version () { return this.package.version }, -}, '1.x', '2.x', {}), 'range that is acceptable') +}, '1.x', '2.x', emptyRequestor), 'range that is acceptable') t.ok(depValid({ isLink: true, realpath: '/some/path', -}, normalizePaths(npa('file:/some/path')), null, {}), 'links must point at intended target') +}, normalizePaths(npa('file:/some/path')), null, emptyRequestor), 'links must point at intended target') t.notOk(depValid({ isLink: true, realpath: '/some/other/path', -}, 'file:/some/path', null, {}), 'links must point at intended target') +}, 'file:/some/path', null, emptyRequestor), 'links must point at intended target') t.notOk(depValid({ realpath: '/some/path', -}, 'file:/some/path', null, {}), 'file:// must be a link') +}, 'file:/some/path', null, emptyRequestor), 'file:// must be a link') t.ok(depValid({ name: 'foo', @@ -47,7 +52,7 @@ t.ok(depValid({ get version () { return this.package.version }, -}, 'git://host/repo#semver:1.x', null, {}), 'git url with semver range') +}, 'git://host/repo#semver:1.x', null, emptyRequestor), 'git url with semver range') t.ok(depValid({ name: 'foo', @@ -58,7 +63,7 @@ t.ok(depValid({ get version () { return this.package.version }, -}, 'npm:bar@1.2.3', null, {}), 'alias is ok') +}, 'npm:bar@1.2.3', null, emptyRequestor), 'alias is ok') t.ok(depValid({ resolved: 'https://registry/abbrev-1.1.1.tgz', @@ -66,7 +71,7 @@ t.ok(depValid({ get version () { return this.package.version }, -}, 'https://registry/abbrev-1.1.1.tgz', null, {}), 'remote url match') +}, 'https://registry/abbrev-1.1.1.tgz', null, emptyRequestor), 'remote url match') t.ok(depValid({ resolved: 'git+ssh://git@github.com/foo/bar', @@ -74,7 +79,7 @@ t.ok(depValid({ get version () { return this.package.version }, -}, 'git+ssh://git@github.com/foo/bar.git', null, {}), 'matching _from saveSpec') +}, 'git+ssh://git@github.com/foo/bar.git', null, emptyRequestor), 'matching _from saveSpec') t.notOk(depValid({ resolved: 'git+ssh://git@github.com/foo/bar', @@ -82,26 +87,26 @@ t.notOk(depValid({ get version () { return this.package.version }, -}, 'git+ssh://git@github.com/bar/foo.git', null, {}), 'different repo') +}, 'git+ssh://git@github.com/bar/foo.git', null, emptyRequestor), 'different repo') t.notOk(depValid({ package: {}, get version () { return this.package.version }, -}, 'git+ssh://git@github.com/bar/foo.git', null, {}), 'missing repo') +}, 'git+ssh://git@github.com/bar/foo.git', null, emptyRequestor), 'missing repo') t.ok(depValid({ resolved: `file:${resolve('/path/to/tarball.tgz')}`, -}, resolve('/path/to/tarball.tgz'), null, {}), 'same tarball') +}, resolve('/path/to/tarball.tgz'), null, emptyRequestor), 'same tarball') t.notOk(depValid({ resolved: 'file:/path/to/other/tarball.tgz', -}, '/path/to/tarball.tgz', null, {}), 'different tarball') +}, '/path/to/tarball.tgz', null, emptyRequestor), 'different tarball') t.notOk(depValid({ isLink: true, -}, '/path/to/tarball.tgz', null, {}), 'links are not tarballs') +}, '/path/to/tarball.tgz', null, emptyRequestor), 'links are not tarballs') t.ok(depValid({ package: { @@ -112,28 +117,28 @@ t.ok(depValid({ get version () { return this.package.version }, -}, './tarball.tgz', null, {}), 'probably the same-ish, hopefully') +}, './tarball.tgz', null, emptyRequestor), 'probably the same-ish, hopefully') t.notOk(depValid({ package: {}, get version () { return this.package.version }, -}, './tarball.tgz', null, {}), 'too uncertain, nope') +}, './tarball.tgz', null, emptyRequestor), 'too uncertain, nope') t.ok(depValid({ resolved: 'https://registry.npmjs.org/foo/foo-1.2.3.tgz', -}, 'latest', null, {}), 'tagged registry version needs remote tarball') +}, 'latest', null, emptyRequestor), 'tagged registry version needs remote tarball') t.notOk(depValid({ resolved: 'git+https://registry.npmjs.org/foo/foo-1.2.3.git', -}, 'latest', null, {}), 'tagged registry version needs remote tarball, not git') +}, 'latest', null, emptyRequestor), 'tagged registry version needs remote tarball, not git') -t.notOk(depValid({}, 'latest', null, {}), +t.notOk(depValid({}, 'latest', null, emptyRequestor), 'tagged registry version needs remote tarball resolution') t.test('unsupported dependency type', t => { - const requestor = { errors: [] } + const requestor = { errors: [], edgesOut: new Map() } const child = { name: 'kid' } const request = { type: 'not a type' } t.notOk(depValid(child, request, null, requestor)) @@ -148,7 +153,7 @@ t.test('unsupported dependency type', t => { }) t.test('invalid tag name', t => { - const requestor = { errors: [] } + const requestor = { errors: [], edgesOut: new Map() } const child = { name: 'kid' } const request = '!!@#$%!#@$!' t.notOk(depValid(child, request, null, requestor)) @@ -163,7 +168,7 @@ t.test('invalid tag name', t => { }) t.test('invalid request all together', t => { - const requestor = { errors: [] } + const requestor = { errors: [], edgesOut: new Map() } const child = { name: 'kid' } const request = null t.notOk(depValid(child, request, null, requestor)) @@ -178,7 +183,7 @@ t.test('invalid request all together', t => { }) t.test('installLinks makes Link nodes invalid', t => { - const requestor = { errors: [], installLinks: true } + const requestor = { errors: [], installLinks: true, edgesOut: new Map() } const child = { isLink: true, name: 'kid' } const request = { type: 'directory' } t.notOk(depValid(child, request, null, requestor)) diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index 9d1352351741e..f45cb87917702 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -71,6 +71,7 @@ const a = { }, isTop: false, parent: top, + root: top, resolve (n) { return n === 'aa' ? aa : this.parent.resolve(n) }, @@ -1008,6 +1009,7 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) return this.package.version }, parent: root, + root, resolve (n) { return n === 'bar' ? bar : this.parent.resolve(n) }, @@ -1036,6 +1038,7 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) return this.package.version }, parent: foo, + root, resolve (n) { return this.parent.resolve(n) }, diff --git a/workspaces/arborist/test/from-path.js b/workspaces/arborist/test/from-path.js index 5b5ea2084c498..3bfd0b6398a74 100644 --- a/workspaces/arborist/test/from-path.js +++ b/workspaces/arborist/test/from-path.js @@ -1,7 +1,7 @@ const t = require('tap') const { normalizePath } = require('./fixtures/utils.js') const fp = require('../lib/from-path.js') -const fromPath = obj => normalizePath(fp(obj)) +const fromPath = (node, edge) => normalizePath(fp(node, edge)) t.equal(fromPath({ realpath: '/some/path', @@ -21,3 +21,30 @@ t.equal(fromPath({ realpath: '/some/path/to/install/target', resolved: 'https://registry.com/package.tgz', }), '/some/path/to/install/target', 'use dirname if not dir or file type') + +t.equal(fromPath({ + root: { + realpath: '/some/root', + }, +}, { + name: 'foo', + overrides: { + name: 'foo', + value: 'foo@2', + }, +}), '/some/root', 'uses root realpath for overridden edges') + +t.equal(fromPath({ + sourceReference: { + root: { + realpath: '/some/root', + }, + }, + realpath: '/some/red/herring', +}, { + name: 'foo', + overrides: { + name: 'foo', + value: 'foo@2', + }, +}), '/some/root', 'uses sourceReferences root realpath for overridden edges')