Skip to content

Commit

Permalink
fix: allow using forks for prebuilt modules (#1155)
Browse files Browse the repository at this point in the history
* feat: allow forks of node-pre-gyp

* feat: check for prebuild-install and prebuildify forks

* test: finding forked modules

* test: don't reset test module for checking forks
  • Loading branch information
samuelmaddock authored Oct 2, 2024
1 parent 9ebf9bd commit 683129a
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 13 deletions.
18 changes: 18 additions & 0 deletions src/module-type/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@ export class NativeModule {

return binary?.napi_versions;
}

/**
* Search dependencies for package using either `packageName` or
* `@namespace/packageName` in the case of forks.
*/
async findPackageInDependencies(packageName: string, packageProperty = 'dependencies'): Promise<string | null> {
const dependencies = await this.packageJSONFieldWithDefault(packageProperty, {});
if (typeof dependencies !== 'object') return null;

// Look for direct dependency match
// eslint-disable-next-line no-prototype-builtins
if (dependencies.hasOwnProperty(packageName)) return packageName;

const forkedPackage = Object.keys(dependencies).find(dependency =>
dependency.startsWith('@') && dependency.endsWith(`/${packageName}`));

return forkedPackage || null;
}
}

export async function locateBinary(basePath: string, suffix: string): Promise<string | null> {
Expand Down
12 changes: 8 additions & 4 deletions src/module-type/node-pre-gyp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ const d = debug('electron-rebuild');

export class NodePreGyp extends NativeModule {
async usesTool(): Promise<boolean> {
const dependencies = await this.packageJSONFieldWithDefault('dependencies', {});
// eslint-disable-next-line no-prototype-builtins
return dependencies.hasOwnProperty('@mapbox/node-pre-gyp');
const packageName = await this.findPackageInDependencies('node-pre-gyp');
return !!packageName;
}

async locateBinary(): Promise<string | null> {
return locateBinary(this.modulePath, 'node_modules/@mapbox/node-pre-gyp/bin/node-pre-gyp');
const packageName = await this.findPackageInDependencies('node-pre-gyp');
if (!packageName) return null;
return locateBinary(
this.modulePath,
`node_modules/${packageName}/bin/node-pre-gyp`
);
}

async run(nodePreGypPath: string): Promise<void> {
Expand Down
12 changes: 8 additions & 4 deletions src/module-type/prebuild-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@ const d = debug('electron-rebuild');

export class PrebuildInstall extends NativeModule {
async usesTool(): Promise<boolean> {
const dependencies = await this.packageJSONFieldWithDefault('dependencies', {});
// eslint-disable-next-line no-prototype-builtins
return dependencies.hasOwnProperty('prebuild-install');
const packageName = await this.findPackageInDependencies('prebuild-install');
return !!packageName;
}

async locateBinary(): Promise<string | null> {
return locateBinary(this.modulePath, 'node_modules/prebuild-install/bin.js');
const packageName = await this.findPackageInDependencies('prebuild-install');
if (!packageName) return null;
return locateBinary(
this.modulePath,
`node_modules/${packageName}/bin.js`
);
}

async run(prebuildInstallPath: string): Promise<void> {
Expand Down
8 changes: 3 additions & 5 deletions src/module-type/prebuildify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,11 @@ export function determineNativePrebuildExtension(arch: string): string {

export class Prebuildify extends NativeModule {
async usesTool(): Promise<boolean> {
const devDependencies = await this.packageJSONFieldWithDefault('devDependencies', {});
// eslint-disable-next-line no-prototype-builtins
return devDependencies.hasOwnProperty('prebuildify');
const packageName = await this.findPackageInDependencies('prebuildify', 'devDependencies');
return !!packageName;
}

async findPrebuiltModule(): Promise<boolean> {
const nodeArch = getNodeArch(this.rebuilder.arch, process.config.variables as ConfigVariables);

d(`Checking for prebuilds for "${this.moduleName}"`);

const prebuildsDir = path.join(this.modulePath, 'prebuilds');
Expand All @@ -48,6 +45,7 @@ export class Prebuildify extends NativeModule {
return false;
}

const nodeArch = getNodeArch(this.rebuilder.arch, process.config.variables as ConfigVariables);
const prebuiltModuleDir = path.join(prebuildsDir, `${this.rebuilder.platform}-${determineNativePrebuildArch(nodeArch)}`);
const nativeExt = determineNativePrebuildExtension(nodeArch);
const electronNapiModuleFilename = path.join(prebuiltModuleDir, `electron.napi.${nativeExt}`);
Expand Down
17 changes: 17 additions & 0 deletions test/fixture/forked-module-test/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "forked-module-test",
"productName": "Native App",
"version": "1.0.0",
"description": "",
"main": "src/index.js",
"keywords": [],
"author": "",
"license": "MIT",
"dependencies": {
"@electron/node-pre-gyp": "npm:@mapbox/[email protected]",
"@electron/prebuild-install": "npm:[email protected]"
},
"devDependencies": {
"@electron/prebuildify": "npm:[email protected]"
}
}
4 changes: 4 additions & 0 deletions test/helpers/module-setup.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import debug from 'debug';
import crypto from 'crypto';
import fs from 'fs-extra';
import os from 'os';
import path from 'path';
import { spawn } from '@malept/cross-spawn-promise';

const d = debug('electron-rebuild');

const originalGypMSVSVersion: string | undefined = process.env.GYP_MSVS_VERSION;
const TIMEOUT_IN_MINUTES = process.platform === 'win32' ? 5 : 2;

Expand All @@ -23,6 +26,7 @@ const testModuleTmpPath = fs.mkdtempSync(path.resolve(os.tmpdir(), 'e-r-test-mod
export async function resetTestModule(testModulePath: string, installModules = true, fixtureName = 'native-app1'): Promise<void> {
const oneTimeModulePath = path.resolve(testModuleTmpPath, `${crypto.createHash('SHA1').update(testModulePath).digest('hex')}-${fixtureName}-${installModules}`);
if (!await fs.pathExists(oneTimeModulePath)) {
d(`creating test module '%s' in %s`, fixtureName, oneTimeModulePath);
await fs.mkdir(oneTimeModulePath, { recursive: true });
await fs.copy(path.resolve(__dirname, `../../test/fixture/${ fixtureName }`), oneTimeModulePath);
if (installModules) {
Expand Down
6 changes: 6 additions & 0 deletions test/module-type-node-pre-gyp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,10 @@ describe('node-pre-gyp', function () {
nodePreGyp = new NodePreGyp(rebuilder, modulePath);
expect(await nodePreGyp.findPrebuiltModule()).to.equal(true);
});

it('should find module fork', async () => {
const rebuilder = new Rebuilder(rebuilderArgs);
const nodePreGyp = new NodePreGyp(rebuilder, path.join(__dirname, 'fixture', 'forked-module-test'));
expect(await nodePreGyp.usesTool()).to.equal(true);
});
});
6 changes: 6 additions & 0 deletions test/module-type-prebuild-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,10 @@ describe('prebuild-install', () => {
expect(await prebuild.findPrebuiltModule()).to.equal(true);
});
});

it('should find module fork', async () => {
const rebuilder = new Rebuilder(rebuilderArgs);
const prebuildInstall = new PrebuildInstall(rebuilder, path.join(__dirname, 'fixture', 'forked-module-test'));
expect(await prebuildInstall.usesTool()).to.equal(true);
});
});
6 changes: 6 additions & 0 deletions test/module-type-prebuildify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,10 @@ describe('prebuildify', () => {
expect(await prebuildify.findPrebuiltModule()).to.equal(true);
});
});

it('should find module fork', async () => {
const rebuilder = createRebuilder();
const prebuildify = new Prebuildify(rebuilder, path.join(__dirname, 'fixture', 'forked-module-test'));
expect(await prebuildify.usesTool()).to.equal(true);
});
});

0 comments on commit 683129a

Please sign in to comment.