Check os and platform even when engines is not present in package.json

CHANGELOG.md

@@ -34,6 +34,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa
 
   [#6942](https://github.com/yarnpkg/yarn/pull/6942) - [**John-David Dalton**](https://twitter.com/jdalton)
 
+- Check `os` and `platform` requirements from `package.json` even when the package does not specify any `engines` requirements
+
+  [#6976](https://github.com/yarnpkg/yarn/pull/6976) - [**Micha Reiser**](https://github.com/MichaReiser)  
+
 ## 1.13.0
 
 - Implements a new `package.json` field: `peerDependenciesMeta`

__tests__/package-compatibility.js

@@ -1,6 +1,6 @@
 /* @flow */
 
-import {testEngine} from '../src/package-compatibility.js';
+import {testEngine, shouldCheck} from '../src/package-compatibility.js';
 
 test('node semver semantics', () => {
   expect(testEngine('node', '^5.0.0', {node: '5.1.0'}, true)).toEqual(true);
@@ -19,3 +19,67 @@ test('node semver semantics', () => {
 test('ignore semver prerelease semantics for yarn', () => {
   expect(testEngine('yarn', '^1.3.0', {yarn: '1.4.1-20180208.2355'}, true)).toEqual(true);
 });
+
+test('shouldCheck returns true if ignorePlatform is false and the manifest specifies an os or cpu requirement', () => {
+  expect(
+    shouldCheck(
+      {
+        os: ['darwin'],
+      },
+      {ignorePlatform: false, ignoreEngines: false},
+    ),
+  ).toBe(true);
+
+  expect(
+    shouldCheck(
+      {
+        cpu: ['i32'],
+      },
+      {ignorePlatform: false, ignoreEngines: false},
+    ),
+  ).toBe(true);
+
+  expect(shouldCheck({}, {ignorePlatform: false, ignoreEngines: false})).toBe(false);
+
+  expect(
+    shouldCheck(
+      {
+        os: [],
+        cpu: [],
+      },
+      {ignorePlatform: false, ignoreEngines: false},
+    ),
+  ).toBe(false);
+
+  expect(
+    shouldCheck(
+      {
+        cpu: ['i32'],
+        os: ['darwin'],
+      },
+      {ignorePlatform: true, ignoreEngines: false},
+    ),
+  ).toBe(false);
+});
+
+test('shouldCheck returns true if ignoreEngines is false and the manifest specifies engines', () => {
+  expect(
+    shouldCheck(
+      {
+        engines: {node: '>= 10'},
+      },
+      {ignorePlatform: false, ignoreEngines: false},
+    ),
+  ).toBe(true);
+
+  expect(shouldCheck({}, {ignorePlatform: false, ignoreEngines: false})).toBe(false);
+
+  expect(
+    shouldCheck(
+      {
+        engines: {node: '>= 10'},
+      },
+      {ignorePlatform: false, ignoreEngines: true},
+    ),
+  ).toBe(false);
+});

packages/pkg-tests/pkg-tests-specs/sources/basic.js

@@ -380,5 +380,69 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
         },
       ),
     );
+
+    test(
+      `it should fail if the environment does not satisfy the os platform`,
+      makeTemporaryEnv(
+        {
+          os: ['unicorn'],
+        },
+        async ({path, run, source}) => {
+          await expect(run(`install`)).rejects.toThrow(/The platform "\w+" is incompatible with this module\./);
+        },
+      ),
+    );
+
+    test(
+      `it should fail if the environment does not satisfy the cpu architecture`,
+      makeTemporaryEnv(
+        {
+          cpu: ['unicorn'],
+        },
+        async ({path, run, source}) => {
+          await expect(run(`install`)).rejects.toThrow(/The CPU architecture "\w+" is incompatible with this module\./);
+        },
+      ),
+    );
+
+    test(
+      `it should fail if the environment does not satisfy the engine requirements`,
+      makeTemporaryEnv(
+        {
+          engines: {
+            node: "0.18.1"
+          }
+        },
+        async ({path, run, source}) => {
+          await expect(run(`install`)).rejects.toThrow(/The engine "node" is incompatible with this module\. Expected version "0.18.1"./);
+        },
+      ),
+    );
+
+    test(
+      `it should not fail if the environment does not satisfy the os and cpu architecture but ignore platform is true`,
+      makeTemporaryEnv(
+        {
+          os: ['unicorn'],
+        },
+        async ({path, run, source}) => {
+          await run(`install`, '--ignore-platform');
+        },
+      ),
+    );
+
+    test(
+      `it should not fail if the environment does not satisfy the engine requirements but ignore engines is true`,
+      makeTemporaryEnv(
+        {
+          engines: {
+            node: "0.18.1"
+          }
+        },
+        async ({path, run, source}) => {
+          await run(`install`, '--ignore-engines');
+        },
+      ),
+    );
   });
 };

src/cli/commands/install.js

@@ -572,7 +572,7 @@ export class Install {
       this.scripts.setArtifacts(artifacts);
     }
 
-    if (!this.flags.ignoreEngines && typeof manifest.engines === 'object') {
+    if (compatibility.shouldCheck(manifest, this.flags)) {
       steps.push(async (curr: number, total: number) => {
         this.reporter.step(curr, total, this.reporter.lang('checkingManifest'), emoji.get('mag'));
         await this.checkCompatibility();

src/package-compatibility.js

@@ -15,6 +15,8 @@ const VERSIONS = Object.assign({}, process.versions, {
   yarn: yarnVersion,
 });
 
+type PartialManifest = $Shape<Manifest>;
+
 function isValid(items: Array<string>, actual: string): boolean {
   let isNotWhitelist = true;
   let isBlacklist = false;
@@ -127,19 +129,19 @@ export function checkOne(info: Manifest, config: Config, ignoreEngines: boolean)
   };
 
   const invalidPlatform =
-    !config.ignorePlatform && Array.isArray(info.os) && info.os.length > 0 && !isValidPlatform(info.os);
+    shouldCheckPlatform(info, config.ignorePlatform) && info.os != null && !isValidPlatform(info.os);
 
   if (invalidPlatform) {
     pushError(reporter.lang('incompatibleOS', process.platform));
   }
 
-  const invalidCpu = !config.ignorePlatform && Array.isArray(info.cpu) && info.cpu.length > 0 && !isValidArch(info.cpu);
+  const invalidCpu = shouldCheckCpu(info, config.ignorePlatform) && info.cpu != null && !isValidArch(info.cpu);
 
   if (invalidCpu) {
     pushError(reporter.lang('incompatibleCPU', process.arch));
   }
 
-  if (!ignoreEngines && typeof info.engines === 'object') {
+  if (shouldCheckEngines(info, ignoreEngines)) {
     for (const entry of entries(info.engines)) {
       let name = entry[0];
       const range = entry[1];
@@ -168,3 +170,26 @@ export function check(infos: Array<Manifest>, config: Config, ignoreEngines: boo
     checkOne(info, config, ignoreEngines);
   }
 }
+
+function shouldCheckCpu(manifest: PartialManifest, ignorePlatform: boolean): boolean {
+  return !ignorePlatform && Array.isArray(manifest.cpu) && manifest.cpu.length > 0;
+}
+
+function shouldCheckPlatform(manifest: PartialManifest, ignorePlatform: boolean): boolean {
+  return !ignorePlatform && Array.isArray(manifest.os) && manifest.os.length > 0;
+}
+
+function shouldCheckEngines(manifest: PartialManifest, ignoreEngines: boolean): boolean {
+  return !ignoreEngines && typeof manifest.engines === 'object';
+}
+
+export function shouldCheck(
+  manifest: PartialManifest,
+  options: {ignoreEngines: boolean, ignorePlatform: boolean},
+): boolean {
+  return (
+    shouldCheckCpu(manifest, options.ignorePlatform) ||
+    shouldCheckPlatform(manifest, options.ignorePlatform) ||
+    shouldCheckEngines(manifest, options.ignoreEngines)
+  );
+}