fix: use 'target_arch' for node-pre-gyp (#1115)

* fix: use 'target_arch' for node-pre-gyp * fix: use both arch and target_arch * fix: overwrite binary architecture * chore: add read-binary-file-arch * feat: redownload binary only if arch differs * fix: make node 12 compatible * fix: flip flop between ia32/x64 on windows not arm64 --------- Co-authored-by: Samuel Attard <[email protected]>
electron · Dec 4, 2023 · 5e6f727 · 5e6f727
1 parent 0e934ad
commit 5e6f727
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 8 deletions.
diff --git a/package.json b/package.json
@@ -49,6 +49,7 @@
     "node-api-version": "^0.1.4",
     "node-gyp": "^9.0.0",
     "ora": "^5.1.0",
+    "read-binary-file-arch": "^1.0.6",
     "semver": "^7.3.5",
     "tar": "^6.0.5",
     "yargs": "^17.0.1"

diff --git a/src/module-type/node-pre-gyp.ts b/src/module-type/node-pre-gyp.ts
@@ -1,5 +1,6 @@
 import debug from 'debug';
 import { spawn } from '@malept/cross-spawn-promise';
+import { readBinaryFileArch } from 'read-binary-file-arch';
 
 import { locateBinary, NativeModule } from '.';
 const d = debug('electron-rebuild');
@@ -16,14 +17,18 @@ export class NodePreGyp extends NativeModule {
   }
 
   async run(nodePreGypPath: string): Promise<void> {
+    const redownloadBinary = await this.shouldUpdateBinary(nodePreGypPath);
+
     await spawn(
       process.execPath,
       [
         nodePreGypPath,
         'reinstall',
         '--fallback-to-build',
-        `--arch=${this.rebuilder.arch}`,
-        `--platform=${this.rebuilder.platform}`,
+        ...(redownloadBinary ? ['--update-binary'] : []),
+        `--arch=${this.rebuilder.arch}`, // fallback build arch
+        `--target_arch=${this.rebuilder.arch}`, // prebuild arch
+        `--target_platform=${this.rebuilder.platform}`,
         ...await this.getNodePreGypRuntimeArgs(),
       ],
       {
@@ -65,4 +70,53 @@ export class NodePreGyp extends NativeModule {
       ];
     }
   }
+
+  private async shouldUpdateBinary(nodePreGypPath: string) {
+    let shouldUpdate = false;
+
+    // Redownload binary only if the existing module arch differs from the
+    // target arch.
+    try {
+      const modulePaths = await this.getModulePaths(nodePreGypPath);
+      d('module paths:', modulePaths);
+      for (const modulePath of modulePaths) {
+        let moduleArch;
+        try {
+          moduleArch = await readBinaryFileArch(modulePath);
+          d('module arch:', moduleArch);
+        } catch (error) {
+          d('failed to read module arch:', error.message);
+          continue;
+        }
+
+        if (moduleArch && moduleArch !== this.rebuilder.arch) {
+          shouldUpdate = true;
+          d('module architecture differs:', `${moduleArch} !== ${this.rebuilder.arch}`);
+          break;
+        }
+      }
+    } catch (error) {
+      d('failed to get existing binary arch:', error.message);
+
+      // Assume architecture differs
+      shouldUpdate = true;
+    }
+
+    return shouldUpdate;
+  }
+
+  private async getModulePaths(nodePreGypPath: string): Promise<string[]> {
+    const results = await spawn(process.execPath, [
+      nodePreGypPath,
+      'reveal',
+      'module', // pick property with module path
+      `--target_arch=${this.rebuilder.arch}`,
+      `--target_platform=${this.rebuilder.platform}`,
+    ], {
+      cwd: this.modulePath,
+    });
+
+    // Packages with multiple binaries will output one per line
+    return results.split('\n').filter(Boolean);
+  }
 }
diff --git a/test/module-type-node-pre-gyp.ts b/test/module-type-node-pre-gyp.ts
@@ -9,7 +9,9 @@ import { Rebuilder } from '../lib/rebuild';
 
 chai.use(chaiAsPromised);
 
-describe('node-pre-gyp', () => {
+describe('node-pre-gyp', function () {
+  this.timeout(TIMEOUT_IN_MILLISECONDS);
+
   const modulePath = path.join(testModulePath, 'node_modules', 'sqlite3');
   const rebuilderArgs = {
     buildPath: testModulePath,
@@ -18,12 +20,10 @@ describe('node-pre-gyp', () => {
     lifecycle: new EventEmitter()
   };
 
-  describe('Node-API support', function() {
-    this.timeout(TIMEOUT_IN_MILLISECONDS);
-
-    before(async () => await resetTestModule(testModulePath));
-    after(async () => await cleanupTestModule(testModulePath));
+  before(async () => await resetTestModule(testModulePath));
+  after(async () => await cleanupTestModule(testModulePath));
 
+  describe('Node-API support', function() {
     it('should find correct napi version and select napi args', async () => {
       const rebuilder = new Rebuilder(rebuilderArgs);
       const nodePreGyp = new NodePreGyp(rebuilder, modulePath);
@@ -47,4 +47,21 @@ describe('node-pre-gyp', () => {
       expect(nodePreGyp.findPrebuiltModule()).to.eventually.be.rejectedWith("Native module 'sqlite3' requires Node-API but Electron v2.0.0 does not support Node-API");
     });
   });
+
+  it('should redownload the module if the architecture changes', async () => {
+    let rebuilder = new Rebuilder(rebuilderArgs);
+    let nodePreGyp = new NodePreGyp(rebuilder, modulePath);
+    expect(await nodePreGyp.findPrebuiltModule()).to.equal(true);
+
+    let alternativeArch: string;
+    if (process.platform === 'win32') {
+      alternativeArch = rebuilderArgs.arch === 'x64' ? 'ia32' : 'x64';
+    } else {
+      alternativeArch = rebuilderArgs.arch === 'arm64' ? 'x64' : 'arm64'
+    }
+
+    rebuilder = new Rebuilder({ ...rebuilderArgs, arch: alternativeArch });
+    nodePreGyp = new NodePreGyp(rebuilder, modulePath);
+    expect(await nodePreGyp.findPrebuiltModule()).to.equal(true);
+  });
 });
diff --git a/yarn.lock b/yarn.lock
@@ -1200,6 +1200,13 @@ debug@4, [email protected], debug@^4.0.1, debug@^4.1.0, debug@^4.1.1, debug@^4.3.1:
   dependencies:
     ms "2.1.2"
 
+debug@^4.3.4:
+  version "4.3.4"
+  resolved "https://registry.yarnpkg.com/debug/-/debug-4.3.4.tgz#1319f6579357f2338d3337d2cdd4914bb5dcc865"
+  integrity sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==
+  dependencies:
+    ms "2.1.2"
+
 decamelize@^1.2.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290"
@@ -2773,6 +2780,13 @@ randombytes@^2.1.0:
   dependencies:
     safe-buffer "^5.1.0"
 
+read-binary-file-arch@^1.0.6:
+  version "1.0.6"
+  resolved "https://registry.yarnpkg.com/read-binary-file-arch/-/read-binary-file-arch-1.0.6.tgz#959c4637daa932280a9b911b1a6766a7e44288fc"
+  integrity sha512-BNg9EN3DD3GsDXX7Aa8O4p92sryjkmzYYgmgTAc6CA4uGLEDzFfxOxugu21akOxpcXHiEgsYkC6nPsQvLLLmEg==
+  dependencies:
+    debug "^4.3.4"
+
 readable-stream@^3.4.0, readable-stream@^3.6.0:
   version "3.6.0"
   resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-3.6.0.tgz#337bbda3adc0706bd3e024426a286d4b4b2c9198"