fix: replacing macro {napi_build_version} in binary filename (#1078)

* fix: replacing macro {napi_build_versions} in binary filename by using `binary.napi_build_versions` in package.json. Fixes: #554 * cleanup * move napi_build_version to separate test fixture refactored rebuild helper * whoops. missed an unused import * log all files? * log upper dir? something is missing on linux builds * fix linux builds by detecting libc family * cleanup * refactor due to new file location (tsc now runs first) * fix test? * fix test? * revert yarnworkspace changes to check against master * add fixtureName to tmp dir * cleanup and simplify * switch back to use detect-libc and see if test passes
electron · Apr 22, 2023 · 82da9d9 · 82da9d9
1 parent 02d7c66
commit 82da9d9
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 39 deletions.
diff --git a/src/module-type/node-gyp/node-gyp.ts b/src/module-type/node-gyp/node-gyp.ts
@@ -49,6 +49,10 @@ export class NodeGyp extends NativeModule {
 
   async buildArgsFromBinaryField(): Promise<string[]> {
     const binary = await this.packageJSONFieldWithDefault('binary', {}) as Record<string, string>;
+    let napiBuildVersion: number | undefined = undefined
+    if (Array.isArray(binary.napi_versions)) {
+      napiBuildVersion = this.nodeAPI.getNapiVersion(binary.napi_versions.map(str => Number(str)))
+    }
     const flags = await Promise.all(Object.entries(binary).map(async ([binaryKey, binaryValue]) => {
       if (binaryKey === 'napi_versions') {
         return;
@@ -66,7 +70,9 @@ export class NodeGyp extends NativeModule {
         .replace('{arch}', this.rebuilder.arch)
         .replace('{version}', await this.packageJSONField('version') as string)
         .replace('{libc}', await detectLibc.family() || 'unknown');
-
+      if (napiBuildVersion !== undefined) {
+        value = value.replace('{napi_build_version}', napiBuildVersion.toString())
+      }
       for (const [replaceKey, replaceValue] of Object.entries(binary)) {
         value = value.replace(`{${replaceKey}}`, replaceValue);
       }

diff --git a/test/fixture/napi-build-version/package.json b/test/fixture/napi-build-version/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "workspace-app",
+  "productName": "Workspace App",
+  "version": "1.0.0",
+  "description": "",
+  "main": "src/index.js",
+  "keywords": [],
+  "author": "",
+  "license": "MIT",
+  "dependencies": {
+    "sqlite3": "5.1.6"
+  }
+}
diff --git a/test/helpers/module-setup.ts b/test/helpers/module-setup.ts
@@ -10,6 +10,8 @@ const TIMEOUT_IN_MINUTES = process.platform === 'win32' ? 5 : 2;
 export const MINUTES_IN_MILLISECONDS = 60 * 1000;
 export const TIMEOUT_IN_MILLISECONDS = TIMEOUT_IN_MINUTES * MINUTES_IN_MILLISECONDS;
 
+export const TEST_MODULE_PATH = path.resolve(os.tmpdir(), 'electron-rebuild-test');
+
 export function resetMSVSVersion(): void {
   if (originalGypMSVSVersion) {
     process.env.GYP_MSVS_VERSION = originalGypMSVSVersion;
@@ -18,14 +20,11 @@ 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> {
-  const oneTimeModulePath = path.resolve(testModuleTmpPath, `${crypto.createHash('SHA1').update(testModulePath).digest('hex')}-${installModules}`);
+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)) {
     await fs.mkdir(oneTimeModulePath, { recursive: true });
-    await fs.copyFile(
-      path.resolve(__dirname, '../../test/fixture/native-app1/package.json'),
-      path.resolve(oneTimeModulePath, 'package.json')
-    );
+    await fs.copy(path.resolve(__dirname, `../../test/fixture/${ fixtureName }`), oneTimeModulePath);
     if (installModules) {
       await spawn('yarn', ['install'], { cwd: oneTimeModulePath });
     }

diff --git a/test/module-type-node-gyp.ts b/test/module-type-node-gyp.ts
@@ -1,15 +1,12 @@
 import { EventEmitter } from 'events';
 import { expect } from 'chai';
-import os from 'os';
-import path from 'path';
 
-import { cleanupTestModule, resetTestModule } from './helpers/module-setup';
+import { cleanupTestModule, resetTestModule, TEST_MODULE_PATH as testModulePath } from './helpers/module-setup';
 import { NodeGyp } from '../lib/module-type/node-gyp/node-gyp';
 import { Rebuilder } from '../lib/rebuild';
 
 describe('node-gyp', () => {
   describe('buildArgs', () => {
-    const testModulePath = path.resolve(os.tmpdir(), 'electron-rebuild-test');
 
     before(async () => await resetTestModule(testModulePath, false));
     after(async () => await cleanupTestModule(testModulePath));

diff --git a/test/module-type-prebuild-install.ts b/test/module-type-prebuild-install.ts
@@ -1,17 +1,14 @@
 import chai, { expect } from 'chai';
 import chaiAsPromised from 'chai-as-promised';
 import { EventEmitter } from 'events';
-import os from 'os';
 import path from 'path';
 
-import { cleanupTestModule, resetTestModule, TIMEOUT_IN_MILLISECONDS } from './helpers/module-setup';
+import { cleanupTestModule, resetTestModule, TIMEOUT_IN_MILLISECONDS, TEST_MODULE_PATH as testModulePath } from './helpers/module-setup';
 import { PrebuildInstall } from '../lib/module-type/prebuild-install';
 import { Rebuilder } from '../lib/rebuild';
 
 chai.use(chaiAsPromised);
 
-const testModulePath = path.resolve(os.tmpdir(), 'electron-rebuild-test');
-
 describe('prebuild-install', () => {
   const modulePath = path.join(testModulePath, 'node_modules', 'farmhash');
   const rebuilderArgs = {

diff --git a/test/rebuild-napibuildversion.ts b/test/rebuild-napibuildversion.ts
@@ -0,0 +1,49 @@
+import * as fs from 'fs-extra';
+import * as path from 'path';
+
+import { expect } from 'chai';
+import { rebuild } from '../lib/rebuild';
+import { getExactElectronVersionSync } from './helpers/electron-version';
+import { TIMEOUT_IN_MILLISECONDS, TEST_MODULE_PATH as testModulePath, cleanupTestModule, resetTestModule } from './helpers/module-setup';
+import { expectNativeModuleToBeRebuilt } from './helpers/rebuild';
+import detectLibc from 'detect-libc';
+
+const testElectronVersion = getExactElectronVersionSync();
+
+describe('rebuild with napi_build_versions in binary config', async function () {
+  this.timeout(TIMEOUT_IN_MILLISECONDS);
+
+  const napiBuildVersion = 6;
+  const napiBuildVersionSpecificPath = (arch: string, libc: string) => path.resolve(testModulePath, `node_modules/sqlite3/lib/binding/napi-v${ napiBuildVersion }-${ process.platform }-${ libc }-${ arch }/node_sqlite3.node`);
+
+  before(async () => {
+    await resetTestModule(testModulePath, true, 'napi-build-version')
+    // Forcing `msvs_version` needed in order for `arm64` `win32` binary to be built
+    process.env.GYP_MSVS_VERSION = "2019"
+  });
+  after(() => cleanupTestModule(testModulePath));
+
+  // https://github.com/electron/rebuild/issues/554
+  const archs = ['x64', 'arm64']
+  for (const arch of archs) {
+    it(`${ arch } arch should have rebuilt bianry with 'napi_build_versions' array and 'libc' provided`, async () => {
+      const libc = await detectLibc.family() || 'unknown'
+      const binaryPath = napiBuildVersionSpecificPath(arch, libc)
+
+      if (await fs.pathExists(binaryPath)) {
+        fs.removeSync(binaryPath)
+      }
+      expect(await fs.pathExists(binaryPath)).to.be.false;
+
+      await rebuild({
+        buildPath: testModulePath,
+        electronVersion: testElectronVersion,
+        arch
+      });
+
+      await expectNativeModuleToBeRebuilt(testModulePath, 'sqlite3');
+      expect(await fs.pathExists(binaryPath)).to.be.true;
+    });
+  }
+
+});
diff --git a/test/rebuild-yarnworkspace.ts b/test/rebuild-yarnworkspace.ts
@@ -1,30 +1,19 @@
-import * as fs from 'fs-extra';
 import * as path from 'path';
-import * as os from 'os';
-import { spawn } from '@malept/cross-spawn-promise';
 
 import { expectNativeModuleToBeRebuilt, expectNativeModuleToNotBeRebuilt } from './helpers/rebuild';
 import { getExactElectronVersionSync } from './helpers/electron-version';
 import { getProjectRootPath } from '../lib/search-module';
 import { rebuild } from '../lib/rebuild';
+import { TIMEOUT_IN_MILLISECONDS, TEST_MODULE_PATH as testModulePath, cleanupTestModule, resetTestModule } from './helpers/module-setup';
 
 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;
+  this.timeout(TIMEOUT_IN_MILLISECONDS);
 
   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;
-      }
-
+      await resetTestModule(testModulePath, true, 'workspace-test')
       const projectRootPath = await getProjectRootPath(path.join(testModulePath, 'workspace-test', 'child-workspace'));
 
       await rebuild({
@@ -34,6 +23,7 @@ describe('rebuild for yarn workspace', function() {
         projectRootPath
       });
     });
+    after(() => cleanupTestModule(testModulePath));
 
     it('should have rebuilt top level prod dependencies', async () => {
       await expectNativeModuleToBeRebuilt(testModulePath, 'snappy');
@@ -42,12 +32,5 @@ describe('rebuild for yarn workspace', function() {
     it('should not have rebuilt top level devDependencies', async () => {
       await expectNativeModuleToNotBeRebuilt(testModulePath, 'sleep');
     });
-
-    after(async () => {
-      await fs.remove(testModulePath);
-      if (msvs_version) {
-        process.env.GYP_MSVS_VERSION = msvs_version;
-      }
-    });
   });
 });
diff --git a/test/rebuild.ts b/test/rebuild.ts
@@ -1,17 +1,15 @@
 import { expect } from 'chai';
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import * as os from 'os';
 
-import { cleanupTestModule, MINUTES_IN_MILLISECONDS, resetMSVSVersion, resetTestModule, TIMEOUT_IN_MILLISECONDS } from './helpers/module-setup';
+import { cleanupTestModule, MINUTES_IN_MILLISECONDS, TEST_MODULE_PATH as testModulePath, resetMSVSVersion, resetTestModule, TIMEOUT_IN_MILLISECONDS } from './helpers/module-setup';
 import { expectNativeModuleToBeRebuilt, expectNativeModuleToNotBeRebuilt } from './helpers/rebuild';
 import { getExactElectronVersionSync } from './helpers/electron-version';
 import { rebuild } from '../lib/rebuild';
 
 const testElectronVersion = getExactElectronVersionSync();
 
 describe('rebuilder', () => {
-  const testModulePath = path.resolve(os.tmpdir(), 'electron-rebuild-test');
 
   describe('core behavior', function() {
     this.timeout(TIMEOUT_IN_MILLISECONDS);