Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 104 additions & 1 deletion ext/src/ruby_api/wasi_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@ use crate::error;
use crate::helpers::OutputLimitedBuffer;
use magnus::{
class, function, gc::Marker, method, typed_data::Obj, value::Opaque, DataTypeFunctions, Error,
Module, Object, RArray, RHash, RString, Ruby, TryConvert, TypedData,
Module, Object, RArray, RHash, RString, Ruby, Symbol, TryConvert, TypedData,
};
use rb_sys::ruby_rarray_flags::RARRAY_EMBED_FLAG;
use std::cell::RefCell;
use std::fs;
use std::path::Path;
use std::{fs::File, path::PathBuf};
use wasmtime_wasi::p2::pipe::MemoryInputPipe;
use wasmtime_wasi::p2::{OutputFile, WasiCtx, WasiCtxBuilder};
use wasmtime_wasi::preview1::WasiP1Ctx;
use wasmtime_wasi::{DirPerms, FilePerms};

enum ReadStream {
Inherit,
Expand Down Expand Up @@ -51,6 +54,7 @@ struct WasiConfigInner {
env: Option<Opaque<RHash>>,
args: Option<Opaque<RArray>>,
deterministic: bool,
mapped_directories: Option<Opaque<RArray>>,
}

impl WasiConfigInner {
Expand All @@ -70,6 +74,9 @@ impl WasiConfigInner {
if let Some(v) = self.args.as_ref() {
marker.mark(*v);
}
if let Some(v) = self.mapped_directories.as_ref() {
marker.mark(*v);
}
}
}

Expand Down Expand Up @@ -233,6 +240,40 @@ impl WasiConfig {
rb_self
}

/// @yard
/// Set mapped directory for host path and guest path.
/// @param host_path [String]
/// @param guest_path [String]
/// @param dir_perms [Symbol] Directory permissions, one of :read, :mutate, or :all
/// @param file_perms [Symbol] File permissions, one of :read, :write, or :all
/// @def set_mapped_directory(host_path, guest_path, dir_perms, file_perms)
/// @return [WasiConfig] +self+
pub fn set_mapped_directory(
rb_self: RbSelf,
host_path: RString,
guest_path: RString,
dir_perms: Symbol,
file_perms: Symbol,
) -> RbSelf {
let mapped_directory = RArray::new();
mapped_directory.push(host_path).unwrap();
mapped_directory.push(guest_path).unwrap();
mapped_directory.push(dir_perms).unwrap();
mapped_directory.push(file_perms).unwrap();

let init_directory = RArray::new();
Copy link
Member

Choose a reason for hiding this comment

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

Should the creation of this RArray be moved under the if on line 267? Else we'd be creating an array even though it won't be needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would we consider storing these in a Vec<SomeStruct> instead of nested RArrays?

Pros:

  • Sidesteps the refcell+alloc issue altogether (the top-level RArray is gone).
  • Fewer Ruby allocations: no need to alloc an Array to store the arguments for each call.
  • Errors converting Ruby args into WasiCtxBuilder.preopened_dir-compatible args will show up immediately, not delayed in a future call. Tiny detail, but makes errors easier to debug.

Cons:

  • Requires implementing such struct to hold the arguments, and marking the string it contains.
  • Something else I'm not seeing?

I didn't mean to hijack the PR, feel free to ignore if not useful.

Copy link
Member

Choose a reason for hiding this comment

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

I think the suggestion is reasonable, personally the main advantage that I see, is not having to incur in the overhead of creating a Ruby array each time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I switched to using a Vec and there are still no garbage collection problems.


let mut inner = rb_self.inner.borrow_mut();
if inner.mapped_directories.is_none() {
inner.mapped_directories = Some(init_directory.into());
}

let ruby = Ruby::get().unwrap();
let mapped_directories = ruby.get_inner(inner.mapped_directories.unwrap());
mapped_directories.push(mapped_directory).unwrap();
rb_self
}

pub fn build_p1(&self, ruby: &Ruby) -> Result<WasiP1Ctx, Error> {
let mut builder = self.build_impl(ruby)?;
let ctx = builder.build_p1();
Expand Down Expand Up @@ -317,6 +358,63 @@ impl WasiConfig {
deterministic_wasi_ctx::add_determinism_to_wasi_ctx_builder(&mut builder);
}

if let Some(mapped_directories) = inner.mapped_directories.as_ref() {
for item in unsafe { ruby.get_inner(*mapped_directories).as_slice() } {
let mapped_directory = RArray::try_convert(*item)?;
if mapped_directory.len() == 4 {
let host_path =
RString::try_convert(mapped_directory.entry(0)?)?.to_string()?;
let guest_path =
RString::try_convert(mapped_directory.entry(1)?)?.to_string()?;
let dir_perms = Symbol::from_value(mapped_directory.entry(2)?)
.unwrap()
.name()?;
let file_perms = Symbol::from_value(mapped_directory.entry(3)?)
.unwrap()
.name()?;

let host_path_dir = Path::new(&host_path);
let guest_path_path = guest_path.as_str();

// Convert to FilePerms and DirPerms enums
let dir_perms_flags;
match dir_perms {
std::borrow::Cow::Borrowed("read") => dir_perms_flags = DirPerms::READ,
std::borrow::Cow::Borrowed("mutate") => dir_perms_flags = DirPerms::MUTATE,
std::borrow::Cow::Borrowed("all") => dir_perms_flags = DirPerms::all(),
_ => {
return Err(error!(
"Invalid dir_perms: {}. Use one of :read, :mutate, or :all",
dir_perms
))
}
}

let file_perms_flags;
match file_perms {
std::borrow::Cow::Borrowed("read") => file_perms_flags = FilePerms::READ,
std::borrow::Cow::Borrowed("write") => file_perms_flags = FilePerms::WRITE,
std::borrow::Cow::Borrowed("all") => file_perms_flags = FilePerms::all(),
_ => {
return Err(error!(
"Invalid file_perms: {}. Use one of :read, :write, or :all",
file_perms
))
}
}

builder
.preopened_dir(
host_path_dir,
guest_path_path,
dir_perms_flags,
file_perms_flags,
)
.map_err(|e| error!("{}", e))?;
}
}
}

Ok(builder)
}
}
Expand Down Expand Up @@ -355,5 +453,10 @@ pub fn init() -> Result<(), Error> {

class.define_method("set_argv", method!(WasiConfig::set_argv, 1))?;

class.define_method(
"set_mapped_directory",
method!(WasiConfig::set_mapped_directory, 4),
)?;

Ok(())
}