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';
  }