-
Notifications
You must be signed in to change notification settings - Fork 175
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
Comments
I can confirm the issue when trying to compile realm-js for Node on a Mac M1 with |
Same happening here... |
Any update on this ?? |
I have the same issue. |
Hi! 👋 Today I used patch-package to patch 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. |
@Sara2009 huge huge thanks for diving into this and sharing the solution. For anyone not familiar with |
Is there a PR for this or are people just using patch package and not sending the fix upstream? |
@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. |
…g `binary.napi_build_versions` in package.json. Fixes: electron#554
@MarshallOfSound finally got the tests working across mac, linux 12/14/16, and windows. Its tests cover both PR is ready for review: #1078 |
Running into this. Just wanted to bump on getting the PR merged in. Thank you 🙏 |
* 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
Hey @MarshallOfSound, mind restarting the CI for the merged fix. Timed out before releasing: https://github.com/electron/rebuild/runs/12949338516 |
🎉 This issue has been resolved in version 3.2.13 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Includes upstream fix for ["Incorrect folder name when rebuilding "sqlite3" for arm64/x64 architecture"](electron/rebuild#554)
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:
I believe
{napi_build_version}
should be a number (probably3
). What could be the reason for that?UPDATE
Same thing happens for "x64" build as well.
The text was updated successfully, but these errors were encountered: