wasmtime-rb icon indicating copy to clipboard operation
wasmtime-rb copied to clipboard

Implement preopened_dir using set_mapped_directories setting

Open alhafoudh opened this issue 1 year ago • 4 comments

alhafoudh avatar Feb 06 '25 06:02 alhafoudh

Wasmtime's API for this takes 1 dir at a time -- not a collection of (src, dest). Unless we have a reason to, I'd rather stick to the Rust API shape and naming.

Sticking to the rust crate API been a general rule of thumb for the Ruby bindings. In this case, it makes the API easier to describe. Especially as we eventually migrate to wasmtime_wasi instead of wasi_common which has additional params for permissions.

Should I just append one by one to the array and then call the builder.preopened_dir() by iterating the array?

alhafoudh avatar Feb 06 '25 15:02 alhafoudh

Should I just append one by one to the array and then call the builder.preopened_dir() by iterating the array?

Sounds reasonable to me.

The array should contain RStrings to avoid memcopys from owned Strings ((RString, RString) maybe?). And then, when creating the paths / bufs, try and avoid creating owned strings with RString::as_str (unsafe). Note: this only works if we are guaranteed to not allocate for the duration of the str reference, which I think here we can guarantee.

Let me know once you're done with the change and the tests.

jbourassa avatar Feb 06 '25 15:02 jbourassa

I have updated the way how set_mapped_directory works. This is my first time writing rust, sorry. I dont have the insight into the magnus, rust and memory management alltogether. :-) Maybe can you finish it up so it is written correctly?

alhafoudh avatar Feb 06 '25 15:02 alhafoudh

I can take a look at cleaning it up once the tests are in -- care to add those in?

jbourassa avatar Feb 06 '25 15:02 jbourassa