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

test(wasi) Running all WASI tests with wasmer_vfs::mem_fs too #2546

Merged
merged 12 commits into from
Sep 3, 2021

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Aug 31, 2021

Description

This WIP PR updates our WASI test framework to specifically test with wasmer_vfs::host_fs (the default, as of now) and with wasmer_vfs::mem_fs (🆕!).

Bug trophies:

Review

  • Add a short description of the change to the CHANGELOG.md file

Hywan added 6 commits August 31, 2021 11:26
This PR updates how we generate the WASI test suites to test against
the `wasmer_vfs::host_fs` (the default), and `wasmer_vfs::mem_fs`
(that's new).
When opening a file with the `append` option turned on, all `seek`
operations must be ignored. As described by
[`open(2)`](https://man7.org/linux/man-pages/man2/open.2.html), the
`O_APPEND` option describes this behavior well:

> Before each write(2), the file offset is positioned at
> the end of the file, as if with lseek(2).  The
> modification of the file offset and the write operation
> are performed as a single atomic step.
>
> O_APPEND may lead to corrupted files on NFS filesystems
> if more than one process appends data to a file at once.
> This is because NFS does not support appending to a file,
> so the client kernel has to simulate it, which can't be
> done without a race condition.

This patch implements that behavior.
Also, this patch rewind the file cursor if opened in read-mode.
Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

There are very few changes on mem_fs itself, which demonstrates the resilience of current implementation.
Great work on the PR!

@Hywan
Copy link
Contributor Author

Hywan commented Sep 2, 2021

bors try

bors bot added a commit that referenced this pull request Sep 2, 2021
@bors
Copy link
Contributor

bors bot commented Sep 2, 2021

bors bot added a commit that referenced this pull request Sep 3, 2021
2550: feat(vfs) Update `Metadata.len` when updating the file buffer r=Hywan a=Hywan

# Description

This patch updates `wasmer_vfs::mem_fs`  to handle `Metadata.len` correctly. The `len` value is updated when something is written in the file buffer. `len` is also updated when the file is truncated (with open options) or when the file is update with `set_len`.

It helps to fix one test in #2546.

Co-authored-by: Ivan Enderlin <[email protected]>
bors bot added a commit that referenced this pull request Sep 3, 2021
2551: feat(vfs) Add ability to rename a file with `mem_fs` r=Hywan a=Hywan

# Description

This patch updates `wasmer_vfs::mem_fs::FileSystem::rename` to handle file. Directories were handle previously, but not file.

~~Tests must be added though.~~

It solves the last bits of #2546.

Co-authored-by: Ivan Enderlin <[email protected]>
@Hywan Hywan marked this pull request as ready for review September 3, 2021 09:29
@Hywan
Copy link
Contributor Author

Hywan commented Sep 3, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 3, 2021

@bors bors bot merged commit 4ab4e91 into wasmerio:master Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants