Add nodefs readdir handling for directories that contain exotic entries #22925
Conversation
src/library_syscall.js
Outdated
| // If the entry is not a directory, file, or symlink, nodefs | ||
| // lookupNode will raise EINVAL. Skip these and continue. | ||
| if (e?.errno === {{{ cDefs.EINVAL }}}) { | ||
| idx += 1; |
There was a problem hiding this comment.
Actually I wonder if this while loop could be re-written into a 4 loop to avoid the duplication?
var startIdx = Math.floor(off / struct_size);
var endIdx = Math.max(stream.getdents.length, startIdx + count)
for (var idx = startIdx; idx < endIdx; idx++) {
...
}
| if not WINDOWS and not MACOS: | ||
| # Add an entry that isn't a directory, file, or link to test that we handle | ||
| # it correctly. | ||
| os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) |
There was a problem hiding this comment.
macos doesn't support os.mkfifo either? TIL.
There was a problem hiding this comment.
Actually I think it is supported on MacOS I'll remove that filter.
|
Looks like this perhaps requires a wasmfs change to match? wasmfs.test_fs_nodefs_readdir is now failing. Looks like maybe wasmfs is reporting "." and ".." but the old FS is not ? Maybe we want go back to the old way of verifying the results to avoid having to fix these decrpencies as part of this PR? |
|
Yeah maybe the best thing to do would be to grab the output and make a series of assertions that it contains or does not contain certain strings? It seems that the order is not stable but the set of lines should be. |
Sure. That sounds reasonable. I was a little surprised to see who different the wasmfs output was though. I guess there is stuff we need to address there: Specifically it looks like wasmfs supports |
Resolves #22924.