skip optional deps with mismatched platform

lib/arborist/reify.js

@@ -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')
@@ -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))

notes/optional-deps.md

@@ -10,7 +10,8 @@ Types of failure:
 2. Optional dependency tgz download failure `optional-dep-tgz-missing`
 3. Optional dependency version not satisfiable `optional-dep-enotarget`
 4. Optional dependency build failure `optional-dep-postinstall-fail`
-5. Optional dependency depends on any failure above
+5. Optional dependency specifies unsupported engine or platform `optional-platform-specification`
+6. Optional dependency depends on any failure above
       1. `optional-metadep-missing`
       2. `optional-metadep-tgz-missing`
       3. `optional-metadep-enotarget`
@@ -20,7 +21,7 @@ These failures can occur in one of three places:
 
 1. buildIdealTree (unresolveable or failed metadata optional dep/meta-dep)
 
-2. reifyNode (failure to download/unpack tgz)
+2. reifyNode (failure to download/unpack tgz, engine/platform unsupported)
 
 3. lifecycle scripts
 

tap-snapshots/test-arborist-reify.js-TAP.test.js

@@ -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 {
@@ -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",

test/arborist/build-ideal-tree.js

@@ -83,7 +83,7 @@ t.test('warn on mismatched engine when engineStrict is false', t => {
   ]))
 })
 
-t.test('fail on mismatched platform', { skip: process.platform === 'win32' && '*nix specific test' }, async t => {
+t.test('fail on mismatched platform', async t => {
   const path = resolve(fixtures, 'platform-specification')
   t.rejects(buildIdeal(path, {
     ...OPT,
@@ -95,11 +95,12 @@ t.test('fail on mismatched platform', { skip: process.platform === 'win32' && '*
 
 t.test('ignore mismatched platform for optional dependencies', async t => {
   const path = resolve(fixtures, 'optional-platform-specification')
-  await buildIdeal(path, {
+  const tree = await buildIdeal(path, {
     ...OPT,
     nodeVersion: '12.18.4',
     engineStrict: true,
   })
+  t.equal(tree.children.get('platform-specifying-test-package').package.version, '1.0.0', 'added the optional dep to the ideal tree')
 })
 
 t.test('no options', t => {

test/arborist/reify.js

@@ -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',

test/fixtures/registry-mocks/content/platform-specifying-test-package.json

@@ -19,14 +19,12 @@
       },
       "license": "ISC",
       "os": [
-        "win32"
+        "not-your-os"
       ],
       "_id": "platform-specifying-test-package@1.0.0",
       "_nodeVersion": "12.18.4",
       "_npmVersion": "6.14.6",
       "dist": {
-        "integrity": "sha512-3Np3+FCsBeOWdhTftiBnvjBmK2dkqu/bjwU0smtx+2j4wVAzZBaOh25YvdqZGB6Eo5WykUHRLtQRPJ6V1sNeRQ==",
-        "shasum": "e8ebf03cb5a17e1d9e77e9d19ab22ae16c7f3daf",
         "tarball": "https://registry.npmjs.org/platform-specifying-test-package/-/platform-specifying-test-package-1.0.0.tgz",
         "fileCount": 5,
         "unpackedSize": 3481,

test/fixtures/registry-mocks/content/platform-specifying-test-package.min.json

@@ -8,15 +8,13 @@
       "name": "platform-specifying-test-package",
       "version": "1.0.0",
       "dist": {
-        "integrity": "sha512-3Np3+FCsBeOWdhTftiBnvjBmK2dkqu/bjwU0smtx+2j4wVAzZBaOh25YvdqZGB6Eo5WykUHRLtQRPJ6V1sNeRQ==",
-        "shasum": "e8ebf03cb5a17e1d9e77e9d19ab22ae16c7f3daf",
         "tarball": "https://registry.npmjs.org/platform-specifying-test-package/-/platform-specifying-test-package-1.0.0.tgz",
         "fileCount": 5,
         "unpackedSize": 3481,
         "npm-signature": "-----BEGIN PGP SIGNATURE-----\r\nVersion: OpenPGP.js v3.0.13\r\nComment: https://openpgpjs.org\r\n\r\nwsFcBAEBCAAQBQJfbRvlCRA9TVsSAnZWagAAIQMP/jgNB73uGp5pHvkumGq7\nyA6DC6pqL3vU7Pwctr7QzYi+1jHcWwHtNHHTYwrZZru04xLpiQk8BohAfmOt\nEXstFVfMR/C/Y1pAxi3ITPj79zTmgixUVaor9knGWov8gHyFgQsg+2Jf/90E\nv0fOnAKe4+tb1xZNtKiG+Jc2KBIBhVG3g+YjYc8xYz8FaS4Gj9GPwozhE/kl\nHOu+cAXI/CM2nu1U0Q9Jgxna3i/uzXs8exZVzErgHs6sQPKxBpRwItcl4bOi\nkvUJA+QKmiUNHJauJc/1/vK4O8TD35+kiFSrg82MHnrYuDSzfpRhnjWFKlSb\nfcdLICfV0PhR/6KX4ct+uepUAHHJXBpOe48X5zXjMtFcl1MkImzNzXLbuOZ+\nXo+8LCCc7K5AuBx4HDkAHU0hjdG7k6FLf+fjIdeDbFy/bjPHzb6ecyKf73hX\n+Fh1szs37uwsT2M500qgYgfsLl2GuguBEz2IrFaA7ZGmzXZSF34lsH1dXDgY\nl/CFekaEvA4QaIvGL8BM8CdWkFA2VN8xTuZ9+gC9vsLOSnmaM8Qp1iS+mfr9\nVC5FhANvIi8Ckx0iNJQZ0pC8hMdrT5ox8/oSA42l8XZn4PWQWUco3RA6ksig\n2tcUTuRUDeb40SZV3i1l2xS76YBT4ROeYGyXoiPHBECIo4ps0mGmSd1v3Jdk\nk1n3\r\n=gZQk\r\n-----END PGP SIGNATURE-----\r\n"
       },
       "os": [
-        "win32"
+        "not-your-os"
       ]
     }
   },

test/fixtures/reify-cases/optional-platform-specification.js

@@ -0,0 +1,20 @@
+// generated from test/fixtures/optional-platform-specification
+module.exports = t => {
+  const path = t.testdir({
+  "package.json": JSON.stringify({
+    "name": "platform-test",
+    "version": "1.0.0",
+    "description": "",
+    "main": "index.js",
+    "scripts": {
+      "test": "echo \"Error: no test specified\" && exit 1"
+    },
+    "author": "",
+    "license": "ISC",
+    "optionalDependencies": {
+      "platform-specifying-test-package": "1.0.0"
+    }
+  })
+})
+  return path
+}

test/fixtures/reify-cases/platform-specification.js

@@ -0,0 +1,20 @@
+// generated from test/fixtures/platform-specification
+module.exports = t => {
+  const path = t.testdir({
+  "package.json": JSON.stringify({
+    "name": "platform-test",
+    "version": "1.0.0",
+    "description": "",
+    "main": "index.js",
+    "scripts": {
+      "test": "echo \"Error: no test specified\" && exit 1"
+    },
+    "author": "",
+    "license": "ISC",
+    "dependencies": {
+      "platform-specifying-test-package": "1.0.0"
+    }
+  })
+})
+  return path
+}