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

yarnpkg · Jan 29, 2019 · 28a65ae · 28a65ae
1 parent 32b7bb9
commit 28a65ae
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 6 deletions.
diff --git a/CHANGELOG.md b/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`

diff --git a/__tests__/package-compatibility.js b/__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);
+});
diff --git a/src/cli/commands/install.js b/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();

diff --git a/src/package-compatibility.js b/src/package-compatibility.js
@@ -126,20 +126,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);
+  const invalidPlatform = shouldCheckPlatform(info, config.ignorePlatform) && !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) && !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 +167,23 @@ export function check(infos: Array<Manifest>, config: Config, ignoreEngines: boo
     checkOne(info, config, ignoreEngines);
   }
 }
+
+function shouldCheckCpu(manifest: Manifest, ignorePlatform: boolean): boolean {
+  return !ignorePlatform && Array.isArray(manifest.cpu) && manifest.cpu.length > 0;
+}
+
+function shouldCheckPlatform(manifest: Manifest, ignorePlatform: boolean): boolean {
+  return !ignorePlatform && Array.isArray(manifest.os) && manifest.os.length > 0;
+}
+
+function shouldCheckEngines(manifest: Manifest, ignoreEngines: boolean): boolean {
+  return !ignoreEngines && typeof manifest.engines === 'object';
+}
+
+export function shouldCheck(manifest: Manifest, options: {ignoreEngines: boolean, ignorePlatform: boolean}): boolean {
+  return (
+    shouldCheckCpu(manifest, options.ignorePlatform) ||
+    shouldCheckPlatform(manifest, options.ignorePlatform) ||
+    shouldCheckEngines(manifest, options.ignoreEngines)
+  );
+}