Skip to content

Add createFile to JS API for wasmfs (#23667) #23668

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add createFile to JS API for wasmfs (#23667) #23668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
82c5e54
Add createFile to JS API for wasmfs (#23667)
JoeOsborn Feb 13, 2025
587c135
Add test for createfile
JoeOsborn Feb 13, 2025
24edb2f
Revert year change
JoeOsborn Feb 13, 2025
41ce74a
Change WASMFS createFile to match JS API arg order
JoeOsborn Feb 14, 2025
5edbd0b
Fix improper use of assert in JS tests
JoeOsborn Feb 14, 2025
893d559
Missing comma
JoeOsborn Feb 14, 2025
cc7f2af
fix createFile in case of non-string parent
JoeOsborn Feb 14, 2025
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
5 changes: 5 additions & 0 deletions src/lib/libwasmfs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ FS.init();
FS.ErrnoError.prototype = new Error();
FS.ErrnoError.prototype.constructor = FS.ErrnoError;
},
createFile(parent, name, backend, canRead, canWrite) {
var pathName = name ? parent + '/' + name : parent;
var mode = FS_getMode(canRead, canWrite);
return FS.handleError(withStackSave(() => _wasmfs_create_file(stringToUTF8OnStack(path), mode, backend)));
},
createDataFile(parent, name, fileData, canRead, canWrite, canOwn) {
FS_createDataFile(parent, name, fileData, canRead, canWrite, canOwn);
},
Expand Down
18 changes: 18 additions & 0 deletions test/fs/test_fs_js_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,23 @@ void cleanup() {
remove("closetestfile");
}

void test_fs_createfile_js() {
#if WASMFS
EM_ASM(
var backend = MEMFS.createBackend({});
var file = FS.createFile("/", "test.txt", backend, true, true);
if (file <= 0) throw "No file created";
FS.close(file);
);
#else
EM_ASM(
var file = FS.createFile("/", "test.txt", {}, true, true);
Copy link
Collaborator

@sbc100 sbc100 Feb 14, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

@sbc100 sbc100 Feb 14, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@JoeOsborn JoeOsborn Feb 15, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@sbc100 sbc100 Feb 15, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@JoeOsborn JoeOsborn Feb 15, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@JoeOsborn JoeOsborn Mar 1, 2025

Choose a reason for hiding this comment

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

Yeah. I want to use the fetch backend to fetch files from many distinct paths on a web server into one folder (with specific file names) in the filesystem. I had proposed a manifest parameter for fetchfs to support this kind of thing:

https://server/path1/file1.txt => dir/fileA
https://server/path2/file2.txt => dir/fileB

If it’s hard to implement or slow to land a mapping backend, I guess I could change my server to use a common root url (these are DB records grouped as a result of a query so this is nontrivial), but currently I solve it by making a (small) number of single file fetchfs backends and use create_file for each.

Copy link
Collaborator

@sbc100 sbc100 Mar 1, 2025

Choose a reason for hiding this comment

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

In the above example could we not have have fetchfs mounted at dir with two files withing it?

Is that idea that each file gets its own backend/mount point?

If so it doesn't seems like maybe better design would be to allow a single fetch backend to have any number of files under a common root (dir in the example above). Maybe that right-hand-side entries in the manifest could be relative to where the backend is mounted rather than relative to where the actual root FS.

Copy link
Contributor Author

@JoeOsborn JoeOsborn Mar 1, 2025

Choose a reason for hiding this comment

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

fetchfs doesn’t include manifest support right now (I pulled it out in favor of a hypothetical future mapfs that maps one path to another path); it’s true that the manifest would actually be /fileA, /fileB.

As it is, because fetchfs uses the path relative to the mount point as the URL relative to the baseURL, if I didn’t have create_file I would either need to be ok with a filesystem like dir/path1/file1.txt, dir/path2/file2.txt (using a baseURL of /), change my server (nontrivial due to the rationale above), or implement mapfs to map a path like fileA->/fetch/path1/file1 and fileB->/fetch/path2/file2 and mount my fetchfs backend at /fetch.

I do actually like the mapfs idea, but it’s a bigger change and I’m not sure when I’ll have bandwidth to do it. Even if you don’t want to commit to including createFile in the JS api, leaving it in the C api for a few versions would give me time to try and build and land a PR for mapfs.

As a separate question, is it possible to write out-of-tree wasmfs backend implementations? What would compiling them and linking them into a build look like since they’d potentially provide both C and JS symbols?

Copy link
Contributor Author

@JoeOsborn JoeOsborn Mar 2, 2025

Choose a reason for hiding this comment

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

I got a little obsessed with this problem so I experimented and ended up with #23808, though I haven't fully explored it for my use case yet I think it should probably work.

Copy link
Contributor Author

@JoeOsborn JoeOsborn Mar 3, 2025

Choose a reason for hiding this comment

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

It also occurs to me that wasmfs fetch backend as far back as I’m aware requires users to create individual files before reading them. I don’t know how to (as in, I haven’t looked into) make the backend support creating files on read/stat (which feels like a better user experience anyway).

if (file <= 0) throw "No file created";
FS.close(file);
);
#endif
}

int main() {
test_fs_open();
test_fs_createPath();
Expand All @@ -484,6 +501,7 @@ int main() {
// TODO: Fix legacy API FS.mmap bug involving emscripten_builtin_memalign
test_fs_mmap();
#endif
test_fs_createfile_js();
test_fs_mkdirTree();
test_fs_utime();

Expand Down
Loading