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

Shim fd_sync #29

Merged
merged 2 commits into from
Jun 10, 2023
Merged

Shim fd_sync #29

merged 2 commits into from
Jun 10, 2023

Conversation

shritesh
Copy link
Contributor

@shritesh shritesh commented Jun 9, 2023

First of all, a huge thanks for creating this project. I have been using this for about a month with a few additions in a fairly large project (at work) and its been great. After the last few commits, only the remaining two changes are necessary to make it work.

The fd_sync one is trivial. For fd_seek on directories, IIUC the return value is supposed to be treated as an opaque value so I return 0 there.

@bjorn3
Copy link
Owner

bjorn3 commented Jun 9, 2023

I'm happy to hear that this project has been helpful for you.

Adding a nop fd_sync impl makes sense to me. The entire VFS is in-memory anyway. Maybe you could move it to Fd directly so that it doesn't have to be implemented for every class that is deriving from Fd?

I think fd_seek on a directory returns EBADF in Wasmtime however.

Remove wasmtime-incompatible fd_seek implementation for dirs
@shritesh shritesh changed the title Shim files fd_sync and dirs fd_seek Shim fd_sync Jun 9, 2023
@shritesh
Copy link
Contributor Author

shritesh commented Jun 9, 2023

Cool. I've done so. Also removed the incompatible fd_seek.

@shritesh
Copy link
Contributor Author

shritesh commented Jun 9, 2023

Huh, turns out we didn't need the fd_seek on dirs anymore. Everything works correctly as is.

@bjorn3 bjorn3 merged commit 462409e into bjorn3:main Jun 10, 2023
1 check passed
@bjorn3
Copy link
Owner

bjorn3 commented Jun 10, 2023

Released as v0.2.13.

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.

None yet

2 participants