Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect folder name when rebuilding "sqlite3" for arm64/x64 architecture #554

Closed
gmantri opened this issue Dec 21, 2020 · 13 comments · Fixed by #1078
Closed

Incorrect folder name when rebuilding "sqlite3" for arm64/x64 architecture #554

gmantri opened this issue Dec 21, 2020 · 13 comments · Fixed by #1078
Labels

Comments

@gmantri
Copy link

gmantri commented Dec 21, 2020

I am using version 2.3.4 for electron-rebuild. When I try to rebuild "sqlite3" (version 5.0.0) for "arm64" architecture using the following command:

electron-rebuild -m app -f -a arm64 -w sqlite3

the rebuild operation completes however the folder name seems to be incorrect. Please see the screenshot below:

Screenshot 2020-12-21 at 9 46 06 PM

I believe {napi_build_version} should be a number (probably 3). What could be the reason for that?

UPDATE

Same thing happens for "x64" build as well.

Screenshot 2020-12-22 at 1 23 10 PM

@gmantri gmantri changed the title Incorrect folder name when rebuilding "sqlite3" for arm64 architecture Incorrect folder name when rebuilding "sqlite3" for arm64/x64 architecture Dec 22, 2020
@marcoancona
Copy link

I can confirm the issue when trying to compile realm-js for Node on a Mac M1 with electron-rebuild --force --arch=arm64

image

@sators
Copy link

sators commented Dec 21, 2021

Same happening here...

@balajiv113
Copy link

Any update on this ??
Facing same issue as well

@Sara2009
Copy link

Sara2009 commented Mar 7, 2023

Same issue +1.
image

@h3poteto
Copy link

h3poteto commented Mar 9, 2023

I have the same issue.

@Sara2009
Copy link

Hi! 👋

Today I used patch-package to patch @electron/[email protected] for the project I'm working on. It works.

Here is the diff that solved my problem:

diff --git a/node_modules/@electron/rebuild/lib/src/module-type/node-gyp.js b/node_modules/@electron/rebuild/lib/src/module-type/node-gyp.js
index 8fd6878..a8f341c 100644
--- a/node_modules/@electron/rebuild/lib/src/module-type/node-gyp.js
+++ b/node_modules/@electron/rebuild/lib/src/module-type/node-gyp.js
@@ -46,6 +46,10 @@ class NodeGyp extends _1.NativeModule {
     }
     async buildArgsFromBinaryField() {
         const binary = await this.packageJSONFieldWithDefault('binary', {});
+        let napi_build_version;
+        if (binary.napi_versions) {
+          napi_build_version = this.nodeAPI.getNapiVersion(binary.napi_versions)
+        }
         const flags = await Promise.all(Object.entries(binary).map(async ([binaryKey, binaryValue]) => {
             if (binaryKey === 'napi_versions') {
                 return;
@@ -60,6 +64,9 @@ class NodeGyp extends _1.NativeModule {
                 .replace('{arch}', this.rebuilder.arch)
                 .replace('{version}', await this.packageJSONField('version'))
                 .replace('{libc}', await detect_libc_1.default.family() || 'unknown');
+            if (napi_build_version) {
+              value = value.replace('{napi_build_version}', napi_build_version)
+            }
             for (const [replaceKey, replaceValue] of Object.entries(binary)) {
                 value = value.replace(`{${replaceKey}}`, replaceValue);
             }

This issue body was partially generated by patch-package.

@Nantris
Copy link

Nantris commented Apr 6, 2023

@Sara2009 huge huge thanks for diving into this and sharing the solution.

For anyone not familiar with patch-package, the name of the file you put those contents into should be (assuming using @electron/[email protected]) - @electron+rebuild+3.2.10.patch

@MarshallOfSound
Copy link
Member

Is there a PR for this or are people just using patch package and not sending the fix upstream?

@Nantris
Copy link

Nantris commented Apr 6, 2023

@MarshallOfSound thanks for your speedy reply - I'll submit one once I am able to confirm this works on an ARM64 machine as well. Right now I can only confirm it fixed my issue for x64 - although I presume this is a universal-friendly solution.

mmaietta added a commit to mmaietta/electron-rebuild that referenced this issue Apr 12, 2023
…g `binary.napi_build_versions` in package.json. Fixes: electron#554
@mmaietta
Copy link
Contributor

@MarshallOfSound finally got the tests working across mac, linux 12/14/16, and windows. Its tests cover both napi_build_versions and libc macros.

PR is ready for review: #1078

@nikhilro
Copy link

Running into this. Just wanted to bump on getting the PR merged in. Thank you 🙏

MarshallOfSound pushed a commit that referenced this issue Apr 22, 2023
* 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
@nikhilro
Copy link

Hey @MarshallOfSound, mind restarting the CI for the merged fix.

Timed out before releasing: https://github.com/electron/rebuild/runs/12949338516

@continuous-auth
Copy link

🎉 This issue has been resolved in version 3.2.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

davej added a commit to ToDesktop/electron-builder that referenced this issue Apr 25, 2023
Includes upstream fix for ["Incorrect folder name when rebuilding "sqlite3" for arm64/x64 architecture"](electron/rebuild#554)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
10 participants