Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(maker-base): throw a better error when external binaries are missing #1306

Merged
merged 4 commits into from
Nov 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/api/core/src/api/make.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -132,6 +132,8 @@ export default async ({
].join(''));
}

maker.ensureExternalBinariesExist();

makers[targetId] = maker;
targetId += 1;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/api/core/test/slow/api_spec_slow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 15 additions & 2 deletions packages/maker/base/src/Maker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export default abstract class Maker<C> implements IForgeMaker {

public abstract defaultPlatforms: ForgePlatform[];

public requiredExternalBinaries: string[] = [];

__isElectronForgeMaker!: true;

constructor(
Expand Down Expand Up @@ -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(', ')}`);
}
}

/**
Expand Down
19 changes: 19 additions & 0 deletions packages/maker/base/test/support_spec.ts
Original file line number Diff line number Diff line change
@@ -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/);
});
});
4 changes: 3 additions & 1 deletion packages/maker/deb/src/MakerDeb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 3 additions & 1 deletion packages/maker/flatpak/src/MakerFlatpak.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 2 additions & 2 deletions packages/maker/flatpak/test/MakerFlatpak_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/maker/rpm/src/MakerRpm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 2 additions & 2 deletions packages/maker/rpm/test/MakerRpm_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/maker/snap/src/MakerSnap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export default class MakerSnap extends MakerBase<MakerSnapConfig> {

defaultPlatforms: ForgePlatform[] = ['linux'];

requiredExternalBinaries: string[] = ['snapcraft'];

isSupportedOnCurrentPlatform() {
return process.platform === 'linux';
}
Expand Down