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

Commit

Permalink
fix: skip optional deps with mismatched platform or engine
Browse files Browse the repository at this point in the history
PR-URL: #231
Credit: @nlf
Close: #231
Reviewed-by: @isaacs
  • Loading branch information
nlf authored and isaacs committed Feb 18, 2021
1 parent a98d6fd commit 4a3322d
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 0 deletions.
13 changes: 13 additions & 0 deletions lib/arborist/reify.js
Expand Up @@ -16,6 +16,7 @@ const mkdirp = require('mkdirp-infer-owner')
const moveFile = require('@npmcli/move-file')
const rimraf = promisify(require('rimraf'))
const packageContents = require('@npmcli/installed-package-contents')
const { checkEngine, checkPlatform } = require('npm-install-checks')

const treeCheck = require('../tree-check.js')
const relpath = require('../relpath.js')
Expand Down Expand Up @@ -446,7 +447,19 @@ module.exports = cls => class Reifier extends cls {
process.emit('time', timer)
this.addTracker('reify', node.name, node.location)

const { npmVersion, nodeVersion } = this.options
const p = Promise.resolve()
.then(() => {
// when we reify an optional node, check the engine and platform
// first. be sure to ignore the --force and --engine-strict flags,
// since we always want to skip any optional packages we can't install.
// these checks throwing will result in a rollback and removal
// of the mismatches
if (node.optional) {
checkEngine(node.package, npmVersion, nodeVersion, false)
checkPlatform(node.package, false)
}
})
.then(() => this[_checkBins](node))
.then(() => this[_extractOrLink](node))
.then(() => this[_warnDeprecated](node))
Expand Down
36 changes: 36 additions & 0 deletions tap-snapshots/test-arborist-reify.js-TAP.test.js
Expand Up @@ -1018,6 +1018,24 @@ ArboristNode {
}
`

exports[`test/arborist/reify.js TAP do not install optional deps with mismatched platform specifications > expect resolving Promise 1`] = `
ArboristNode {
"edgesOut": Map {
"platform-specifying-test-package" => EdgeOut {
"name": "platform-specifying-test-package",
"spec": "1.0.0",
"to": null,
"type": "optional",
},
},
"location": "",
"name": "reify-do-not-install-optional-deps-with-mismatched-platform-specifications",
"packageName": "platform-test",
"path": "{CWD}/test/arborist/reify-do-not-install-optional-deps-with-mismatched-platform-specifications",
"version": "1.0.0",
}
`

exports[`test/arborist/reify.js TAP do not update shrinkwrapped deps > expect resolving Promise 1`] = `
ArboristNode {
"children": Map {
Expand Down Expand Up @@ -29410,6 +29428,24 @@ exports[`test/arborist/reify.js TAP scoped registries > should preserve original
@ruyadorno/theoretically-private-pkg@https://npm.pkg.github.com/@ruyadorno/theoretically-private-pkg/-/theoretically-private-pkg-1.2.3.tgz
`

exports[`test/arborist/reify.js TAP still do not install optional deps with mismatched platform specifications even when forced > expect resolving Promise 1`] = `
ArboristNode {
"edgesOut": Map {
"platform-specifying-test-package" => EdgeOut {
"name": "platform-specifying-test-package",
"spec": "1.0.0",
"to": null,
"type": "optional",
},
},
"location": "",
"name": "reify-still-do-not-install-optional-deps-with-mismatched-platform-specifications-even-when-forced",
"packageName": "platform-test",
"path": "{CWD}/test/arborist/reify-still-do-not-install-optional-deps-with-mismatched-platform-specifications-even-when-forced",
"version": "1.0.0",
}
`

exports[`test/arborist/reify.js TAP store files with a custom indenting > must match snapshot 1`] = `
{
"name": "tab-indented-package-json",
Expand Down
11 changes: 11 additions & 0 deletions test/arborist/reify.js
Expand Up @@ -363,6 +363,17 @@ t.test('do not update shrinkwrapped deps', t =>
fixture(t, 'shrinkwrapped-dep-with-lock'),
{ update: { names: ['abbrev']}})))

t.test('do not install optional deps with mismatched platform specifications', t =>
t.resolveMatchSnapshot(printReified(
fixture(t, 'optional-platform-specification'))))

t.test('still do not install optional deps with mismatched platform specifications even when forced', t =>
t.resolveMatchSnapshot(printReified(
fixture(t, 'optional-platform-specification'), { force: true })))

t.test('fail to install deps with mismatched platform specifications', t =>
t.rejects(printReified(fixture(t, 'platform-specification')), { code: 'EBADPLATFORM' }))

t.test('dry run, do not get anything wet', async t => {
const cases = [
'shrinkwrapped-dep-with-lock-empty',
Expand Down

0 comments on commit 4a3322d

Please sign in to comment.