Implement WASI preview 2 plugin RFC #958
Conversation
jeffcharles
force-pushed
the
jc.wasi-p2
branch
from
8a894f6 to
a89723c
Compare
jeffcharles
force-pushed
the
jc.wasi-p2
branch
from
a89723c to
4e8add0
Compare
jeffcharles
changed the title
crates/cli/src/plugin.rs
Outdated
|
|
||
| fn validate(plugin_bytes: &'a [u8]) -> Result<()> { | ||
| let plugin_bytes = match javy_plugin_processing::extract_core_module(plugin_bytes) { | ||
| Err(e) => bail!("Problem with plugin: {e}"), |
There was a problem hiding this comment.
Nit:
| Err(e) => bail!("Problem with plugin: {e}"), | |
| Err(e) => bail!("Could not process plugin: {e}"), |
There was a problem hiding this comment.
Or maybe "Could not validate plugin" ?
crates/codegen/src/plugin.rs
Outdated
| @@ -81,12 +97,16 @@ impl Plugin { | |||
|
|
|||
| let module = walrus::Module::from_buffer(plugin_bytes)?; | |||
|
|
|||
| if let Err(err) = validate_exported_func(&module, "initialize_runtime", &[], &[]) { | |||
| if module.exports.get_func("compile_src").is_ok() { | |||
| bail!("Problem with plugin: Using unsupported legacy plugin API"); | |||
There was a problem hiding this comment.
Similar here? Failed to validate? Or something along those lines?
crates/codegen/src/lib.rs
Outdated
There was a problem hiding this comment.
Given how the functionality in this module has grown in complexity, I think it'd really useful to introduce snapshot testing for the generated code, mainly to make it easier for reviewers to have an better idea of how the generated code looks like, when introducing net new code, and also to make it easier to spot any new changes to existing code.
crates/plugin-processing/src/lib.rs
Outdated
| if !Parser::is_component(component_bytes) { | ||
| if !Parser::is_core_wasm(component_bytes) { | ||
| bail!("Expected Wasm component, received unknown file type"); | ||
| } else { | ||
| bail!("Expected Wasm component, received Wasm module"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm finding this a bit hard to read, couldn't we do something like:
if !Parser::is_component(...) && !Parser::is_core_wasm{ bail!(...) }
if !Parser::is_component(...) { bail!(...) }I think it's mostly the nested if which I find hard to read.
Description of the change
This implements #962 to have plugins target WASI preview 2. This would represent a breaking change for the Javy plugin API in the following ways:
compile_src: func(bytecode_ptr: u32, bytecode_len: u32) -> ()becomescompile-src: func(bytecode: list<u8>) -> result<list<u8>, string>initialize_runtime: func() -> ()becomesinitialize-runtime: func() -> ()invoke: func(bytecode_ptr: u32, bytecode_len: u32, fn_name_ptr: u32, fn_name_len: u32) -> ()becomesinvoke: func(bytecode: list<u8>, function: option<string>);canonical_abi_realloc(old_ptr: u32, old_size: u32, alignment: u32, new_size: u32)becomescabi_realloc(old_ptr: u32, old_size: u32, alignment: u32, new_size: u32)Why am I making this change?
WASI preview 1 will be deprecated at some point in the future. We should use WASI preview 2 to ensure we can continue to compile Javy.
Checklist
javy-cliandjavy-plugindo not require updating CHANGELOG files.