Skip to content
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.

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

Basic filesystems support for Wasm: support WASI-based filesystem for wasmWasi #257

Merged
merged 16 commits into from
Feb 19, 2024

Conversation

fzhinkin
Copy link
Collaborator

@fzhinkin fzhinkin commented Feb 7, 2024

Supported filesystem for wasmWasi target by implementing all the functionality on top of WASI preview 1.

The implementation is inspired by fs-implementations from kowasm and Okio.

Unlike other platforms, in Wasm-WASI, there are no such things as the current working directory (but there's an initial-cwd in wasi-cli:0.2.0) and a system temporary directory.

The lack of CWD affects relative paths resolution (which are usually resolved against CWD).

There are several approaches adopted by the community:

  • wasi-libc use / as a CWD;
  • zig suggests pre-opening . and all relative paths will be resolved against it;
  • kowasm and Okio use a first pre-opened directory as CWD.

There's also an option to rely on environment variables, but that approach is not very favorable, according to WebAssembly/wasi-filesystem#24.

I decided to stick to the approach employed by kowasm and Okio as that approach could be more familiar to Kotlin developers.

For the temporary directory, for now, I decided to use /tmp as a temp dir, or fail if the directory is not available. The behavior could be changed once it becomes an issue for users.

@fzhinkin fzhinkin marked this pull request as ready for review February 7, 2024 14:18
@fzhinkin fzhinkin added the fs label Feb 7, 2024
* @return a resolved path.
* @throws FileNotFoundException if there is no file or directory corresponding to the specified path.
*/
override fun resolve(path: Path): Path {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dkhalanskyjb (you asked, I tagged).
Here's a hand-written canonization implementation. It's used only for Wasm-WASI.

core/wasmWasi/src/files/FileSystemWasm.kt Show resolved Hide resolved
rootPaths
}

internal fun getRootOrNull(path: Path): PreOpen? {
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand it, this function is not looking for the root directory, but an preopen one, maybe it should be called findPreopen(...)?

return null
}

internal fun getRoot(path: Path): PreOpen {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same about name, mentioning the root is a bit confusing

core/wasmWasi/src/files/FileSystemWasm.kt Outdated Show resolved Hide resolved

SystemFileSystem.createDirectories(Path("/tmp/a/b/c/d/e"))
try {
WasiFileSystem.symlink(Path("/tmp/a/b/c/d/e"), Path("/tmp/l"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also check for recursive symlinks?

)
if (res != Errno.success) {
if (res == Errno.noent) throw FileNotFoundException("File does not exist: $path")
throw IOException("Can't open $path for write: ${res.description}")
Copy link
Contributor

Choose a reason for hiding this comment

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

for write?

val root = PreOpens.getRoot(path)

val fd = withScopedMemoryAllocator { allocator ->
val fdPtr = allocator.allocate(4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Magic number.
Can we create some constants and util functions for sizes and null-terminated string length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For allocate(4) - definitely. I am not sure about Null-terminated strings. I think it should be enough to explicitly comment on why there's an extra 1 in the code for now.


while (remaining > 0) {
val toRead = minOf(remaining, TEMP_CIOVEC_BUFFER_LEN).toInt()
source.readToLinearMemory(buffer, toRead)
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not immediately clear which buffer was meant. Perhaps we should call it something else?

core/wasmWasi/src/files/FileSystemWasm.kt Show resolved Hide resolved
core/wasmWasi/src/files/FileSystemWasm.kt Show resolved Hide resolved
* code clean-up and renaming
* improved exception messages
* improved build scripts
@fzhinkin fzhinkin merged commit d188fe3 into wasm-fs-support-part-1 Feb 19, 2024
1 check passed
@fzhinkin fzhinkin deleted the wasm-fs-support-part-2 branch February 19, 2024 16:42
@fzhinkin fzhinkin restored the wasm-fs-support-part-2 branch February 19, 2024 16:42
fzhinkin added a commit that referenced this pull request Feb 19, 2024
… wasmWasi (#257)

Implemented basic filesystem support on top of Wasm WASI.
@fzhinkin fzhinkin deleted the wasm-fs-support-part-2 branch February 19, 2024 16:44
@fzhinkin fzhinkin linked an issue Mar 18, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Basic filesystems support for Wasm target
3 participants