[WasmFS] Skip root directory setup under NODERAWFS by kleisauke · Pull Request #26607 · emscripten-core/emscripten

kleisauke · 2026-04-02T08:04:26Z

Resolves: #24836.

Split out from #24733.

sbc100 · 2026-04-02T15:39:12Z

  virtual std::shared_ptr<Directory> createDirectory(mode_t mode) = 0;
  virtual std::shared_ptr<Symlink> createSymlink(std::string target) = 0;

+  virtual bool shouldMountSpecialFiles() { return true; }


I was thinking maybe shouldPopulateRoot, but maybe your name is more explicit?

This function name was mentioned in comment #24733 (comment). But I'd like shouldPopulateRoot as well.

Changed to shouldPopulateRoot with commit 98baba6.

sbc100 · 2026-04-02T15:40:27Z

    self.set_setting('WASMFS')
    self.do_run_in_out_file_test('wasmfs/wasmfs_chown.c')

-  @wasmfs_all_backends


Why remove this?

Does wasmfs_getdents.c need to get re-written to handle the rawfs mabye?

Should we just skip under NODERAWFS rather than removing wasmfs_all_backends completely?

I re-added the @wasmfs_all_backends decorator and skipped this only under NODERAWFS with commit f3b85c9.

See for details:

emscripten/test/wasmfs/wasmfs_getdents.out

Lines 51 to 59 in 34f6bcf

------------- Reading from /dev Directory via JS -------------

.

..

null

random

stderr

stdin

stdout

urandom

kleisauke · 2026-04-06T17:04:34Z

Perhaps this setup logic belongs in the MEMFS backend (and possibly others) instead? In other words, when FS.mount(MEMFS, {}, '/memory'); is called on a NODERAWFS root backend, should it produce the following layout?:

$ tree / -L 3 --dirsfirst
/ (NODERAWFS)
└── memory (MEMFS)
    ├── dev
    │   ├── null -> SpecialFiles::getNull()
    │   ├── random -> SpecialFiles::getRandom()
    │   ├── stderr -> SpecialFiles::getStderr()
    │   ├── stdin -> SpecialFiles::getStdin()
    │   ├── stdout -> SpecialFiles::getStdout()
    │   └── urandom -> SpecialFiles::getURandom()
    └── tmp

This would avoid the need of the shouldPopulateRoot overridable hook.

sbc100

lgmt, with testing question

sbc100 · 2026-06-22T20:09:39Z

  @wasmfs_all_backends
  def test_wasmfs_getdents(self):
+    if self.get_setting('NODERAWFS'):
+      self.skipTest('test expectations assumes /dev is virtualized')


Are there no tests that can now be enabled by this change?

Seems a little odd that this change would strictly make more tests fail.

PR #24733 (from which this change is split out) would allow more tests to be enabled (e.g. it removes the skip for other.test_fs_dev_random which is currently disabled in this PR).

The downside of splitting this change early is that absolute path access under -sNODERAWFS mode in WasmFS is still broken. So, opening /dev/random under -sNODERAWFS -sWASMFS after this PR lands will no longer work, since it's a absolute path that is no longer virtualized. Though, this only affects the -sNODERAWFS -sWASMFS combo, which is likely not widely used (and -sWASMFS is still marked experimental).

sbc100 · 2026-06-23T16:28:57Z

lgtm. Happy to see this land incrementally.

sbc100 · 2026-06-23T16:30:03Z

Perhaps this setup logic belongs in the MEMFS backend (and possibly others) instead? In other words, when FS.mount(MEMFS, {}, '/memory'); is called on a NODERAWFS root backend, should it produce the following layout?:
$ tree / -L 3 --dirsfirst
/ (NODERAWFS)
└── memory (MEMFS)
    ├── dev
    │   ├── null -> SpecialFiles::getNull()
    │   ├── random -> SpecialFiles::getRandom()
    │   ├── stderr -> SpecialFiles::getStderr()
    │   ├── stdin -> SpecialFiles::getStdin()
    │   ├── stdout -> SpecialFiles::getStdout()
    │   └── urandom -> SpecialFiles::getURandom()
    └── tmp
This would avoid the need of the shouldPopulateRoot overridable hook.

Yes.. I agree. MEMFS-as-the-root is really the only time we should be creating these root dirs from scratch, right? @tlively WDYT?

tlively · 2026-06-24T00:29:24Z

Yes, I think I agree that the special files only make sense with an in-memory root backend. But IMO it's moot because only the in-memory backend makes sense as the root, besides the special case of NODERAWFS, which really maybe should not be using a virtual filesystem layer at all.

kleisauke · 2026-06-25T19:39:11Z

I removed the shouldPopulateRoot flag in favor of a populateRoot callback, but it seems to regress code size somewhat, so the previous approach may have been better.

FWIW, the changes to syscalls.cpp and paths.cpp are NFC. I tried to remove the dependency on wasmFS.getRootDirectory(), assuming that repeatedly calling getParent() would eventually reach the root directory. However, that doesn't seem to hold when multiple backends are mounted. See commit c065f72 for details.

sbc100 · 2026-06-25T19:41:36Z

FWIW, the changes to syscalls.cpp and paths.cpp are NFC.

Can these be split out then maybe? (to keep this PR focuced).

sbc100 · 2026-06-25T19:42:09Z

I'm not too worried about small regression in wasmfs binary size, I don't think we are that the point of micro-optimizing wasmfs for size just yet.

kleisauke · 2026-06-25T19:55:05Z

FWIW, the changes to syscalls.cpp and paths.cpp are NFC.

Can these be split out then maybe? (to keep this PR focuced).

Sure, reverted with commit f11ec8f and fe159c1.

AFAIK, the test failures fixed by commit c065f72 were only related to test/setup_nodefs.js.

emscripten/test/setup_nodefs.js

Lines 1 to 6 in fb69be2

    
           Module['onRuntimeInitialized'] = () => { 
        
             out('mounting node filesystem under /nodefs'); 
        
             FS.mkdir('/nodefs'); 
        
             FS.mount(NODEFS, { root: '.' }, '/nodefs'); 
        
             FS.chdir('/nodefs'); 
        
           };

(but perhaps that can be removed in favor of -sNODERAWFS, I'll have a look)

Resolves: emscripten-core#24836.

This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (2) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_codesize_cxx_wasmfs.json: 179468 => 179510 [+42 bytes / +0.02%] codesize/test_codesize_files_wasmfs.json: 63681 => 63723 [+42 bytes / +0.07%] Average change: +0.04% (+0.02% - +0.07%) ```

This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (2) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_codesize_cxx_wasmfs.json: 179465 => 179507 [+42 bytes / +0.02%] codesize/test_codesize_files_wasmfs.json: 63678 => 63720 [+42 bytes / +0.07%] Average change: +0.04% (+0.02% - +0.07%) ```

kleisauke mentioned this pull request Apr 2, 2026

[WasmFS] Fix absolute path access under NODERAWFS #24733

Draft

sbc100 reviewed Apr 2, 2026

View reviewed changes

kleisauke changed the title ~~[WasmFS] Skip mounting special files under NODERAWFS~~ [WasmFS] Skip root directory setup under NODERAWFS May 22, 2026

sbc100 approved these changes Jun 22, 2026

View reviewed changes

kleisauke added 3 commits June 27, 2026 12:27

[WasmFS] Skip mounting special files under NODERAWFS

f5e8b23

Resolves: emscripten-core#24836.

Replace flag with callback function

58257c3

kleisauke force-pushed the wasmfs-skip-special-files-mount-noderawfs branch from fe159c1 to f988737 Compare June 27, 2026 10:50

sbc100 approved these changes Jun 27, 2026

View reviewed changes

kleisauke added 2 commits June 27, 2026 19:16

Merge branch 'main' into wasmfs-skip-special-files-mount-noderawfs

741db0f

	------------- Reading from /dev Directory via JS -------------
	.
	..
	null
	random
	stderr
	stdin
	stdout
	urandom

Uh oh!

Conversation

kleisauke commented Apr 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kleisauke commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 commented Jun 23, 2026

Uh oh!

sbc100 commented Jun 23, 2026

Uh oh!

tlively commented Jun 24, 2026

Uh oh!

kleisauke commented Jun 25, 2026

Uh oh!

sbc100 commented Jun 25, 2026

Uh oh!

sbc100 commented Jun 25, 2026

Uh oh!

kleisauke commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kleisauke commented Apr 6, 2026 •

edited

Loading