fix: use prebuild correctly, electron support

[ThaUnknown]

I'm attempting to run this library on electron, to do that the package needs to define a "prebuild" command, however to be honest I'm loosing my shit, as even with these changes the lib works fine on NodeJS but still emits a "fatal error" on electron

the way electron handles this is quite simple, it just runs the prebuild command with specific configs, and this is the first native library I used that doesn't work on electron, and errors when calling any of the methods in the library

to rebuild this library electron simply does electron-rebuild -f -w yencode, which uses the @electron/rebuild library.

I honestly just want help with this, because this is the only functional yencode library in the world, but it simply refuses to work on electron

[ThaUnknown]

hooray for my sanity! i figured it out, it was the realloc that was bad! this works, but I'm uncertain if its the correct way of doing it

[animetosho]

Unfortunately I haven't built for Electron so there's not much I can help you with.

Regarding the code changes:

• it'd be preferred if you could not mandate a new dependency. I don't mind if you conditionally include it, but a standard NPM install shouldn't install any dependencies
• do you know if Electron's buffers don't like externally managed memory? Your change introduces a memory copy; if there's no way to avoid this for Electron, I don't mind it being conditionally performed, that is, NodeJS builds don't do the copy but Electron builds do

Thanks for the contribution!

[ThaUnknown]

node-gyp-build is pretty much the standard for native addons, and this is unironically the first one I've seen that doesn't use it, it's not that big and handles a LOT of edge cases and fuckeries with building and bundling

do you know if Electron's buffers don't like externally managed memory?

I'm not sure, I've worked with native modules quite a bit, across many platforms and this is the first time I've had this problem, this is pretty much the "minimal amt of code" required to get it working. I'll see if there are "lower cost" solutions to this, but your project constantly hangs my entire PC when MSVC interacts with it in any way, which is also unique to this project, so my.... enjoyment of trying to fix these problems hasn't been high.

Also please prebuild, prebuildify explains why quite well: And it also omits the woes of compiling said code which VERY often is problematic, especially on windows. I laid the groundwork for this, but haven't gone and prebuild all the platforms, as then they'd be included in the git repo which shouldn't be done. Prebuilds are also effectively a standard, and as you can see required for "weird" nodeJS platforms like Electron, NWjs, Android and alpine. Which is where the node-gyp-build dependency comes from

[ThaUnknown]

yeah it doesn't seem possible to do this zero-copy anymore because of changes in v8: https://www.electronjs.org/pt/blog/v8-memory-cage

[ThaUnknown]

yeah the way you want to do this is to check if the v8 trap is enabled, which I can't find how to do, and only do memcpy then, or do something like this

  Local<Object> nodeBuffer = NEW_BUFFER(arg_len);
  unsigned char *result = (unsigned char*)node::Buffer::Data(nodeBuffer);
  
  // Decode directly into V8-allocated memory (TRUE zero-copy!)
  size_t len = decode(isRaw, (const unsigned char*)node::Buffer::Data(args[0]), result, arg_len, NULL);
  
  // Return the buffer - even if oversized, it's still zero-copy
  // The extra unused bytes at the end are just wasted memory
  // Alternatively, you could create a slice view in JavaScript
  RETURN_VAL(nodeBuffer);

but yeah this wastes a bit of memory, it would be best to allocate directly to a node::Buffer or uint8array preferably as buffers are bad, rather than to a char*, but that's above my capabilities

[animetosho]

handles a LOT of edge cases and fuckeries with building and bundling

From what I can tell, most of the code in it is just trying to handle prebuilds and runtime resolution for prebuilt binaries. So I can't really see any edge cases, other than ones it introduces via prebuilds.
For now, the only edge case we need to deal with here is Electron.

From a quick look at the prebuildify source, it seems that all it does is send some extra flags to node-gyp, probably to get it to target the Electron ABI.
Does a node-gyp rebuild --runtime=electron --dist-url=https://electronjs.org/headers command give you a workable binary?

your project constantly hangs my entire PC when MSVC interacts with it in any way, which is also unique to this project

I'm guessing MSVC spawns a cl process for each CPU thread you have, and saturates all your cores. If this is the case, you can alleviate this by lowering the priority (or restrict affinity) of the command prompt which launches the build to avoid this issue (spawned processes will inherit the lowered priority).

Also please prebuild

Thanks for the suggestion.

It doesn't look like prebuild handles backwards compatibility with glibc, which makes Linux prebuild kinda iffy. I suppose I could just restrict it to Windows+MSVCRT, which is likely the most useful.
The next issue would be that this module doesn't use N-API, which potentially means many builds for the NodeJS versions as V8 doesn't have a stable ABI. I actually do kinda have a N-API variant of the code, but haven't found much reason to use it yet - I suppose this would be one.

So I could do something for Windows, but I'm not sure how useful it is generally - the only project really using it at the moment is Nyuu, which statically links in the library during a build. Thus most Windows users never actually build this module, only devs would.

yeah it doesn't seem possible to do this zero-copy anymore because of changes in v8

Ah I see - thanks for the link.

Functions like encode and decode can probably be removed in such cases - there's already encodeTo/decodeTo which target an existing buffer, and the incremental variants allow for existing buffers.
I'd just need some way to detect that it's building for such a platform to toggle functionality - do you know if there's some compile-time way to detect if sandboxed pointers are enabled?

[ThaUnknown]

really see any edge cases, other than ones it introduces via prebuilds.

pretty much yep, but other tooling uses that logic to compile binaries for very non-standard ecosystems, aka electron, android[nodejsmobile] and bundlers like webpack, vite etc, it's for the most part a standard, but it's simply a suggestion.

probably to get it to target the Electron ABI.

yep, but then you can simply pass some flags to it, and have it use your own ABIs like so, which is also usually done via the prebuild command, again, it's just things tooling developers agreed on, I don't rly get why, I just use the tooling tbf

MSVC spawns a cl

no clue it's some weird git and MSVC interaction, no matter, I got past it

do you know if there's some compile-time way to detect if sandboxed pointers are enabled?

no clue, I looked around but couldn't find anything, I saw v8 flags for toggling it, but not for reading it's state, sorry

Functions like encode and decode can probably be removed in such cases - there's already encodeTo/decodeTo which target an existing buffer, and the incremental variants allow for existing buffers.

yeah you'd need to do that in your JS code, since you don't re-export sub functions for stuff like from_post etc, and uuuh, the "legacy" JS is not very fun to work with for me

I guess this PR is mostly a "heads up these things are problematic in actual web/node development" rather than actual solutions, so feel free to treat this as a discussion/complaint, and make your own decisions on top of it, I'm fine with using my fork for this.

[animetosho]

So I've made changes to cater for the external buffer issue, and tried to auto-detect when to use it. I don't have any setup to test it though.

Does the node-gyp rebuild --runtime=electron --dist-url=https://electronjs.org/headers command work correctly for you?
If so, I can take a look at what can be done to make the build easier - it might just be putting that as a script in package.json

[ThaUnknown]

see, this is why prebuilds are nice, I can't build this on windows, as this version isn't published on npm, and using gh to load it, creates a massive "codeload" url, which exceeds windows's path limit and crashes ^^, anyways I'll simply modify the files locally, but yeah, trust me when I say I'm not making up these problems lol

[ThaUnknown]

to answer your question, the code you pushed to master works on electron, but the command you provided doesn't, ignoring the fact that it doesn't work with electron's build system and I needed to manually overwrite the files for it to even attempt to load, where it also didn't work

I needed to add prebuildify as a prebuild command, after which electron-builder rebuilt it itself via @electron/rebuild, after which it worked correctly, so I guess on top of your changes all that's needed is the prebuildify commands, and my requirements of node-gyp-build shouldn't be needed, but I might be wrong, there could be some form of caching happening here that I'm overlooking, as the build systems for these things are way too complex for me to understand, so this is only like an 85% certainty

TLDR your new code works, the command for rebuild electron doesnt, prebuildify is still needed as a dev dependency

[animetosho]

Thanks for testing that and confirming the code change works.

The NPM install command just uses node-gyp to build modules, as does prebuild and electron-rebuild, so it's odd that those work for you but not node-gyp.

On Windows, I tried the command electron-rebuild -f -w yencode -v 38.2.2 on the Git code, without any modifications/prebuilds, and it seems to build successfully (√ Rebuild Complete). Is this step failing for you, or does it successfully build but fail somewhere later?

From what I can tell, if you use prebuild, electron-rebuild doesn't build anything - it just uses the prebuilt binary. So if prebuild is working for you, but not electron-rebuild without it, it sounds like some difference in build invocation between the two.

Sorry for persisting with this line of investigation - I'm currently trying to rule out some incorrect build configuration/setup on your system as being the issue here.

[ThaUnknown]

√ Rebuild Complete

No it fails during runtime, with a classic NAPI "Fatal Error" and no extra information, you can do ELECTRON_RUN_AS_NODE=1 npx electron [doesn't work in windows CMD because yay!] if you want to run node's REPL from electron and try importing the module, note that paths matter, that's where it will fail

From what I can tell, if you use prebuild,

it check's the module's dependencies to see if it should be rebuilt, then rebuilds them

https://github.com/electron/rebuild/blob/main/src/module-rebuilder.ts#L149-L151 it is likely a difference in invocation

Sorry for persisting with this line of investigation

No, no, you're doing the right thing, I'm clearly not perfectly aware

[animetosho]

No it fails during runtime, with a classic NAPI "Fatal Error" and no extra information

I see - that's annoying. I can't seem to reproduce though.

On a Windows machine with NodeJS v22.20.0, Python v3.11.3 and Visual Studio 2022 installed, I created an empty folder and ran the following commands inside:

npm install --save-dev [email protected]
npm install --save-dev @electron/rebuild

Inside the node_modules folder, I created a folder yencode and put the contents of this repo in there (unmodified).
I then ran npx electron-rebuild -f -w yencode, and it rebuilds successfully.

Outside node_modules, I replaced the package.json with:

{
  "name": "my-electron-app",
  "version": "1.0.0",
  "description": "Hello World!",
  "main": "main.js",
  "scripts": {
    "start": "electron .",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "Jane Doe",
  "license": "MIT",
  "devDependencies": {
    "@electron/rebuild": "^4.0.1",
    "electron": "^38.2.2"
  }
}

and created a main.js:

y=require('yencode')
b=y.encode(Buffer.from('abc'))
console.log('test', b)

Then executed npx electron .
This gave me test <Buffer 8b 8c 8d>, which is what I'd expect.

I presume this last bit doesn't work for you?

you can do ELECTRON_RUN_AS_NODE=1 npx electron [doesn't work in windows CMD because yay!]

The Windows syntax for that would be

set ELECTRON_RUN_AS_NODE=1
npx electron

Though for me, the application just exits - I don't get a REPL (not sure if this is what you meant by "doesn't work").
Changing the last line to npx electron . works, with the application exiting after the console.log; without this env var, the application hangs after printing.

I haven't tried prebuild yet, but this seems to build+run fine with electron-rebuild for me. I'm not sure why it's breaking for you - only thing that comes to mind could be some version mismatch somewhere maybe?

---

So I've tried deliberately making bad builds to see if I can replicate your behaviour. If I issue just a node-gyp rebuild on the module, this will build it for NodeJS instead of Electron, and if I try running the above via Electron, I do get a stack trace:

App threw an error during load
Error: The module '\\?\C:\electron\node_modules\yencode\build\Release\yencode.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 127. This version of Node.js requires
NODE_MODULE_VERSION 139. Please try re-compiling or re-installing
the module (for instance, using `npm rebuild` or `npm install`).
    at process.func [as dlopen] (node:electron/js2c/node_init:2:2617)
    at Module._extensions..node (node:internal/modules/cjs/loader:1874:18)
    at Object.func [as .node] (node:electron/js2c/node_init:2:2617)
    at Module.load (node:internal/modules/cjs/loader:1448:32)
    at Module._load (node:internal/modules/cjs/loader:1270:12)
    at c._load (node:electron/js2c/node_init:2:17993)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:244:24)
    at Module.require (node:internal/modules/cjs/loader:1470:12)
    at require (node:internal/modules/helpers:147:16)

I get similar stack traces (with different error message) if I build with --arch=ia32 or just completely delete the .node file.
I also tried making the library dynamically link with the C runtime, and forcing it to load a bad DLL, but I still get a printed stack trace (Error: The specified procedure could not be found.).
I also tried building to an incorrect version of Electron (npx electron-rebuild -f -w yencode -v 37.0.0), still get a stack trace.
I noticed that the module has a runtime dependency on node.exe, but if I remove node.exe and run via node_modules\electron\dist\electron.exe . (with a valid .node file), it seems to run fine.

So I can't figure out what might be your issue so far.

[ThaUnknown]

yeah using electron-rebuild works, as I said here, I'm running the library inside a utilityProcess, which is likely why electron shits itself and doesn't print a stack trace

it's likely that I'd get:

Error: The module '\\?\C:\electron\node_modules\yencode\build\Release\yencode.node'
was compiled against a different Node.js version using

also as I said in the same comment, the new code works, as long as the proper rebuild command is used, running it manually is an option, but using one of the 3 libraries mentioned previously in your dev dependencies would be optimal, as that would do it automatically during electron's build, instead of having to do it ahead of time

I presume this last bit doesn't work for you?

nope, this works, its the node-gyp rebuild --runtime=electron --dist-url=https://electronjs.org/headers command which you asked about that didn't

Though for me, the application just exit

yeah that's exactly the problem on windows, works fine in linux, not important tho

there also seem to be some issues on macOS with this library, but I'll need to get back to you on that one,

~~EDIT: ah yeah, since you configure a single "Release" path, and don't have sub-paths for each platform/arch separately, building for macOS simply overrides itself, since it builds x64, then arm64, but arm64 overrides the x64 build, which means you cannot create an universal build~~

~~this is what require('node-gyp-build')('./') is for, since that forces you to import for a given arch/platform, rather than a single output, fun...., this ends up with a~~

[ThaUnknown]

this lib also doesn't compile correctly for macOS on arm64, you end up with a error: unknown target CPU 'apple-m3' caused by -march=native

adding

      ['OS=="mac" and enable_native_tuning!=0', {
        "conditions": [
          ['target_arch=="x64"', {
            "cflags": ["-march=haswell"],
            "cxxflags": ["-march=haswell"],
            "xcode_settings": {
              "OTHER_CFLAGS": ["-march=haswell"],
              "OTHER_CXXFLAGS": ["-march=haswell"],
            }
          }],
          ['target_arch=="arm64"', {
            "cflags": [],
            "cxxflags": [],
            "xcode_settings": {
              "OTHER_CFLAGS": [],
              "OTHER_CXXFLAGS": [],
            }
          }]
        ]
      }],

seems to solve it, tho no clue if that's the correct solution, but at the very least it functions on both x64 and arm64 on mac

[animetosho]

yeah using electron-rebuild works, as I said here

So I'm a little confused - your previous comment sounded like you needed to add prebuild before electron-rebuild for it to work, implying that electron-rebuild by itself wouldn't work. If electron-rebuild without prebuild works, why the need for prebuild?

as long as the proper rebuild command is used

So is your build tool not issuing the proper rebuild command? If so, that sounds like an issue with the build setup, which is completely external to this project.

From the striked out text, I get the sense that you're building for multiple targets. One thing prebuild does differently is that is produces multiple target specific files, whilst this module always places the output at build/Release/yencode.node. My hunch is that this last bit is causing you issues because your build setup is presumably running a bunch of stuff in parallel, trying to write multiple targets to the same file, and you get mixed up outputs (e.g. a Windows build tries to pull a MacOS binary).
If my guess is correct, this sounds like an issue with your build setup, not an issue with node-yencode - you'll either need to copy the folder around to ensure independent non-conflicting builds, or make sure one build completes before starting the next. prebuild is a hacky workaround to such a build system, but it's not really a hack I can support (someone else will have a differently broken build system that'll prefer a different hack).

caused by -march=native

For building redistributable binaries, you should disable native targeting. Passing --enable_native_tuning=0 to the node-gyp does it, or if your build tools can't pass config variables to node-gyp, you can monkey patch the "enable_native_tuning%": 1, line in binding.gyp

For MacOS, maybe I can look at something to help with building a fat/universal binary; I'm not sure node-gyp is particularly good at dealing with it though. I also don't have a Mac, which makes supporting MacOS difficult for me.

[ThaUnknown]

as mentioned here https://github.com/animetosho/node-yencode/pull/15#issuecomment-3388679340 without one of those 3 libs in Dev dependencies the developer needs to manually rebuild the package because electron-builder won't detect it as native and won't rebuild it on compile, this is true for other tool chains too

yes it's not issuing a rebuild command because it has no way of knowing that it's a native module again, here: https://github.com/electron/rebuild/blob/main/src/module-rebuilder.ts#L149-L151

yeah it's striked out because it's incorrect but I left it in in case you had already read it, it's wrong i should have removed it

For building redistributable binaries

yeah... that's now how this works tbf, you run npm install, it fails, you can't do anything extra, which is why I added that config to the bindings.gyp which resolves the issue and builds it correctly for arm, this isn't for redistributable binaries, this is simply for running on an arm Mac, not electron related this time

don't worry about the universal builds, as you said it was a build config mistake on my end that I overlooked accidentally and I was being very dumb

[ThaUnknown]

I'll update this PR with fixes you should be satisfied with tomorrow

[animetosho]

yes it's not issuing a rebuild command because it has no way of knowing that it's a native module again, here: https://github.com/electron/rebuild/blob/main/src/module-rebuilder.ts#L149-L151

Sorry for being daft, but my understanding of the linked code is:

• if a prebuilt binary is found, and the user hasn't explicitly forced a rebuild, skip the build step and assume it succeeded (return true)
• otherwise do a rebuild (return await this.rebuildNodeGypModule(cacheKey))

Since this project doesn't integrate any prebuild, the first check is false and a rebuild is always performed.
This doesn't match up with "not issuing a rebuild command".

NPM clearly knows it's a native module and knows that it needs to be built, so I'd expect electron-rebuild to be able to do the same.

If I take my previous example, delete the yencode build and add yencode as a dependency in package.json, issuing a npx electron-rebuild does cause yencode to be rebuilt. This suggests that electron-rebuild does indeed know yencode is a native module which needs to be rebuilt.

you run npm install, it fails, you can't do anything extra, which is why I added that config to the bindings.gyp which resolves the issue and builds it correctly for arm, this isn't for redistributable binaries, this is simply for running on an arm Mac, not electron related this time

Ah I see, thanks for raising the issue.
There's actually a check to detect compilers with broken -march=native, so it's odd that the check passed, but then fails later.
Are you able to try running c++ -MM -E src/common.h -march=native; echo $? and post the output?
I might try changing the check command - could you also try c++ -c -x c++ src/common.h -march=native -o /dev/null; echo $? as well?

[ThaUnknown]

If I take https://github.com/animetosho/node-yencode/pull/15#issuecomment-3392986108, delete the yencode build and add yencode as a dependency in package.json, issuing a npx electron-rebuild does cause yencode to be rebuilt. This suggests that electron-rebuild does indeed know yencode is a native module which needs to be rebuilt.

that is.... odd, highly odd, that's not what happens on my end, I guess the default config for it must differ in electron-builder, it didn't rebuild at all which is where I started this entire issue, and with the prebuild command it did, I'll need a break from this for a while, because working with native code makes my blood boil, but I'll see if I can track down why this happens

There's actually a check to detect compilers with broken -march=native, so it's odd that the check passed, but then fails later.

I noticed that too, but it doesn't seem to work, it sometimes caused errors when I tried fixing it, so I wasnt sure

Are you able to try running c++ -MM -E src/common.h -march=native; echo $? and post the output?

I don't own any apple devices, I was doing this via gh actions for my production app, I could do the same and create a separate repository for testing this thing via actions, but that simply means doing ping-pong with you about things you'd know more about, so I think you should try it via actions yourself with electron-rebuilds --arch to specify x64 or arm64 as either way we'd both need to do this from scratch, and it saves us the time of "hey does this work? how about this? try this?", I'd do it if I had a mac machine but I do not

[animetosho]

Try enabling debugging to see if it's looking at yencode at all:

set DEBUG=electron-rebuild
npx electron-rebuild

The output I get includes:

electron-rebuild ignoreModules [] +3ms
electron-rebuild rebuilding with args: C:\electron-rebuild 38.2.2 win32 x64 Set(0) {} false https://www.electronjs.org/headers [ 'prod', 'optional' ] false +0ms
electron-rebuild exploring C:\electron-rebuild\node_modules\yencode +0ms
electron-rebuild identified prod deps: Set(1) { 'yencode' } +10ms
electron-rebuild scanning: C:\electron-rebuild\node_modules +2ms

[... scans other folders in node_modules...]

electron-rebuild checking yencode against [] +110ms
electron-rebuild rebuilding yencode with args [
'node',
'node-gyp',
'rebuild',
'--runtime=electron',
'--target=38.2.2',
'--arch=x64',
'--dist-url=https://www.electronjs.org/headers',
'--build-from-source',
'--verbose'
] +0ms
- Building module: yencode, Completed: 0  electron-rebuild built via node-gyp: yencode +0ms
electron-rebuild searching for .node file C:\electron-rebuild\node_modules\yencode\build\Release +9ms
electron-rebuild testing files [
'.forge-meta',       'crcutil.lib',
'obj',               'yencode.exp',
'yencode.iobj',      'yencode.ipdb',
'yencode.lib',       'yencode.node',
'yencode.pdb',       'yencode_armcrc.lib',
'yencode_avx.lib',   'yencode_avx2.lib',
'yencode_clmul.lib', 'yencode_clmul256.lib',
'yencode_neon.lib',  'yencode_pmull.lib',
'yencode_rvv.lib',   'yencode_sse2.lib',
'yencode_ssse3.lib', 'yencode_vbmi2.lib',
'yencode_zbkc.lib'
] +2ms
electron-rebuild found .node file C:\electron-rebuild\node_modules\yencode\build\Release\yencode.node +6ms
electron-rebuild copying to prebuilt place: C:\electron-rebuild\node_modules\yencode\bin\win32-x64-139 +9ms

electron-rebuild looks for dependencies in package.json, then prints the 'identified prod deps' debug line showing what it's identified as dependencies. If it's not listed there, ensure yencode is listed as a dependency, or try debugging their code to see why it's missed.

If you haven't done so already, it's best to test with a minimal example to eliminate noise that your other stuff might be introducing.

[animetosho]

I can't reproduce your macOS compilation issue via Github Actions unfortunately. It might be dependent on the hardware GHA decides to run the build on (apple-m3 seems to have been added in Clang 18.1.0, whilst the base image has Clang 16.0, which supports up to apple-m2) so hard to test reliably.

I've made the change regardless, which hopefully resolves the issue. Let me know if you still encounter the macOS compilation failure.

[ThaUnknown]

god, this entire thing is making me look inept, but at least I have workflow logs to back me up 1, so it's odd it didn't happen for you

[ThaUnknown]

yeah that doesn't seem to have fixed it: https://github.com/hayase-app/electron/actions/runs/18466506240/job/52609666071#step:9:395

[animetosho]

Ah, I think I see what's going on, thanks for the logs.

Could you see if the latest changes fixes it for you?

(note that you probably do want to disable native tuning for your builds - native tuning is intended for when you build and run on the same machine; if they aren't the same, the code might not work)

[ThaUnknown]

nope

[animetosho]

Dang, thanks for testing that.

The build logs show that host_arch is incorrectly set to x64. I did some investigation, and suspect it's a bug in electron-rebuild, which I've reported here.

[ThaUnknown]

VERY weird that the issue doesn't occur when I do this: https://github.com/animetosho/node-yencode/pull/15#issuecomment-3393741369

also compiles it for the correct arches

[animetosho]

Your change completely neuters native targeting on MacOS, so it's no surprise it works.

My change tries to detect if a cross build is being performed and disable native targeting accordingly. This is done by comparing the host and target arch - if they don't match, you're definitely not targeting the host machine, so -march=native makes no sense.
Unfortunately electron-rebuild always specifies x64 as the host arch, so when you're not building on an x64 machine, the detection can fail.

As I suspect electron-rebuild isn't exactly fast at responding to reports, I've added a workaround which just disables native targeting when building for Electron. Hopefully this solves it for you.