Closed romansp closed 1 year ago
willemdjong
commented
1 year ago I ran into the same issue. I've this issue since I upgrade from electron-builder 23.6.0 to 24.4.0. It seems that prebuilt binaries are not correctly installed. In the signing process electron builder tries to sign a .node file for mac with a windows certificate.
willemdjong
commented
1 year ago @mmaietta can you have a look at this. It's the same I mentioned before https://github.com/electron-userland/electron-builder/pull/7465 It's easy to reproduce by cloning this repository: https://github.com/electron-react-boilerplate/electron-react-boilerplate Adding a native dependency (like https://github.com/m4heshd/better-sqlite3-multiple-ciphers), and try to package your windows application (with signing) from a mac machine.
Before I saw this line in my build logs: install prebuilt binary name=better-sqlite3-multiple-ciphers version=8.3.0 platform=win32 arch=x64 napi=
davej
commented
1 year ago We are also seeing this issue in 24.x of Electron Builder.
The issue seems to be that Electron Builder now leaves all *.node prebuilds for every platform in the distribution package. The prebuilds should be filtered so that only prebuilds that match the target platform/architecture are in the distribution package.
mmaietta
commented
1 year ago Hey guys, I'm traveling. So I genuinely don't know how prebuilds work tbh. I was under the impression electron/rebuild would automatically handle that. Can someone please post a screenshot of the folder structure where the .node prebuilds are all being included?
davej
commented
1 year ago Hey @mmaietta, thanks for the communication. Here is a clean project with just ffi-napi installed using 24.4.0.
.
├── darwin-x64
│ └── node.napi.uv1.node
├── linux-arm64
│ └── node.napi.uv1.armv8.node
├── linux-x64
│ └── node.napi.uv1.node
├── win32-ia32
│ └── node.napi.uv1.node
└── win32-x64
└── node.napi.uv1.nodeSo im trying to replicate what I believe would be the correct behavior on 23.6.0 (i.e. only the prebuilt library for the respective OS+arch) and I'm seeing that the app.asar.unpacked contains all the prebuilds for every OS+arch still. Tested this building Windows app on arm64 mac. Could anyone confirm that observation?
v24 didn't change anything about the asar packaging approach or logic, so I'm really confused how this issue just appeared from the migration to electron/rebuild.
Wondering if the most efficient approach is to just not unpack prebuilds that aren't for the app's OS+arch, then they wouldn't attempt to be signed?
Alternative ideas are also welcome :)
willemdjong
commented
1 year ago I cannot confirm this information definitively because the specific package I am currently using does overwrite the .node file based on the architecture. However, I can verify that it is still functioning as expected on version 23.6.0. The only discrepancy in the build logs is the inclusion of the following line:
install prebuilt binary name=better-sqlite3-multiple-ciphers version=8.3.0 platform=win32 arch=x64
This line appears after the process of
rebuilding native dependencies, including better-sqlite3-multiple-ciphers@8.3.0
Due to this difference, I am uncertain whether the electron-builder still installs the prebuilt binaries at all.
mmaietta
commented
1 year ago @willemdjong thank you for that info, that was immensely helpful. It appears that the previous Go binary was executing both prebuild-install cli manually and then compiling locally if something failed. Right now, electron-builder v24 is only set up with electron/rebuild (an oversight on my side, my sincere apologies. I really do wish I had the previous domain knowledge of the original maintainer of this project). I'm working to resolve this asap
Additional context, electron/rebuild was needed for the latest electron versions that required new C++ compiler flags IIRC, which AFAICT I can't configure via the Go binary.
What version of electron are you using?
willemdjong
commented
1 year ago @mmaietta Thank you for your response. We are currently using Electron version v24, and we have encountered an issue while trying to upgrade to Electron v25. To resolve this issue, we need to update electron-builder from v23 to v24. However, we encountered this specific issue during the process.
mmaietta
commented
1 year ago Okay, I'm going to attempt reverting the electron/rebuild migration in a new PR. I'm struggling to find a way to create a native module dependency mapping that I can execute the prebuild-install cli on for each native module. From another github comment, the need for electron/rebuild due to Cxx flags may no longer be needed? It appears that may be the case since you're using electron 24 with electron-builder v23.
Just to continue checking all my bases, what changes in v24 do you need that aren't in v23? (i.e. is it requiring electron/rebuild integration?)
willemdjong
commented
1 year ago I will provide you with more information about the issue later on, but for now, reverting this migration would be beneficial for us.
The sole reason for considering the upgrade to v24 is due to Electron upgrade, as v23 is functioning perfectly fine.
leolabs
commented
1 year ago I'm experiencing the same issue with the fsevents package. Downgrading back to electron-builder ^23 helped, but I'd love to use ^24 for some other fixes you've implemented. Let me know if there's anything I can do to help resolve this.
mmaietta
commented
1 year ago Trying to revert but running into a test failing that I can't identify why for windows builds on linux. https://github.com/electron-userland/electron-builder/pull/7668 Back to work now though and running low on free time
mmaietta
commented
1 year ago Alriiiiight, electron/rebuild migration has been reverted. Please try v24.6.1 🤞
willemdjong
commented
1 year ago @mmaietta thanks, this works for me!
davej
commented
1 year ago Thank you @mmaietta! 🙏
mmaietta
commented
1 year ago Wonderful!
romansp
commented
1 year ago Tried upgrading to 24.6.2 but still experiencing the same issue. Maybe it's something else on my side.
Can anyone confirm if their builds are working after the upgrade?
jgresham
commented
1 year ago Tried upgrading to
24.6.2but still experiencing the same issue. Maybe it's something else on my side.Can anyone confirm if their builds are working after the upgrade?
Having this issue too on 26.4.2
johnpyp
commented
1 year ago Thanks, this has fixed all of our builds. It also fixed a couple of other incidental issues, namely:
tobiasmuecksch
commented
1 year ago Yup. I've still got the issue on 26.4.2 too. In my opinion we should re-open this issue.
tobiasmuecksch
commented
1 year ago I just tried 24.6.1 and the error disappears. So it seems that the error was re-introduced in 24.6.2. Could you please reopen this issue?
johnpyp
commented
1 year ago @tobiasmuecksch what native module working for you? And what kind of binary is the .node file?
Could you try to reproduce in a minimum example repo?
Also, a couple possible fixes depending on your setup:
tobiasmuecksch
commented
1 year ago The Module is sqlite3. I'll provide you with more Information and a demo repo tomorrow 👍
willemdjong
commented
1 year ago We have encountered the same issue during our testing. However, the building and installation of install prebuilt binary name=better-sqlite3-multiple-ciphers version=8.5.0 platform=win32 arch=x64 are now functioning properly again.
Unfortunately, the build is currently crashing with the error message: 'Error: Could not detect abi for version 25.2.0 and runtime electron. Updating 'node-abi' might help solve this issue if it is a new release of Electron.' We have already opened an issue (https://github.com/m4heshd/better-sqlite3-multiple-ciphers/issues/52) for this particular problem in the 'better-sqlite3-multiple-ciphers' repository. However, we are uncertain whether the responsibility lies with this package or electron-builder to address and resolve this issue.
johnpyp
commented
1 year ago @willemdjong So, better-sqlite3 and better-sqlite3-multiple-ciphers, with their latest 8.5.0 release 5 days ago (looks like you're using 8.3.0) support electron v25 prebuilds, so if you update to that that should now work and get the right version.
As far as node-abi, the reason is because one of your downstream dependencies hasn't updated node-abi to the latest version which has the mapping information that says "electron v25 means use node abi version 116". To fix that, I'd recommend using override capabilities in whatever package manager you use to pin the version of node abi, then upgrade it when needed (or use a permissive version range).
Node abi is backwards compatible each release, and just adds new supported versions, so there isn't harm to this.
For example, with pnpm, I add
"resolutions": {
"node-abi": "^3.45.0"
},to my root package.json
tobiasmuecksch
commented
1 year ago @johnpyp
npmRebduild's default is true. I haven't disabled it so I assume, that it already was true. Just to be extra sure, I've added it to my configuration.sqlite3 wasn't updated in 4 months, I don't think that there are the needed prebuilds. The idea to create your own fork and providing the prebuilds is genius. I need to find out how to do that.This is my error stack:
But still, I'm confused by this strange error message Unrecognized file type:
Downgrading electron-builder to 23.6.0 fixes the issue for me :(
tobiasmuecksch
commented
1 year ago regarding better-sqlite3-multiple-ciphers I've been advised to stop cross building in this issue
johnpyp
commented
1 year ago @tobiasmuecksch Got it. Yeah so a couple things going on here:
Unrecognized file type error?The Unrecognized file type error is caused because electron-builder is attempting to sign a binary that isn't the same platform as it's expecting. So in this case, it's trying to sign a windows arm64 .node binary, instead of windows x64.
In 24.x, electron-builder started signing .node binaries when it didn't before (for unrelated reasons), so it's just scanning for any file ending in .node, which happens to be those. Those binaries it's referencing in sqlite3 wouldn't actually be used as far as I know, but since they're in the folder, they're getting picked up. Then electron-builder is failing to sign them for the reason mentioned above, but if they were skipped I think the app would work. I think @mmaietta could better comment on if a long term solution could be provided for this issue.
You have a couple options:
@electron/rebuild mode, which had a myriad of issues. I'd retry it. Note that better-sqlite3 (and the multiple-ciphers variant) has now pushed out 8.5.0 that has support for electron v25 prebuilds, which should fix other issues for you, and others. *.node files, for example you can make a beforeSign hook that ignores those files:
// This code was sourced from
// https://github.com/electron-userland/electron-builder/issues/6304#issuecomment-1057739612
const path = require("path");
const { doSign } = require("app-builder-lib/out/codeSign/windowsCodeSign");/**
@type {import("electron-builder").CustomWindowsSign} sign */ module.exports = async function sign(config, packager) { // Do not sign if no certificate is provided. if (!config.cscInfo) { return; }
const targetPath = config.path;
const ext = path.extname(targetPath);
// Don't sign .node files, since the logic doesn't quite understand different binary types yet that may be left over. // Could be improved through better introspection of the files. // PR that introduced signing .node files: https://github.com/electron-userland/electron-builder/pull/7421 if (ext === ".node") { return; }
await doSign(config, packager); };
You still should be able to, but these issues are something that you're going to run into regularly (which is the only reason I know my way around it decently). I stuck it out for a long time, but now just have dedicated build VMs for each os that I publish from with a standardized script. It simplifies the headache to have dedicated build machines, but obviously has that downside. It is possible cross-compile if you really want to, there isn't actually anything technically untenable here, it's just about getting the tooling hooked up right and interop-ing with the ecosystem. For me, it was worth the effort until recently.
What does not cross-compiling get you? -> Generally it'll mean that you won't have to care "much" about prebuilds, because you'll always be able to compile from source on the build machine if needed. A notable exception to this however is trying to make universal builds work on macos for intel and apple silicon - if your force build from source to avoid prebuild nonsense, electron-builder by default I think will not properly account for that (though I could be wrong on this, maybe it was my configuration)
tobiasmuecksch
commented
1 year ago @johnpyp Thank you so much, for this in-depth answer. Now I fully understand the issue at hand. Sadly, I'm also having issues with better-sqlite3-multiple-ciphers even on a real windows machine which is why I decided to stick with sqlite3.
Anyways; now having this information I can continue my work and find a new way to solve my build issues. I guess, this issue can be closed again.
johnpyp
commented
1 year ago @tobiasmuecksch I’m curious what issues you have now building with it even on a windows machine, as I have a very similar setup and it’s working fine for me.
As far as this issue goes, I think there’s still some clear work in the area of documentation, error message clarity and ironing out prebuild issues… I’d be interested if others can post about their issues too if they still have some on 24.6.2, especially non-cross-compile issues which seem strange.
tobiasmuecksch
commented
1 year ago @johnpyp I'm still investigating whether my issues really are electron-builder issues. For now, I found out that the build process starts working again, as soon as I remove the --arm64 flag. So I guess there are two underlying problems.
Removing the flag, also re-enables me to cross-build 😃
willemdjong
commented
1 year ago Thank you, @johnpyp, for your suggestions. I have encountered another issue now, which is related to https://github.com/electron-userland/electron-builder/issues/7683. It appears that everything is working correctly, except for the signing process, which is causing a problem. Based on other issues in electron-notarize, it seems that the issue should be related to electron-builder, as suggested in this particular issue."
@tobiasmuecksch, it's likely that the issue is related to multiple architectures.
johnpyp
commented
1 year ago @willemdjong I'm curious what happens if you specify mac.notarize.teamId in your electron-builder configuration and remove your afterSign configuration (and specify APPLE_APP_SPECIFIC_PASSWORD and APPLE_ID as environment variables) , as electron-builder now has its own handling for signing. This fixed my app's universal builds, and uses notarytool because it discriminates on teamId as the parameter. I haven't tried multi-target builds though.
@tobiasmuecksch Good to hear on the crossbuilds, yeah I didn't notice you were building for basically every architecture in your example post. I wonder why it can cross-build for the other architectures other than arm... maybe something like Apple's Rosetta needs to be triggered to get the signing to work..? Interesting issue.
mmaietta
commented
1 year ago Why did this break now but not on 23.6.0? In 24.x, electron-builder started signing .node binaries when it didn't before (for unrelated reasons), so it's just scanning for any file ending in .node, which happens to be those. Those binaries it's referencing in sqlite3 wouldn't actually be used as far as I know, but since they're in the folder, they're getting picked up. Then electron-builder is failing to sign them for the reason mentioned above, but if they were skipped I think the app would work. I think @mmaietta could better comment on if a long term solution could be provided for this issue.
.node files started being signed due to https://github.com/electron-userland/electron-builder/pull/7421
So it seems, all node files need to be signed for WDAC, but not all node files are able to be signed? Perhaps we need an additional signDlls var for Windows config but now also signNode, default false? I'm always hesitant in adding a new config var, but I don't see another way to about this then.
Actually, if going the route above, then ideally we could just create a signExts: string[] = ['.node', '.exe', '.dll'] config var
What are your thoughts?
johnpyp
commented
1 year ago Maybe the configuration could take the opposite approach of defaulting to maybe “skipping” signing .node files that don’t match the target arch (to catch most cases, and with a special case for macOS unified or other unified builds), and have a way to opt in to signing all .node files if needed.
The other thing I’m curious about is whether just straight up deleting those files might be a solution. Obviously if dependencies are doing weird things with them it’ll break… but for most of them, the path just won’t be triggered in the first place since the arches don’t match.
On Mon, Jul 24, 2023 at 9:18 AM Mike Maietta @.***> wrote:
Why did this break now but not on 23.6.0? In 24.x, electron-builder started signing .node binaries when it didn't before (for unrelated reasons), so it's just scanning for any file ending in .node, which happens to be those. Those binaries it's referencing in sqlite3 wouldn't actually be used as far as I know, but since they're in the folder, they're getting picked up. Then electron-builder is failing to sign them for the reason mentioned above, but if they were skipped I think the app would work. I think @mmaietta https://github.com/mmaietta could better comment on if a long term solution could be provided for this issue.
.node files started being signed due to #7421 https://github.com/electron-userland/electron-builder/pull/7421
So it seems, all node files need to be signed for WDAC, but not all node files are able to be signed? Perhaps we need an additional signDlls var for Windows config but now also signNode, default false? I'm always hesitant in adding a new config var, but I don't see another way to about this then.
What are your thoughts?
— Reply to this email directly, view it on GitHub https://github.com/electron-userland/electron-builder/issues/7652#issuecomment-1648219883, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE5LRZFFB4DIPMZ7NYXKDBLXR2N5BANCNFSM6AAAAAAZ7RQZTI . You are receiving this because you were mentioned.Message ID: @.***>
mmaietta
commented
1 year ago AFAICT, the packager class doesn't have access to what architecture or target it is packaging for during the signing stage, it just runs blindly the same flow. I took a look earlier on how to prune binaries for other architectures, but I couldn't find a way to do so.
I'm okay with default skip .node files to return back to previously working functionality. The WDAC sign-all requirement can be considered supplemental and require the user/developer adding .node to the signExts array.
johnpyp
commented
1 year ago Agreed - skipping behavior seems like a good default.
One side effect of the .node signing is that it "catches" these issues ahead of time (either cross-compilation issues or prebuild nonsense), maybe it's worth some "best-effort" heuristics warnings that will help people go in the right direction?
I don't know much about how native modules get resolved or what matters/doesn't matter, but it seems like the build/Release/examplefile.node is what's usually relevant... maybe we can pre-scan that arch (similar to what file does) and verify it matches?
mmaietta
commented
1 year ago AFAICT, build/Release is only present when node-gyp is actually run. prebuilds are in a different directory. The asar packager doesn't know anything about arch or target, just process.platform which doesn't apply to cross-platform builds.
Opened https://github.com/electron-userland/electron-builder/pull/7685 for signExts option with default empty array. .node and .dll can be provided there if needed. signDll option will be deprecated
jgresham
commented
1 year ago 24.6.3 fixed the issue for me. Thanks! @mmaietta
willemdjong
commented
1 year ago @mmaietta I've tested your latest changes, and both the M1 and Intel Mac builds are working perfectly fine. However, the Windows build is still experiencing crashes. I discovered that this is due to the 'build native dependency from sources' feature. By default, the buildDependenciesFromSource is set to false. However, after installing the prebuilt packages, it attempts to build native dependencies from source for Mac M1, Intel, and Windows. The crashes are occurring on Windows because I'm building on a Mac machine, which is not compatible with the Windows build process.
johnpyp
commented
1 year ago @willemdjong Yeah, that's expected behavior. The only way to cross compile properly is to have prebuilts ready for the different platforms, so compiling source means you won't be able to compile from source.
I'd recommend turning it off, and make sure that whatever native dependencies you have have prebuilts available for the version of electron you're using.
willemdjong
commented
1 year ago @johnpyp I find this behavior unexpected since the previous version (electron-builder v23) worked perfectly. The package better-sqlite3-multiple-ciphers includes prebuilt files, which are installed correctly. However, the build process still attempts to build the native dependency from sources for the name=better-sqlite3-multiple-ciphers version=8.5.0. This results in an error on Windows, stating "cannot build native dependency" with the reason being "prebuild-install failed with error," and building from sources is not possible because the platform or architecture is not compatible, causing an exit status 1.
mmaietta
commented
1 year ago @willemdjong can you create a minimum reproducible repo for this and I can take a look
Sounds like it may be worth opening a new issue/ticket for this though? Doesn't seem related to osslsigncode 'Unrecognized file type' OP
willemdjong
commented
1 year ago @mmaietta Managed to successfully upgrade to Electron v25 using the latest Electron Builder. Thanks once again for your help.
JaviOverflow
commented
9 months ago I'm experiencing this bug again in 24.12.0, downgrading to 24.6.3 solves this. Can you undo the change that causes this in a later version?
mmaietta
commented
9 months ago What's the error? Can you post with DEBUG=electron-builder env var?
Wondering if it's related to this change: https://github.com/electron-userland/electron-builder/pull/7998
In our electron app we're using
websocketpackage that includes native prebuilt .node modules. And we're signing both Mac and Windows x64 app versions on Mac.After upgrading to electron-builder v24 we can no longer sign Windows app on Mac.
It seems that https://github.com/electron-userland/electron-builder/pull/7421 introduced automatic code-signing for such modules, but it looks like
osslsigncodedoesn't support .node files and will produceUnrecognized file typeerror when trying to sign.Is code-signing Windows app on Mac still supported or we should consider splitting our build steps into dedicated pipelines for Mac and Windows?