fix: run node-gyp rebuild correctly (#409)

* fix(module-rebuilder): iterate through all of the node-gyp commands
* fix(module-rebuilder): set node-gyp's working directory to the module being rebuilt & improve errors
* test: add test to ensure that the module actually got rebuilt
* build(ci): set GYP_MSVS_VERSION in CircleCI
* fix(rebuilder): cache the MSVS version when the rebuilder is created
* test: reset MSVS version after rebuild tests

.circleci/config.yml

@@ -59,6 +59,8 @@ jobs:
  executor:
  name: win/vs2019
  shell: bash.exe
+ environment:
+ GYP_MSVS_VERSION: '2019'
  <<: *steps-test
 
  release:

src/module-rebuilder.ts

@@ -93,8 +93,8 @@ export class ModuleRebuilder {
 
  args.push(...(await this.buildNodeGypArgsFromBinaryField()));
 
- if (process.env.GYP_MSVS_VERSION) {
- args.push(`--msvs_version=${process.env.GYP_MSVS_VERSION}`);
+ if (this.rebuilder.msvsVersion) {
+ args.push(`--msvs_version=${this.rebuilder.msvsVersion}`);
  }
 
  return args;
@@ -168,12 +168,25 @@ export class ModuleRebuilder {
  }
 
  const nodeGypArgs = await this.buildNodeGypArgs();
+ d('rebuilding', this.moduleName, 'with args', nodeGypArgs);
 
  const nodeGyp = NodeGyp();
  nodeGyp.parseArgv(nodeGypArgs);
- const rebuildArgs = nodeGyp.todo[0].args;
- d('rebuilding', this.moduleName, 'with args', rebuildArgs);
- await promisify(nodeGyp.commands.rebuild)(rebuildArgs);
+ let command = nodeGyp.todo.shift();
+ const originalWorkingDir = process.cwd();
+ try {
+ process.chdir(this.modulePath);
+ while (command) {
+ await promisify(nodeGyp.commands[command.name])(command.args);
+ command = nodeGyp.todo.shift();
+ }
+ } catch (err) {
+ let errorMessage = `node-gyp failed to rebuild '${this.modulePath}'.\n`;
+ errorMessage += `Error: ${err.message || err}\n\n`;
+ throw new Error(errorMessage);
+ } finally {
+ process.chdir(originalWorkingDir);
+ }
 
  d('built:', this.moduleName);
  await this.writeMetadata();

src/rebuild.ts

@@ -67,6 +67,7 @@ export class Rebuilder {
  public cachePath: string;
  public prebuildTagPrefix: string;
  public projectRootPath?: string;
+ public msvsVersion?: string;
 
  constructor(options: RebuilderOptions) {
  this.lifecycle = options.lifecycle;
@@ -83,6 +84,7 @@ export class Rebuilder {
  this.useCache = options.useCache || false;
  this.cachePath = options.cachePath || path.resolve(os.homedir(), '.electron-rebuild-cache');
  this.prebuildTagPrefix = options.prebuildTagPrefix || 'v';
+ this.msvsVersion = process.env.GYP_MSVS_VERSION;
 
  if (this.useCache && this.force) {
  console.warn('[WARNING]: Electron Rebuild has force enabled and cache enabled, force take precedence and the cache will not be used.');

test/helpers/checksum.ts

@@ -0,0 +1,21 @@
+import * as crypto from 'crypto';
+import * as fs from 'fs-extra';
+import { promisify } from 'util';
+import * as stream from 'stream';
+
+const pipeline = promisify(stream.pipeline);
+
+export async function determineChecksum(filename: string): Promise<string> {
+ let calculated = '';
+ const file = fs.createReadStream(filename, { encoding: 'binary' });
+ const hasher = crypto.createHash('sha256', { defaultEncoding: 'binary' });
+ hasher.on('readable', () => {
+ const data = hasher.read();
+ if (data) {
+ calculated = data.toString('hex');
+ }
+ });
+ await pipeline(file, hasher);
+
+ return calculated;
+}

test/rebuild-yarnworkspace.ts

@@ -13,13 +13,17 @@ const testElectronVersion = getExactElectronVersionSync();
 describe('rebuild for yarn workspace', function() {
  this.timeout(2 * 60 * 1000);
  const testModulePath = path.resolve(os.tmpdir(), 'electron-rebuild-test');
+ const msvs_version: string | undefined = process.env.GYP_MSVS_VERSION;
 
  describe('core behavior', () => {
  before(async () => {
  await fs.remove(testModulePath);
  await fs.copy(path.resolve(__dirname, 'fixture/workspace-test'), testModulePath);
 
  await spawn('yarn', [], { cwd: testModulePath });
+ if (msvs_version) {
+ process.env.GYP_MSVS_VERSION = msvs_version;
+ }
 
  const projectRootPath = await getProjectRootPath(path.join(testModulePath, 'workspace-test', 'child-workspace'));
 
@@ -41,6 +45,9 @@ describe('rebuild for yarn workspace', function() {
 
  after(async () => {
  await fs.remove(testModulePath);
+ if (msvs_version) {
+ process.env.GYP_MSVS_VERSION = msvs_version;
+ }
  });
  });
 });

test/rebuild.ts

@@ -4,6 +4,7 @@ import * as path from 'path';
 import * as os from 'os';
 import { spawn } from '@malept/cross-spawn-promise';
 
+import { determineChecksum } from './helpers/checksum';
 import { expectNativeModuleToBeRebuilt, expectNativeModuleToNotBeRebuilt } from './helpers/rebuild';
 import { getExactElectronVersionSync } from './helpers/electron-version';
 import { rebuild, RebuildOptions } from '../src/rebuild';
@@ -13,7 +14,13 @@ const testElectronVersion = getExactElectronVersionSync();
 describe('rebuilder', () => {
  const testModulePath = path.resolve(os.tmpdir(), 'electron-rebuild-test');
  const timeoutSeconds = process.platform === 'win32' ? 5 : 2;
+ const msvs_version: string | undefined = process.env.GYP_MSVS_VERSION;
 
+ const resetMSVSVersion = () => {
+ if (msvs_version) {
+ process.env.GYP_MSVS_VERSION = msvs_version;
+ }
+ };
  const resetTestModule = async (): Promise<void> => {
  await fs.remove(testModulePath);
  await fs.mkdirs(testModulePath);
@@ -22,8 +29,14 @@ describe('rebuilder', () => {
  path.resolve(testModulePath, 'package.json')
  );
  await spawn('npm', ['install'], { cwd: testModulePath });
+ resetMSVSVersion();
  };
 
+ const cleanupTestModule = async (): Promise<void> => {
+ await fs.remove(testModulePath);
+ resetMSVSVersion();
+ }
+
  const optionSets: {
  name: string;
  args: RebuildOptions | string[];
@@ -81,7 +94,7 @@ describe('rebuilder', () => {
 
  after(async () => {
  delete process.env.ELECTRON_REBUILD_TESTS;
- await fs.remove(testModulePath);
+ await cleanupTestModule();
  });
  });
  }
@@ -90,9 +103,12 @@ describe('rebuilder', () => {
  this.timeout(timeoutSeconds * 60 * 1000);
 
  before(resetTestModule);
+ after(cleanupTestModule);
+ afterEach(resetMSVSVersion);
 
  it('should skip the rebuild step when disabled', async () => {
  await rebuild(testModulePath, testElectronVersion, process.arch);
+ resetMSVSVersion();
  const rebuilder = rebuild(testModulePath, testElectronVersion, process.arch, [], false);
  let skipped = 0;
  rebuilder.lifecycle.on('module-skip', () => {
@@ -104,6 +120,7 @@ describe('rebuilder', () => {
 
  it('should rebuild all modules again when disabled but the electron ABI bumped', async () => {
  await rebuild(testModulePath, testElectronVersion, process.arch);
+ resetMSVSVersion();
  const rebuilder = rebuild(testModulePath, '3.0.0', process.arch, [], false);
  let skipped = 0;
  rebuilder.lifecycle.on('module-skip', () => {
@@ -115,6 +132,7 @@ describe('rebuilder', () => {
 
  it('should rebuild all modules again when enabled', async () => {
  await rebuild(testModulePath, testElectronVersion, process.arch);
+ resetMSVSVersion();
  const rebuilder = rebuild(testModulePath, testElectronVersion, process.arch, [], true);
  let skipped = 0;
  rebuilder.lifecycle.on('module-skip', () => {
@@ -129,20 +147,24 @@ describe('rebuilder', () => {
  this.timeout(2 * 60 * 1000);
 
  beforeEach(resetTestModule);
- afterEach(async () => await fs.remove(testModulePath));
+ afterEach(cleanupTestModule);
 
  it('should rebuild only specified modules', async () => {
+ const nativeModuleBinary = path.join(testModulePath, 'node_modules', 'farmhash', 'build', 'Release', 'farmhash.node');
+ const nodeModuleChecksum = await determineChecksum(nativeModuleBinary);
  const rebuilder = rebuild({
  buildPath: testModulePath,
  electronVersion: testElectronVersion,
  arch: process.arch,
- onlyModules: ['ffi-napi'],
+ onlyModules: ['farmhash'],
  force: true
  });
  let built = 0;
  rebuilder.lifecycle.on('module-done', () => built++);
  await rebuilder;
  expect(built).to.equal(1);
+ const electronModuleChecksum = await determineChecksum(nativeModuleBinary);
+ expect(electronModuleChecksum).to.not.equal(nodeModuleChecksum);
  });
 
  it('should rebuild multiple specified modules via --only option', async () => {
@@ -164,7 +186,7 @@ describe('rebuilder', () => {
  this.timeout(10 * 60 * 1000);
 
  before(resetTestModule);
- afterEach(async () => await fs.remove(testModulePath));
+ after(cleanupTestModule);
 
  it('should have rebuilt ffi-napi module in Debug mode', async () => {
  await rebuild({