WasmFS: Add NODERAWFS support #19400
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // Copyright 2023 The Emscripten Authors. All rights reserved. | ||
| // Emscripten is available under two separate licenses, the MIT license and the | ||
| // University of Illinois/NCSA Open Source License. Both these licenses can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "emscripten/wasmfs.h" | ||
|
|
||
| namespace wasmfs { | ||
|
|
||
| backend_t wasmfs_create_root_dir(void) { | ||
| return wasmfs_create_node_backend("."); | ||
| } | ||
|
|
||
| } // namespace wasmfs | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1867,6 +1867,22 @@ def can_use(self): | |
| return settings.WASMFS | ||
|
|
||
|
|
||
| class libwasmfs_noderawfs(Library): | ||
| name = 'libwasmfs_noderawfs' | ||
|
|
||
| cflags = ['-fno-exceptions', '-std=c++17'] | ||
|
|
||
| includes = ['system/lib/wasmfs'] | ||
|
|
||
| def get_files(self): | ||
| return files_in_path( | ||
| path='system/lib/wasmfs/backends', | ||
| filenames=['noderawfs_root.cpp']) | ||
|
There was a problem hiding this comment. I would prefer not to add another native system library just for this purpose. Can we find another way (other than weak linking) to express the mount point? There was a problem hiding this comment. Sorry, I should have asked you for review probably before landing. I'm not sure there is a better mechanism. We don't want to bloat code size by having some kind of global dispatch "if X then root backend = .. else if Y then root backend = ..". The only slim (and extensible) way to do it that I know is to have a weak symbol that is overridden like this. I can't think of another way at least. There was a problem hiding this comment. An alternative might be to use However, in terms of code size, this mechanism is probably better. There was a problem hiding this comment. Or, perhaps the Details--- a/tools/system_libs.py
+++ b/tools/system_libs.py
@@ -1821,6 +1821,7 @@ class libwasmfs(DebugLibrary, AsanInstrumentedLibrary, MTLibrary):
def __init__(self, **kwargs):
self.ignore_case = kwargs.pop('ignore_case')
+ self.node_rawfs = kwargs.pop('node_rawfs')
super().__init__(**kwargs)
def get_cflags(self):
@@ -1833,6 +1834,8 @@ class libwasmfs(DebugLibrary, AsanInstrumentedLibrary, MTLibrary):
name = super().get_base_name()
if self.ignore_case:
name += '-icase'
+ if self.node_rawfs:
+ name += '-noderawfs'
return name
@classmethod
@@ -1841,7 +1844,7 @@ class libwasmfs(DebugLibrary, AsanInstrumentedLibrary, MTLibrary):
@classmethod
def get_default_variation(cls, **kwargs):
- return super().get_default_variation(ignore_case=settings.CASE_INSENSITIVE_FS, **kwargs)
+ return super().get_default_variation(ignore_case=settings.CASE_INSENSITIVE_FS, node_rawfs=settings.NODERAWFS, **kwargs)
def get_files(self):
backends = files_in_path(
@@ -1852,6 +1855,10 @@ class libwasmfs(DebugLibrary, AsanInstrumentedLibrary, MTLibrary):
'memory_backend.cpp',
'node_backend.cpp',
'opfs_backend.cpp'])
+ if self.node_rawfs:
+ backends += files_in_path(
+ path='system/lib/wasmfs/backends',
+ filenames=['noderawfs_root.cpp'])
return backends + files_in_path(
path='system/lib/wasmfs',
filenames=['file.cpp',
@@ -1867,22 +1874,6 @@ class libwasmfs(DebugLibrary, AsanInstrumentedLibrary, MTLibrary):
return settings.WASMFS
-class libwasmfs_noderawfs(Library):
- name = 'libwasmfs_noderawfs'
-
- cflags = ['-fno-exceptions', '-std=c++17']
-
- includes = ['system/lib/wasmfs']
-
- def get_files(self):
- return files_in_path(
- path='system/lib/wasmfs/backends',
- filenames=['noderawfs_root.cpp'])
-
- def can_use(self):
- return settings.WASMFS and settings.NODERAWFS
-
-
class libhtml5(Library):
name = 'libhtml5'
(untested) There was a problem hiding this comment.
Yeah, exactly - EM_JS can work, and linking is simpler, but it would be more code that can't be removed by the linker, since it won't be able to tell what the EM_JS call will return. Using weak linking, that information is utilized at link time basically. I don't have a strong opinion on using |
||
|
|
||
| def can_use(self): | ||
| return settings.WASMFS and settings.NODERAWFS | ||
|
|
||
|
|
||
| class libhtml5(Library): | ||
| name = 'libhtml5' | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that NODERAWFS exposed
/as the root.. i.e. you can open/etc/passwdif you want.This looks like its exposing the current working directory..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible you can't travel up the tree to get to the true outside root, I'm not sure (also not sure if the old FS supported that).
But this does make us behave the same as the old FS in the common case of just accessing files in the current folder, or beneath it. That is useful for
configuretests like freetype needs, for example. And I think it's likely the common case? But we may need more eventually.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC NODERAWFS is explicitly about granting the generated program complete raw access to the entire underlying filesystem (basically making of VFS layer invisible). I think a lot of folks depend on that so we probably want to keep that behaviour around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, perhaps
NODERAWFScan/should be completely independent of wasmfs?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the code can be independent though? We have a NODERAWFS impl for the old FS and now the beginnings of one for the new FS.
I think this PR might be enough, if we also have support for looking up the directory tree. I checked that now and indeed it works in the JS FS but not in WasmFS. So we would need to set the root to be the actual system root I think. Just changing this PR to do
/instead of.for the root dir doesn't work though, so that needs more investigation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested with this:
A test with that might be useful to add as I don't think we have any more NODERAWFS tests atm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way NODERAWFS works IIUC is to completely replace/override all the old FS code.. basically taking over completely. I think the equivalent for the new version of
NODERAWFSmight be to simply to provide an alternative implementation of the musl syscalls and avoid include wasmfs at all.See
emscripten/src/library_noderawfs.js
Lines 23 to 26 in 6b179e3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at test_core.py and test_other.py I see a fair few NODERAWFS tests.. have you tried all them with WASMFS? I would expect/hope that at least one of them would fail with your currently "mount CWD at root" approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I'm not sure I can think of a good way to do that in WasmFS actually. Yeah, maybe a reimplementation at the syscall layer is worth exploring, as you said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't gotten to them all yet but all those I've tried have passed so far.