Skip to content

[WasmFS] Make the Node backend optional for web builds #26608

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.

Sign up for GitHub

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

Merged
kleisauke merged 8 commits into from
Apr 4, 2026
+30 −5
Merged

[WasmFS] Make the Node backend optional for web builds #26608

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
26eaedc
[WasmFS] Make the Node backend optional for web builds
kleisauke Mar 24, 2026
d30f7de
Incorporate feedback
kleisauke Apr 2, 2026
eea0053
Adjust error message
kleisauke Apr 2, 2026
4143136
Remove unnecessary `eval()`
kleisauke Apr 2, 2026
4b86c1c
Avoid empty `main()`
kleisauke Apr 2, 2026
a2ab145
Incorporate feedback
kleisauke Apr 2, 2026
112bd63
Cherry-pick from PR #26609
kleisauke Apr 2, 2026
d0c6c63
Remove superfluous test
kleisauke Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions src/lib/libwasmfs_node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* SPDX-License-Identifier: MIT
*/
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about simply not allowing this file to be included at all it !ENVIRONMENT_MAY_BE_NODE?

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would lead to linker failures. For example, if I remove all these stubs and run ./test/runner other.test_wasmfs_nodefs_stubs, I see:

Details 
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_fstat_size
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_stat_size
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_open
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_open
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_close
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_close
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_read
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_write
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_ftruncate
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_truncate
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_get_mode
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_insert_file
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_insert_directory
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_symlink
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_rename
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_unlink
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_rmdir
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_readdir
wasm-ld: error: /home/kleisauke/emscripten/cache/sysroot/lib/wasm32-emscripten/libwasmfs-debug.a(node_backend.o): undefined symbol: _wasmfs_node_readlink
...

I also thought about moving these stubs to native code and using weak linkage (like how wasmfs_create_root_dir() is handled), but this seemed easier.

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I guess we should abort/assert in debug builds there though.


addToLibrary({
$wasmfsNodeIsWindows: !!process.platform.match(/^win/),
var WasmFSNodeLibrary = {
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JS names start with lowercase.

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a2ab145

$wasmfsNodeIsWindows: "!!process.platform.match(/^win/)",
Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a small drive-by fix. Previously, this would emit (when building on Linux):

var wasmfsNodeIsWindows = false;

in the JS output, which breaks when deploying these binaries on Windows.

As possible follow-up, we could also do:

Suggested change
$wasmfsNodeIsWindows: "!!process.platform.match(/^win/)",
$wasmfsNodeIsWindows: "!!globalThis.process?.platform.match(/^win/)",

This would allow a -sENVIRONMENT=node,web build to work on both Node.js and the web, using the same pattern as in other.test_wasmfs_nodefs_stubs, i.e.:

#include <emscripten/emscripten.h>
#include <emscripten/wasmfs.h>

EM_JS(bool, is_node, (), { return ENVIRONMENT_IS_NODE; });

backend_t wasmfs_create_root_dir() {
  return is_node() ? wasmfs_create_node_backend("")
                   : wasmfs_create_memory_backend();
}

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... PR #26624 addresses this.


$wasmfsNodeConvertNodeCode__deps: ['$ERRNO_CODES'],
$wasmfsNodeConvertNodeCode: (e) => {
Expand All @@ -29,8 +29,8 @@ addToLibrary({
$wasmfsNodeFixStat__deps: ['$wasmfsNodeIsWindows'],
$wasmfsNodeFixStat: (stat) => {
if (wasmfsNodeIsWindows) {
// Node.js on Windows never represents permission bit 'x', so
// propagate read bits to execute bits
// Windows does not report the 'x' permission bit, so propagate read
// bits to execute bits.
stat.mode |= (stat.mode & {{{ cDefs.S_IRUGO }}}) >> 2;
}
return stat;
Expand Down Expand Up @@ -222,5 +222,32 @@ addToLibrary({
// implicitly return 0
});
},
};

});
#if !ENVIRONMENT_MAY_BE_NODE
function makeStub(x, library) {
if (isJsOnlySymbol(x) || isDecorator(x)) {
return;
}

var t = library[x];
if (typeof t == 'string') return;
t = t.toString();

delete library[x + '__i53abi'];
delete library[x + '__deps'];
t = modifyJSFunction(t, (args, body) => {
return `(${args}) => {\n` +
(ASSERTIONS ? "abort('wasmfs::NodeBackend is no-op when !ENVIRONMENT_MAY_BE_NODE');\n" : '') +
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just abort('attempt to call nodejs backend function without ENVIRONMENT_MAY_BE_NODE)`.

I think the abort should exist even in release builds (you can make the error message optional if you like) since we never actually want a no-op behaviour here do we?

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit eea0053 adjusts the error message.

I tried enabling abort() in release builds, but the wasmImports for these functions are somehow being DCE'd when building other.test_wasmfs_nodefs_stubs with -sASSERTIONS=0 or -O3, which is unexpected. I'll investigate.

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... it was able to DCE away these functions as main() was empty, commit 4b86c1c fixes this.

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding abort() in release builds, perhaps that should be done as follow-up? I also noticed this in wrapSyscallFunction():

emscripten/src/lib/libcore.js

Lines 2514 to 2524 in 34f6bcf

// If a syscall uses FS, but !SYSCALLS_REQUIRE_FILESYSTEM, then the user
// has disabled the filesystem or we have proven some other way that this will
// not be called in practice, and do not need that code.
if (!SYSCALLS_REQUIRE_FILESYSTEM && t.includes('FS.')) {
library[x + '__deps'] = [];
t = modifyJSFunction(t, (args, body) => {
return `(${args}) => {\n` +
(ASSERTIONS ? "abort('it should not be possible to operate on streams when !SYSCALLS_REQUIRE_FILESYSTEM');\n" : '') +
'}';
});
}

'}';
});

library[x] = eval(`(${t})`);
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is eval needed here? I don't think we normally use eval in combination with modifyJSFunction

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied-pasted this from wrapSyscallFunction():

emscripten/src/lib/libcore.js

Line 2596 in 34f6bcf

library[x] = eval(`(${t})`);

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but you're right, it seems unnecessary. Fixed with commit 4143136.

}

for (var x in WasmFSNodeLibrary) {
makeStub(x, WasmFSNodeLibrary);
}
#endif

addToLibrary(WasmFSNodeLibrary);
20 changes: 20 additions & 0 deletions test/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9301,6 +9301,26 @@ def test_noderawfs_access_abspath(self):
''')
self.do_runf('access.c', cflags=['-sNODERAWFS'], args=[os.path.abspath('foo')])

def test_wasmfs_nodefs_stubs(self):
# This is equivalent to building with `-sWASMFS -sNODERAWFS`, except that the Wasm
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I under stand that the This is here?

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should probably be part of the wasmfs_create_root_dir() hook.

# binary can also be used on the web. Ensure that this use case is supported.
create_file('main.c', r'''
#include <emscripten/emscripten.h>
#include <emscripten/wasmfs.h>

EM_JS(bool, is_node, (), { return ENVIRONMENT_IS_NODE; });

backend_t wasmfs_create_root_dir() {
return is_node() ? wasmfs_create_node_backend("")
: wasmfs_create_memory_backend();
}

int main(int argc, char** argv) {
return 0;
}
''')
self.run_process([EMCC, 'main.c', '-sWASMFS', '-sENVIRONMENT=web'])
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what this test is doing exactly. Its not actually running the result so what is it supposed to show?

Also, when -sENVIRONMENT=web I think ENVIRONMENT_IS_NODE not even be declared/defined so this program would just crash with undefined reference I think?

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a test for the linker issue mentioned in comment #26608 (comment), but since commit d30f7de it's probably no longer relevant, as we can now be sure that these stubs are always in sync.

Commit d0c6c63 removes this test.

Also, when -sENVIRONMENT=web I think ENVIRONMENT_IS_NODE not even be declared/defined so this program would just crash with undefined reference I think?

I just checked this, but ENVIRONMENT_IS_NODE is also defined in -sENVIRONMENT=web builds.

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... PR #26624 reinstates this test in test_browser.py.


def test_noderawfs_readfile_prerun(self):
create_file('foo', 'bar')
self.add_pre_run("console.log(FS.readFile('foo', { encoding: 'utf8' }));")
Expand Down
Loading