Add createFile to JS API for wasmfs (#23667) #23668
Conversation
|
I've read "date" as "update", hope that's what you meant! |
test/fs/test_fs_js_api.c
Outdated
| @@ -469,6 +469,17 @@ void cleanup() { | |||
| remove("closetestfile"); | |||
| } | |||
|
|
|||
| #if WASMFS | |||
There was a problem hiding this comment.
This API is not specific to WASMFS is it? I see it exists in libfs.js too.
There was a problem hiding this comment.
Oh indeed it does, I had no idea. But this version of the test is wasm-specific since it uses a backend. It also looks like the libfs version ignores the "properties" field and has a different interface.
Should a different name than createFile be picked for the wasmfs version? createFileInBackend?
There was a problem hiding this comment.
I think the goal with the JS API is mostly feature parity with the existing/old FS. If possible I thin we should avoid extending the JS API.
But yes, if we decide we really do need a new API we should give it a new name.
Would you make making a version of createFile that is compatible with the old FS and testing that as part of this PR? Or as a separate PR?
There was a problem hiding this comment.
I've made WASMFS createFile compatible with the JS version. The JS version never used the third parameter, so I'm using it for the backend here. I also made it so the test (though a bit different) runs on both filesystems.
test/fs/test_fs_js_api.c
Outdated
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright 2023 The Emscripten Authors. All rights reserved. | |||
| * Copyright 2025 The Emscripten Authors. All rights reserved. | |||
There was a problem hiding this comment.
We don't update out copyright dates when we modify files
Although the unused "properties" argument now is used as the "backend" for the file. If null, wasmfs will figure out the right backend to use based on the path.
| ); | ||
| #else | ||
| EM_ASM( | ||
| var file = FS.createFile("/", "test.txt", {}, true, true); |
There was a problem hiding this comment.
Can we make sure that this simple usage (without a backend) is works with both the old FS and with wasmfs.
In fact, it might be best not to expose the backend directly here, since then the two APIs would not be compatible.
In fact I think the for in libfs.js which has createFile call through to FS.create should probably work fine under wasmfs too, so maybe just copy it verbatim and at a test for it.
Do we really need a new API for creating a backend-specific file?
There was a problem hiding this comment.
The fact that no other FS.xxx APIs for wasmfs take the backend as an argument suggest that perhaps this is not needed in this case either.
There was a problem hiding this comment.
It can’t be copied verbatim mainly because the JS version handles either a path or a node for the parent, but the wasm version doesn’t have the concept of a node. AFAIK there’s no way to get the path to a file descriptor/wasmfs file.
The reason the FS.apis in wasmfs don’t take a backend is presumably because they’re meant to mimic JS FS APIs which only support a 1:1 relationship between backends and mount points. But wasmfs backends aren’t necessarily mounted somewhere. Fetchfs, for example, has supported this on the C side (a fetchfs whose url is a specific file, meant to be used to create a file). The tests for the JS wasmfs backend show a similar usage. The wasmfs C API seems to use the metaphor of creating files and directories within specific backends, but we don’t have a way to express that in JS yet.
As for compatibility, in this case the third argument seems to have no meaning in the JS version (it’s unused), so it shouldn’t be a compatibility issue. But a different name can be picked if this is a problem.
There was a problem hiding this comment.
I think we have two different issue here perhaps.
The first issue is that WASMFS lacks the FS.createFile API that the old FS has. Step 1 would be to add this API and add tests for it. (it should work in both WASM FS and the old FS and it should operate on the root mountpoint just like the all the other APIs).
If you want to then extend the JS API to include backend-specific APIs that is anther question, perhaps one that @kripken and @tlively could weigh in on. Perhaps you could explain why there is a need to be able to manipulate backends that are not mounted anywhere in the filesytem?
There was a problem hiding this comment.
Maybe the tension here is that the only way I can see for JS code to use WASMFS is through the WASMFS port of the JS FS API, and I'm trying to extend that. If there were a "native" wasmfs API in JS, which exposed the same API as emscripten/wasmfs.h (with conveniences around strings for example) then JS code could use the WASMFS API in exactly the way C code can (wasmfs backends aren't necessarily mounted anywhere, they just get to create directories or create files on the backend at different paths on the filesystem).
I know there's a TODO on wasmfs_create_file saying that in the future only directories should be mounted, but that's inconsistent with the existing tests (test/wasmfs/wasmfs_jsfile.c, test/wasmfs/wasmfs_fetch.c) and it's convenient to be able to work with a backend that semantically is a single file. For now, that means I could achieve what I had talked about earlier with that "mapfs" or manifest idea purely on the JS side, by creating and mounting single-file backends where I need them.
There was a problem hiding this comment.
Regarding mounting a single file: The ability to "mount" a file is not something that maps to the existing concept of mount from linux/posix. Perhaps the use case is very strong here and we can/should deviate to allow the idea of mounting just a file? I would suggest that we avoid doing that if possible, but again I'll probably defer to @kripken / @tlively who were more involved in the original wasmfs design.
Per #23667, add a createFile function to the WASMFS JS API that calls out to
wasmfs_create_file.