-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
SUPPORT_BIG_ENDIAN=1
causes internal compiler error ReferenceError: maybeExport is not defined
#21751
Comments
It looks like that function was renamed in #21439 but that call was overlooked. I guess we don't have any testing for that setting :-/ Can I ask what your target is that you are using that option? |
Of course! Yarn compiles libzip to WASM for Node.js and needs to support big-endian systems. We don't have access to a big-endian system so our CI emulates one while Node.js CITGM validates that it works on a real machine. |
Thanks! Thats good to know. I didn't know yarn was using wasm like this. BTW, can I ask why you/they didn't just go with a native node binary? What is the advantage they/you seen in using wasm under node? (I'm not saying there are not advantages, I'm just wondering why you made the choice). |
The main reason is reproducibility. We need the output of libzip to be identical on all systems since we checksum the zip files to see if they've been tampered with or corrupted. It's also easier to compile to WASM once and use it everywhere Node.js works than to make a native Node.js addon for each and every architecture, platform, and libc implementation that Node.js runs on. |
I see, I'm not very familiar with node addon. Does that mean that Also, just to confirm, the I guess we should add more testing of this setting if folks like you are depending on it so much. Hopefully it doesn't come with too much of a performance hit. |
Compile on install can be done but then you risk the host system not having the required tools or correct version of tools to build the library. Some npm packages only ship pre-compiled versions and fail if it doesn't have one for the host and Node.js version, while some fall back to compile on install. It's easier for us to ignore all that and compile once to WASM.
Yes.
When support for big-endian was added to Yarn the benchmarks indicated almost no impact on performance yarnpkg/berry#3669 (comment). |
Thanks for the useful data points! |
Starting with version 3.1.55 compiling with
-s SUPPORT_BIG_ENDIAN=1
fails withReferenceError: maybeExport is not defined
.Version of emscripten/emsdk:
Failing command line in full:
Full link command and output with
-v
appended:The text was updated successfully, but these errors were encountered: