Add useful functions for external use of WASI filesystem #595

MarkMcCaskey · 2019-07-30T05:55:12Z

part of #583

kitlith · 2019-07-30T07:00:33Z

lib/wasi/src/state.rs

-    pub fn close(self) {}
+/// This trait relies on your file closing when it goes out of scope via `Drop`
+/// it does not require `Drop` because `std::fs::File` does not implement it
+/// (TODO: figure out how that makes sense, because it clearly does)


std::fs::File does not implement Drop, because it contains a std::sys::fs::File, which does not implement Drop as it contains a std::sys::fd::FileDesc, which implements Drop.

aka, only the innermost item needs to implement Drop, rust figures out the rest on it's own?

Thanks! I was reading through the code, but didn't get that far -- that's good to know

so, yeah, apparently we should (almost?) never require an impl of Drop, because of the above.

kitlith · 2019-07-30T07:08:45Z

lib/wasi/src/state.rs

+
+#[derive(Debug)]
+pub struct Stdout(std::io::Stdout);
+impl Read for Stdout {


I feel like a 'nicer' way of doing this is to essentially encapsulate Read/Write/Seek in our trait, and provide default generic implementations if the type implements Read/Write/Seek.

(this is probably not nicer in many other ways, so meh.)

Hmm, yeah, I see what you mean. I think this is fine because while it is garbage code, we only need to write it once -- and I'll refactor the state.rs file to hide things like this because we mostly never need to see it -- and it lets us use all code working with the standard traits including third party libraries which could save us a lot of time in the future.

I mean, so does this approach. The issue (either way, really) is that if a third party library is missing any one of these three, you have to wrap it in a newtype and create dummy implementations. At least with the 'wrapper trait' approach, you aren't implementing, say, Seek on something that can't actually support Seek. shrug

only solution I can think of that doesn't require boilerplate is probably 'wrapper trait' + specialization, and even then a) i don't think we want to do that and b) it doesn't appear to work like it does in my head so...

kitlith · 2019-07-30T07:25:23Z

I will note that currently this PR by itself does not expose any extra functionality, as WasiFs/WasiState and such is still not exported at the crate top-level.

EDIT: now it does.

MarkMcCaskey · 2019-07-31T02:25:47Z

@kitlith
Let me know if it looks good to you

specifically,

/// Get WasiState from a Ctx
pub unsafe fn get_wasi_state(ctx: &mut Ctx) -> &mut WasiState {
    &mut *(ctx.data as *mut WasiState)
}

Presumably in the future we can also provide higher level APIs that don't require using the raw WASI types which I also exported in this PR. I wrapped the error type for the public facing APIs and maybe we can do the same with permissions and capabilities as well. The fd capabilities should map very closely to what std::file::OpenOptions is doing, so a similar builder pattern should be simple to implement

MarkMcCaskey · 2019-07-31T03:09:53Z

bors r+

595: Add useful functions for external use of WASI filesystem r=MarkMcCaskey a=MarkMcCaskey part of #583 Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <markmccaskey@users.noreply.github.com>

MarkMcCaskey · 2019-07-31T03:10:39Z

bors r-

bors · 2019-07-31T03:10:40Z

Canceled

MarkMcCaskey · 2019-07-31T03:55:13Z

I updated the plugin example to use the new logging feature. It relies on the guest calling into the host. I think we will probably definitely need the builder pattern, too. Because relying on the guest to do that is less than ideal. There's also the possibility of solving this problem with Wasm transformations (the host performing arbitrary modifications to the Guest's Wasm before running) but I don't think that's something we support very well right now

MarkMcCaskey · 2019-07-31T03:56:33Z

bors r+

595: Add useful functions for external use of WASI filesystem r=MarkMcCaskey a=MarkMcCaskey part of #583 Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <markmccaskey@users.noreply.github.com>

bors · 2019-07-31T04:12:33Z

Build succeeded

kitlith · 2019-07-31T05:34:12Z

I don't really see why the guest needs to call into the host, when there's wasmer_runtime::Instance::context_mut? I may be missing something.

And I don't think get_wasi_state should have been changed into a safe function, as it is only sound if the Ctx came from your WASI implementation, right? We (currently) must rely on the user to know this...

MarkMcCaskey · 2019-07-31T06:47:40Z

@kitlith Wow, I did not know about that at all, but it makes sense that that would exist. That would actually be useful in a personal project I'm making with Wasmer. Thanks! I'll update the example.

Hmm, that's a fair point.

So the reason why I made it safe was because in our code we're casting a &Ctx to a &mut Ctx, which is definitely not safe... here I made it require a &mut Ctx so it's safe in that regard, but you're correct, calling this on a non-wasi context will do bad things and have undefined behavior. I'll update the example and make it unsafe in a PR before we release tomorrow

Add useful functions for external use of WASI filesystem

fba09be

MarkMcCaskey added the 📦 lib-wasi About wasmer-wasi label Jul 30, 2019

MarkMcCaskey mentioned this pull request Jul 30, 2019

Embedding API for wasmer-wasi #583

Closed

4 tasks

kitlith reviewed Jul 30, 2019

View reviewed changes

clean up wasi fs public api changes

620a521

syrusakbary approved these changes Jul 31, 2019

View reviewed changes

Merge branch 'master' into feature/public-api-wasi-fs

c099ed5

add logging to plugin example, improve swap, and add some consts

b407633

bors bot merged commit b407633 into master Jul 31, 2019

bors bot deleted the feature/public-api-wasi-fs branch July 31, 2019 04:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add useful functions for external use of WASI filesystem #595

Add useful functions for external use of WASI filesystem #595

MarkMcCaskey commented Jul 30, 2019 •

edited

Loading

kitlith Jul 30, 2019

MarkMcCaskey Jul 30, 2019

kitlith Jul 30, 2019

kitlith Jul 30, 2019

MarkMcCaskey Jul 31, 2019

kitlith Jul 31, 2019

kitlith commented Jul 30, 2019 •

edited

Loading

MarkMcCaskey commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019

bors bot commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019 •

edited

Loading

MarkMcCaskey commented Jul 31, 2019

bors bot commented Jul 31, 2019

kitlith commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019

Add useful functions for external use of WASI filesystem #595

Add useful functions for external use of WASI filesystem #595

Conversation

MarkMcCaskey commented Jul 30, 2019 • edited Loading

kitlith Jul 30, 2019

Choose a reason for hiding this comment

MarkMcCaskey Jul 30, 2019

Choose a reason for hiding this comment

kitlith Jul 30, 2019

Choose a reason for hiding this comment

kitlith Jul 30, 2019

Choose a reason for hiding this comment

MarkMcCaskey Jul 31, 2019

Choose a reason for hiding this comment

kitlith Jul 31, 2019

Choose a reason for hiding this comment

kitlith commented Jul 30, 2019 • edited Loading

MarkMcCaskey commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019

bors bot commented Jul 31, 2019

Canceled

MarkMcCaskey commented Jul 31, 2019 • edited Loading

MarkMcCaskey commented Jul 31, 2019

bors bot commented Jul 31, 2019

Build succeeded

kitlith commented Jul 31, 2019

MarkMcCaskey commented Jul 31, 2019

MarkMcCaskey commented Jul 30, 2019 •

edited

Loading

kitlith commented Jul 30, 2019 •

edited

Loading

MarkMcCaskey commented Jul 31, 2019 •

edited

Loading