perf: use worker processes for node-gyp rebuilds (#1080)

* perf: use worker processes for node-gyp rebuilds

* refactor: use lib not src in test

* build: try parelel

* fix: use unique dev dir when not sequential

* ...

package.json

@@ -2,8 +2,8 @@
   "name": "@electron/rebuild",
   "version": "0.0.0-development",
   "description": "Electron supporting package to rebuild native node modules against the currently installed electron",
-  "main": "lib/src/main.js",
-  "typings": "lib/src/main.d.ts",
+  "main": "lib/main.js",
+  "typings": "lib/main.d.ts",
   "scripts": {
     "codecov": "nyc report --reporter=text-lcov > coverage.lcov && codecov",
     "compile": "tsc",
@@ -12,7 +12,7 @@
     "prepare": "npm run compile",
     "mocha": "cross-env TS_NODE_FILES=true mocha",
     "lint": "eslint --ext .ts .",
-    "spec": "npm run mocha -- test/*.ts",
+    "spec": "tsc && npm run mocha -- test/*.ts",
     "test": "npm run lint && npm run spec"
   },
   "bin": {

src/clang-fetcher.ts

@@ -2,7 +2,6 @@ import * as cp from 'child_process';
 import debug from 'debug';
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import semver from 'semver';
 import * as tar from 'tar';
 import * as zlib from 'zlib';
 import { ELECTRON_GYP_DIR } from './constants';
@@ -37,14 +36,10 @@ export async function getClangEnvironmentVars(electronVersion: string, targetArc
   const clangDownloadDir = await downloadClangVersion(electronVersion);
 
   const clangDir = path.resolve(clangDownloadDir, 'bin');
-  const cxxflags = [];
   const clangArgs: string[] = [];
   if (process.platform === 'darwin') {
     clangArgs.push('-isysroot', getSDKRoot());
   }
-  if (semver.major(electronVersion) >= 20) {
-    cxxflags.push('-std=c++17');
-  }
 
   const gypArgs = [];
   if (process.platform === 'win32') {
@@ -61,7 +56,6 @@ export async function getClangEnvironmentVars(electronVersion: string, targetArc
     env: {
       CC: `"${path.resolve(clangDir, 'clang')}" ${clangArgs.join(' ')}`,
       CXX: `"${path.resolve(clangDir, 'clang++')}" ${clangArgs.join(' ')}`,
-      CXXFLAGS: `${cxxflags.join(' ')}`
     },
     args: gypArgs,
   }

src/module-rebuilder.ts

@@ -3,7 +3,7 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 
 import { cacheModuleState } from './cache';
-import { NodeGyp } from './module-type/node-gyp';
+import { NodeGyp } from './module-type/node-gyp/node-gyp';
 import { Prebuildify } from './module-type/prebuildify';
 import { PrebuildInstall } from './module-type/prebuild-install';
 import { IRebuilder } from './types';

src/module-type/node-gyp.ts → src/module-type/node-gyp/node-gyp.ts

@@ -1,13 +1,12 @@
 import debug from 'debug';
 import detectLibc from 'detect-libc';
-import NodeGypRunner from 'node-gyp';
 import path from 'path';
-import { promisify } from 'util';
 import semver from 'semver';
 
-import { ELECTRON_GYP_DIR } from '../constants';
-import { getClangEnvironmentVars } from '../clang-fetcher';
-import { NativeModule } from '.';
+import { ELECTRON_GYP_DIR } from '../../constants';
+import { getClangEnvironmentVars } from '../../clang-fetcher';
+import { NativeModule } from '..';
+import { fork } from 'child_process';
 
 const d = debug('electron-rebuild');
 
@@ -86,76 +85,45 @@ export class NodeGyp extends NativeModule {
       // throw new Error(`node-gyp does not support building modules with spaces in their path, tried to build: ${modulePath}`);
     }
 
-    let env: Record<string, string | undefined>;
+    const env = {
+      ...process.env,
+    };
     const extraNodeGypArgs: string[] = [];
 
-    if (semver.major(this.rebuilder.electronVersion) >= 20) {
-      process.env.CXXFLAGS = '-std=c++17';
-    }
-
     if (this.rebuilder.useElectronClang) {
-      env = { ...process.env };
       const { env: clangEnv, args: clangArgs } = await getClangEnvironmentVars(this.rebuilder.electronVersion, this.rebuilder.arch);
-      Object.assign(process.env, clangEnv);
+      Object.assign(env, clangEnv);
       extraNodeGypArgs.push(...clangArgs);
     }
 
     const nodeGypArgs = await this.buildArgs(extraNodeGypArgs);
     d('rebuilding', this.moduleName, 'with args', nodeGypArgs);
 
-    const nodeGyp = NodeGypRunner();
-    nodeGyp.parseArgv(nodeGypArgs);
-    nodeGyp.devDir = ELECTRON_GYP_DIR;
-    let command = nodeGyp.todo.shift();
-    const originalWorkingDir = process.cwd();
-    try {
-      process.chdir(this.modulePath);
-      while (command) {
-        if (command.name === 'configure') {
-          command.args = command.args.filter((arg: string) => !extraNodeGypArgs.includes(arg));
-        } else if (command.name === 'build' && process.platform === 'win32') {
-          // This is disgusting but it prevents node-gyp from destroying our MSBuild arguments
-          command.args.map = (fn: (arg: string) => string) => {
-            return Array.prototype.map.call(command.args, (arg: string) => {
-              if (arg.startsWith('/p:')) return arg;
-              return fn(arg);
-            });
-          }
-        }
-        await promisify(nodeGyp.commands[command.name])(command.args);
-        command = nodeGyp.todo.shift();
-      }
-    } catch (err) {
-      const errorMessage = `node-gyp failed to rebuild '${this.modulePath}'.
-For more information, rerun with the DEBUG environment variable set to "electron-rebuild".
-
-Error: ${err.message || err}\n\n`;
-      throw new Error(errorMessage);
-    } finally {
-      process.chdir(originalWorkingDir);
-    }
-
-    if (this.rebuilder.useElectronClang) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      this.restoreEnv(env!);
-    }
-  }
-
-  private restoreEnv(env: Record<string, string | undefined>): void {
-    const gotKeys = new Set<string>(Object.keys(process.env));
-    const expectedKeys = new Set<string>(Object.keys(env));
-
-    for (const key of Object.keys(process.env)) {
-      if (!expectedKeys.has(key)) {
-        delete process.env[key];
-      } else if (env[key] !== process.env[key]) {
-        process.env[key] = env[key];
-      }
-    }
-    for (const key of Object.keys(env)) {
-      if (!gotKeys.has(key)) {
-        process.env[key] = env[key];
-      }
-    }
+    const forkedChild = fork(path.resolve(__dirname, 'worker.js'), {
+      env,
+      cwd: this.modulePath,
+      stdio: 'pipe',
+    });
+    const outputBuffers: Buffer[] = [];
+    forkedChild.stdout?.on('data', (chunk) => {
+      outputBuffers.push(chunk);
+    });
+    forkedChild.stderr?.on('data', (chunk) => {
+      outputBuffers.push(chunk);
+    });
+    forkedChild.send({
+      moduleName: this.moduleName,
+      nodeGypArgs,
+      extraNodeGypArgs,
+      devDir: this.rebuilder.mode === 'sequential' ? ELECTRON_GYP_DIR : path.resolve(ELECTRON_GYP_DIR, '_p', this.moduleName),
+    });
+
+    await new Promise<void>((resolve, reject) => {
+      forkedChild.on('exit', (code) => {
+        if (code === 0) return resolve();
+        console.error(Buffer.concat(outputBuffers).toString());
+        reject(new Error(`node-gyp failed to rebuild '${this.modulePath}'`))
+      });
+    });
   }
 }

src/module-type/node-gyp/worker.ts

@@ -0,0 +1,34 @@
+import NodeGypRunner from 'node-gyp';
+import { promisify } from 'util';
+
+process.on('message', async ({
+  nodeGypArgs,
+  devDir,
+  extraNodeGypArgs,
+}) => {
+  const nodeGyp = NodeGypRunner();
+  nodeGyp.parseArgv(nodeGypArgs);
+  nodeGyp.devDir = devDir;
+  let command = nodeGyp.todo.shift();
+  try {
+    while (command) {
+      if (command.name === 'configure') {
+        command.args = command.args.filter((arg: string) => !extraNodeGypArgs.includes(arg));
+      } else if (command.name === 'build' && process.platform === 'win32') {
+        // This is disgusting but it prevents node-gyp from destroying our MSBuild arguments
+        command.args.map = (fn: (arg: string) => string) => {
+          return Array.prototype.map.call(command.args, (arg: string) => {
+            if (arg.startsWith('/p:')) return arg;
+            return fn(arg);
+          });
+        }
+      }
+      await promisify(nodeGyp.commands[command.name])(command.args);
+      command = nodeGyp.todo.shift();
+    }
+    process.exit(0);
+  } catch (err) {
+    console.error(err);
+    process.exit(1);
+  }
+});

src/module-type/prebuild-install.ts

@@ -18,13 +18,10 @@ export class PrebuildInstall extends NativeModule {
   }
 
   async run(prebuildInstallPath: string): Promise<void> {
-    const shimExt = process.env.ELECTRON_REBUILD_TESTS ? 'ts' : 'js';
-    const executable = process.env.ELECTRON_REBUILD_TESTS ? path.resolve(__dirname, '..', '..', 'node_modules', '.bin', 'ts-node') : process.execPath;
-
     await spawn(
-      executable,
+      process.execPath,
       [
-        path.resolve(__dirname, '..', `prebuild-shim.${shimExt}`),
+        path.resolve(__dirname, '..', `prebuild-shim.js`),
         prebuildInstallPath,
         `--arch=${this.rebuilder.arch}`,
         `--platform=${this.rebuilder.platform}`,

test/arch.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 
-import { getNodeArch, uname } from '../src/arch';
+import { getNodeArch, uname } from '../lib/arch';
 
 // Copied from @electron/get
 describe('uname()', () => {

test/electron-locator.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai';
 import * as fs from 'fs-extra';
 import * as path from 'path';
 
-import { locateElectronModule } from '../src/electron-locator';
+import { locateElectronModule } from '../lib/electron-locator';
 
 const baseFixtureDir = path.resolve(__dirname, 'fixture', 'electron-locator')
 

test/fixture/native-app1/package.json

@@ -15,7 +15,7 @@
   },
   "devDependencies": {
     "@types/node": "^12.0.10",
-    "ffi-napi": "2.4.5"
+    "ffi-napi": "4.0.3"
   },
   "dependencies": {
     "@newrelic/native-metrics": "5.3.0",

test/helpers/module-setup.ts

@@ -1,4 +1,6 @@
+import crypto from 'crypto';
 import fs from 'fs-extra';
+import os from 'os';
 import path from 'path';
 import { spawn } from '@malept/cross-spawn-promise';
 
@@ -14,20 +16,30 @@ export function resetMSVSVersion(): void {
   }
 }
 
+const testModuleTmpPath = fs.mkdtempSync(path.resolve(os.tmpdir(), 'e-r-test-module-'));
+
 export async function resetTestModule(testModulePath: string, installModules = true): Promise<void> {
-  await fs.remove(testModulePath);
-  await fs.mkdir(testModulePath, { recursive: true });
-  await fs.copyFile(
-    path.resolve(__dirname, '../../test/fixture/native-app1/package.json'),
-    path.resolve(testModulePath, 'package.json')
-  );
-  if (installModules) {
-    await spawn('yarn', ['install'], { cwd: testModulePath });
+  const oneTimeModulePath = path.resolve(testModuleTmpPath, `${crypto.createHash('SHA1').update(testModulePath).digest('hex')}-${installModules}`);
+  if (!await fs.pathExists(oneTimeModulePath)) {
+    await fs.mkdir(oneTimeModulePath, { recursive: true });
+    await fs.copyFile(
+      path.resolve(__dirname, '../../test/fixture/native-app1/package.json'),
+      path.resolve(oneTimeModulePath, 'package.json')
+    );
+    if (installModules) {
+      await spawn('yarn', ['install'], { cwd: oneTimeModulePath });
+    }
   }
+  await fs.remove(testModulePath);
+  await fs.copy(oneTimeModulePath, testModulePath);
   resetMSVSVersion();
 }
 
 export async function cleanupTestModule(testModulePath: string): Promise<void> {
   await fs.remove(testModulePath);
   resetMSVSVersion();
 }
+
+process.on('exit', () => {
+  fs.removeSync(testModuleTmpPath);
+});

test/module-type-node-gyp.ts

@@ -4,8 +4,8 @@ import os from 'os';
 import path from 'path';
 
 import { cleanupTestModule, resetTestModule } from './helpers/module-setup';
-import { NodeGyp } from '../src/module-type/node-gyp';
-import { Rebuilder } from '../src/rebuild';
+import { NodeGyp } from '../lib/module-type/node-gyp/node-gyp';
+import { Rebuilder } from '../lib/rebuild';
 
 describe('node-gyp', () => {
   describe('buildArgs', () => {

test/module-type-prebuild-install.ts

@@ -5,8 +5,8 @@ import os from 'os';
 import path from 'path';
 
 import { cleanupTestModule, resetTestModule, TIMEOUT_IN_MILLISECONDS } from './helpers/module-setup';
-import { PrebuildInstall } from '../src/module-type/prebuild-install';
-import { Rebuilder } from '../src/rebuild';
+import { PrebuildInstall } from '../lib/module-type/prebuild-install';
+import { Rebuilder } from '../lib/rebuild';
 
 chai.use(chaiAsPromised);
 
@@ -21,10 +21,6 @@ describe('prebuild-install', () => {
     lifecycle: new EventEmitter()
   };
 
-  before(() => {
-    process.env.ELECTRON_REBUILD_TESTS = 'true';
-  });
-
   describe('Node-API support', function() {
     this.timeout(TIMEOUT_IN_MILLISECONDS);
 
@@ -57,8 +53,4 @@ describe('prebuild-install', () => {
       expect(prebuildInstall.findPrebuiltModule()).to.eventually.be.rejectedWith("Native module 'farmhash' requires Node-API but Electron v2.0.0 does not support Node-API");
     });
   });
-
-  after(() => {
-    delete process.env.ELECTRON_REBUILD_TESTS;
-  });
 });

test/module-type-prebuildify.ts

@@ -6,8 +6,8 @@ import {
   determineNativePrebuildArch,
   determineNativePrebuildExtension,
   Prebuildify
-} from '../src/module-type/prebuildify';
-import { Rebuilder, RebuilderOptions } from '../src/rebuild';
+} from '../lib/module-type/prebuildify';
+import { Rebuilder, RebuilderOptions } from '../lib/rebuild';
 
 describe('determineNativePrebuildArch', () => {
   it('returns arm if passed in armv7l', () => {

test/read-package-json.ts

@@ -1,7 +1,7 @@
 import * as path from 'path';
 import { expect } from 'chai';
 
-import { readPackageJson } from '../src/read-package-json';
+import { readPackageJson } from '../lib/read-package-json';
 
 describe('read-package-json', () => {
   it('should find a package.json file from the given directory', async () => {

test/rebuild-yarnworkspace.ts

@@ -5,8 +5,8 @@ import { spawn } from '@malept/cross-spawn-promise';
 
 import { expectNativeModuleToBeRebuilt, expectNativeModuleToNotBeRebuilt } from './helpers/rebuild';
 import { getExactElectronVersionSync } from './helpers/electron-version';
-import { getProjectRootPath } from '../src/search-module';
-import { rebuild } from '../src/rebuild';
+import { getProjectRootPath } from '../lib/search-module';
+import { rebuild } from '../lib/rebuild';
 
 const testElectronVersion = getExactElectronVersionSync();