fix(maker-base): throw a better error when external binaries are miss…

…ing (#1306) ISSUES CLOSED: #1262
electron · Nov 30, 2019 · c8baa1e · c8baa1e
1 parent a0c15d9
commit c8baa1e
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 12 deletions.
diff --git a/packages/api/core/src/api/make.ts b/packages/api/core/src/api/make.ts
@@ -21,9 +21,9 @@ import requireSearch from '../util/require-search';
 import packager from './package';
 
 class MakerImpl extends MakerBase<any> {
- name = 'impl';
+  name = 'impl';
 
- defaultPlatforms = [];
+  defaultPlatforms = [];
 }
 
 export interface MakeOptions {
@@ -132,6 +132,8 @@ export default async ({
       ].join(''));
     }
 
+    maker.ensureExternalBinariesExist();
+
     makers[targetId] = maker;
     targetId += 1;
   }

diff --git a/packages/api/core/test/slow/api_spec_slow.ts b/packages/api/core/test/slow/api_spec_slow.ts
@@ -337,7 +337,8 @@ describe(`electron-forge API (with installer=${nodeInstaller})`, () => {
           .filter((makerPath) => {
             const MakerClass = require(makerPath).default;
             const maker = new MakerClass();
-            return maker.isSupportedOnCurrentPlatform() === good;
+            return maker.isSupportedOnCurrentPlatform() === good
+              && maker.externalBinariesExist() === good;
           })
           .map((makerPath) => () => ({
             name: makerPath,

diff --git a/packages/maker/base/src/Maker.ts b/packages/maker/base/src/Maker.ts
@@ -44,6 +44,8 @@ export default abstract class Maker<C> implements IForgeMaker {
 
   public abstract defaultPlatforms: ForgePlatform[];
 
+  public requiredExternalBinaries: string[] = [];
+
   __isElectronForgeMaker!: true;
 
   constructor(
@@ -131,8 +133,19 @@ export default abstract class Maker<C> implements IForgeMaker {
   /**
    * Checks if the specified binaries exist, which are required for the maker to be used.
    */
-  externalBinariesExist(binaries: string[]): boolean {
-    return binaries.every((binary) => which.sync(binary, { nothrow: true }) !== null);
+  externalBinariesExist(): boolean {
+    return this.requiredExternalBinaries.every(
+      (binary) => which.sync(binary, { nothrow: true }) !== null,
+    );
+  }
+
+  /**
+   * Throws an error if any of the binaries don't exist.
+   */
+  ensureExternalBinariesExist() {
+    if (!this.externalBinariesExist()) {
+      throw new Error(`Cannot make for ${this.name}, the following external binaries need to be installed: ${this.requiredExternalBinaries.join(', ')}`);
+    }
   }
 
   /**

diff --git a/packages/maker/base/test/support_spec.ts b/packages/maker/base/test/support_spec.ts
@@ -0,0 +1,19 @@
+import { expect } from 'chai';
+
+import MakerBase from '../src/Maker';
+
+class MakerImpl extends MakerBase<{}> {
+  name = 'test';
+
+  defaultPlatforms = [];
+
+  requiredExternalBinaries = ['bash', 'nonexistent'];
+}
+
+describe('ensureExternalBinariesExist', () => {
+  const maker = new MakerImpl({}, []);
+
+  it('throws an error when one of the binaries does not exist', () => {
+    expect(() => maker.ensureExternalBinariesExist()).to.throw(/the following external binaries need to be installed: bash, nonexistent/);
+  });
+});
diff --git a/packages/maker/deb/src/MakerDeb.ts b/packages/maker/deb/src/MakerDeb.ts
@@ -19,8 +19,10 @@ export default class MakerDeb extends MakerBase<MakerDebConfig> {
 
   defaultPlatforms: ForgePlatform[] = ['linux'];
 
+  requiredExternalBinaries: string[] = ['dpkg', 'fakeroot'];
+
   isSupportedOnCurrentPlatform() {
-    return this.isInstalled('electron-installer-debian') && this.externalBinariesExist(['dpkg', 'fakeroot']);
+    return this.isInstalled('electron-installer-debian');
   }
 
   async make({

diff --git a/packages/maker/flatpak/src/MakerFlatpak.ts b/packages/maker/flatpak/src/MakerFlatpak.ts
@@ -21,8 +21,10 @@ export default class MakerFlatpak extends MakerBase<MakerFlatpakConfig> {
 
   defaultPlatforms: ForgePlatform[] = ['linux'];
 
+  requiredExternalBinaries: string[] = ['flatpak-builder'];
+
   isSupportedOnCurrentPlatform() {
-    return this.isInstalled('@malept/electron-installer-flatpak') && this.externalBinariesExist(['flatpak-builder']);
+    return this.isInstalled('@malept/electron-installer-flatpak');
   }
 
   async make({

diff --git a/packages/maker/flatpak/test/MakerFlatpak_spec.ts b/packages/maker/flatpak/test/MakerFlatpak_spec.ts
@@ -11,9 +11,9 @@ import { flatpakArch } from '../src/MakerFlatpak';
 import { MakerFlatpakConfig } from '../src/Config';
 
 class MakerImpl extends MakerBase<MakerFlatpakConfig> {
- name = 'test';
+  name = 'test';
 
- defaultPlatforms = [];
+  defaultPlatforms = [];
 }
 
 describe('MakerFlatpak', () => {

diff --git a/packages/maker/rpm/src/MakerRpm.ts b/packages/maker/rpm/src/MakerRpm.ts
@@ -19,8 +19,10 @@ export default class MakerRpm extends MakerBase<MakerRpmConfig> {
 
   defaultPlatforms: ForgePlatform[] = ['linux'];
 
+  requiredExternalBinaries: string[] = ['rpmbuild'];
+
   isSupportedOnCurrentPlatform() {
-    return this.isInstalled('electron-installer-redhat') && this.externalBinariesExist(['rpmbuild']);
+    return this.isInstalled('electron-installer-redhat');
   }
 
   async make({

diff --git a/packages/maker/rpm/test/MakerRpm_spec.ts b/packages/maker/rpm/test/MakerRpm_spec.ts
@@ -10,9 +10,9 @@ import { MakerRpmConfig } from '../src/Config';
 import { rpmArch } from '../src/MakerRpm';
 
 class MakerImpl extends MakerBase<MakerRpmConfig> {
- name = 'test';
+  name = 'test';
 
- defaultPlatforms = [];
+  defaultPlatforms = [];
 }
 
 describe('MakerRpm', () => {

diff --git a/packages/maker/snap/src/MakerSnap.ts b/packages/maker/snap/src/MakerSnap.ts
@@ -10,6 +10,8 @@ export default class MakerSnap extends MakerBase<MakerSnapConfig> {
 
   defaultPlatforms: ForgePlatform[] = ['linux'];
 
+  requiredExternalBinaries: string[] = ['snapcraft'];
+
   isSupportedOnCurrentPlatform() {
     return process.platform === 'linux';
   }