electron-builder
electron-builder copied to clipboard
fix(linux): native modules overwritten with wrong arch
Fixes: #7608
Description
When building an app for multiple Linux architectures, native modules are hardlinked to the modules in appDir/node_modules/**/*.node if CI env is set.
When the builder processes the first arch, it works fine. The second arch overwrites appDir/node_modules/**/*.node. Since these files are hardlinked to linux-*unpacked/resources/app.asar.unpacked/node_modules/**/*.node it overwrites native modules for all archs, so now the first arch distributive is broken.
FileCopier in builder-util/out/fs copies files by default, but once it detects a CI environment, it switches to hadlinking mode instead.
Example package.json which reproduces the issue
{
"name": "builder-arm64-linux-issue",
"version": "1.0.0",
"main": "index.js",
"scripts": {
"build": "env CI=1 electron-builder --linux --x64 --arm64 && find dist/linux-unpacked -name realm.node -exec file {} \\;"
},
"build": {
"linux": {
"target": {
"arch": ["x64", "arm64"],
"target": "dir"
}
}
},
"devDependencies": {
"electron": "^29.1.0",
"electron-builder": "^24.13.3"
},
"dependencies": {
"realm": "^12.6.2"
}
}
🦋 Changeset detected
Latest commit: 702a291e7f45fa2cd5e104a54bd587410ca83db5
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 8 packages
| Name | Type |
|---|---|
| app-builder-lib | Patch |
| dmg-builder | Patch |
| electron-builder-squirrel-windows | Patch |
| electron-builder | Patch |
| electron-forge-maker-appimage | Patch |
| electron-forge-maker-nsis-web | Patch |
| electron-forge-maker-nsis | Patch |
| electron-forge-maker-snap | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
Deploy Preview for car-park-attendant-cleat-11576 ready!
| Name | Link |
|---|---|
| Latest commit | 702a291e7f45fa2cd5e104a54bd587410ca83db5 |
| Latest deploy log | https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/65e9c4955c328900082c2449 |
| Deploy Preview | https://deploy-preview-8107--car-park-attendant-cleat-11576.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
I'm wary of this change as I'm assuming the owner/author of this project had a reason for not having hard links always disabled.
For your local project, why not use env var USE_HARD_LINKS=false?
I'm wary of this change as I'm assuming the owner/author of this project had a reason for not having hard links always disabled. For your local project, why not use env var
USE_HARD_LINKS=false?
It's faster to use hardlinks instead of copying. But this change affects only *.node files inside app.asar.unpacked, so it can't impact performance too much. Anyway, it would be nice to ask @develar whether there are some pitfalls or hadrlinks are used only for speed reasons.
Sure, I use this env as a workaround. But it took pretty much time for me to find the reason of this issue, so it would be nice to include this fix by default.
Maybe it makes sense to disable hardlinking on the Linux build only since there is no such issue on macOS build?
I really need @develar to chime in before moving forward with this PR. I do understand the need to resolve this issue, but I don't have the historical context to understand the reasoning for it to be disabled by default.
@develar ?
@thebalaa can you still reproduce this on next v25.0.1? I'm trying to get a local project set up with the package.json you specify but I think I'm doing something wrong. Would you be willing to put together a sample minimum reproducible repo and I can take a deeper look if the issue still persists?
How does anyone contact @develar? I don't think he's been actively checking out this repo since 2022. @mmaietta Also, one more thing, I think this repo is community maintained as of 2024 and the current maintainers have the full rights to make new changes since stuff in the software world changes and we have to adapt according to that.
Even though there might be a very good reason behind whatever was implemented in this repo during the time when it was actively owned and maintained by @develar, it does not necessary mean imo that it should always stay like this. AFAIK, @develar did not also want autoupdates on targets other than AppImage, which @mmaietta implemented and was merged anyways.
There are a lot of things in this repo that are have a very unclear and vague "why" to them. That is why imo this repo deserves to move forward, and of course, if issues are discovered, you can always undo a change (before a release).
Can you try this on latest electron-builder? Should be 25.0.5 at the time of writing this. If a minimum reproducible repo could be provided, I'd be happy to investigate further as well
@mmaietta I will try it next week.
It looks like it's fixed in 25.0.5 or earlier