Skip to content

Commit

Permalink
Allow plugins when building static modules (#813)
Browse files Browse the repository at this point in the history
* Allow plugins when building static modules

* Make plugin a `-C` option

* Get lint passing

* Change check when removing exports
  • Loading branch information
jeffcharles authored Nov 5, 2024
1 parent b04b7a1 commit 947dceb
Show file tree
Hide file tree
Showing 16 changed files with 196 additions and 20 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ jobs:
name: plugin
path: target/wasm32-wasip1/release/

- name: Build test-plugin
run: |
cargo build --package=javy-test-plugin --release --target=wasm32-wasip1
CARGO_PROFILE_RELEASE_LTO=off cargo build --package=javy-cli --release
target/release/javy init-plugin target/wasm32-wasip1/release/test_plugin.wasm -o crates/runner/test_plugin.wasm
- name: Test CLI
run: CARGO_PROFILE_RELEASE_LTO=off cargo test --package=javy-cli --release -- --nocapture

Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/cli-features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ jobs:
with:
os: linux

- name: Build test-plugin
run: |
cargo build --package=javy-test-plugin --release --target=wasm32-wasip1
CARGO_PROFILE_RELEASE_LTO=off cargo build --package=javy-cli --release
target/release/javy init-plugin target/wasm32-wasip1/release/test_plugin.wasm -o crates/runner/test_plugin.wasm
- name: Test `experimental_event_loop`
run: |
cargo build --package=javy-plugin --target=wasm32-wasip1 --release --features=experimental_event_loop
Expand Down
8 changes: 8 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ members = [
"crates/plugin",
"crates/plugin-api",
"crates/test-macros",
"crates/test-plugin",
"crates/runner",
"fuzz",
]
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ cli: plugin
plugin:
cargo build --package=javy-plugin --release --target=wasm32-wasip1 --features=$(PLUGIN_FEATURES)

build-test-plugin: cli
cargo build --package=javy-test-plugin --release --target=wasm32-wasip1
target/release/javy init-plugin target/wasm32-wasip1/release/test_plugin.wasm -o crates/runner/test_plugin.wasm

docs:
cargo doc --package=javy-cli --open
cargo doc --package=javy-plugin --open --target=wasm32-wasip1
Expand All @@ -33,7 +37,7 @@ test-plugin:
# Test in release mode to skip some debug assertions
# Note: to make this faster, the engine should be optimized beforehand (wasm-strip + wasm-opt).
# Disabling LTO substantially improves compile time
test-cli: plugin
test-cli: plugin build-test-plugin
CARGO_PROFILE_RELEASE_LTO=off cargo test --package=javy-cli --release --features=$(CLI_FEATURES) -- --nocapture

test-runner:
Expand Down
51 changes: 40 additions & 11 deletions crates/cli/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ pub(crate) enum CodeGenType {
// This is an internal detail of this module.
pub(crate) struct Identifiers {
canonical_abi_realloc: FunctionId,
eval_bytecode: FunctionId,
eval_bytecode: Option<FunctionId>,
invoke: FunctionId,
memory: MemoryId,
}

impl Identifiers {
fn new(
canonical_abi_realloc: FunctionId,
eval_bytecode: FunctionId,
eval_bytecode: Option<FunctionId>,
invoke: FunctionId,
memory: MemoryId,
) -> Self {
Expand Down Expand Up @@ -173,7 +173,7 @@ impl Generator {
match self.ty {
CodeGenType::Static => {
let canonical_abi_realloc_fn = module.exports.get_func("canonical_abi_realloc")?;
let eval_bytecode = module.exports.get_func("eval_bytecode")?;
let eval_bytecode = module.exports.get_func("eval_bytecode").ok();
let invoke = module.exports.get_func("invoke")?;
let ExportItem::Memory(memory) = module
.exports
Expand Down Expand Up @@ -226,7 +226,7 @@ impl Generator {

Ok(Identifiers::new(
canonical_abi_realloc_fn_id,
eval_bytecode_fn_id,
Some(eval_bytecode_fn_id),
invoke_fn_id,
memory_id,
))
Expand All @@ -247,7 +247,8 @@ impl Generator {

let mut main = FunctionBuilder::new(&mut module.types, &[], &[]);
let bytecode_ptr_local = module.locals.add(ValType::I32);
main.func_body()
let mut instructions = main.func_body();
instructions
// Allocate memory in plugin instance for bytecode array.
.i32_const(0) // orig ptr
.i32_const(0) // orig size
Expand All @@ -259,11 +260,35 @@ impl Generator {
.i32_const(0) // offset into data segment for mem.init
.i32_const(bytecode_len) // size to copy from data segment
// top-2: dest addr, top-1: offset into source, top-0: size of memory region in bytes.
.memory_init(imports.memory, bytecode_data)
// Evaluate bytecode.
.local_get(bytecode_ptr_local) // ptr to bytecode
.i32_const(bytecode_len)
.call(imports.eval_bytecode);
.memory_init(imports.memory, bytecode_data);
// Evaluate top level scope.
if let Some(eval_bytecode) = imports.eval_bytecode {
instructions
.local_get(bytecode_ptr_local) // ptr to bytecode
.i32_const(bytecode_len)
.call(eval_bytecode);
} else {
// Assert we're not emitting a call with a null function to
// invoke for `javy_quickjs_provider_v*`.
// `javy_quickjs_provider_v2` will never support calling `invoke`
// with a null function. Older `javy_quickjs_provider_v3`'s do not
// support being called with a null function. User plugins and
// newer `javy_quickjs_provider_v3`s do support being called with a
// null function.
// Using `assert!` instead of `debug_assert!` because integration
// tests are executed with Javy built with the release profile so
// `debug_assert!`s are stripped out.
assert!(
self.plugin.is_user_plugin(),
"Using invoke with null function only supported for user plugins"
);
instructions
.local_get(bytecode_ptr_local) // ptr to bytecode
.i32_const(bytecode_len)
.i32_const(0) // set function name ptr to null
.i32_const(0) // set function name len to 0
.call(imports.invoke);
}
let main = main.finish(vec![], &mut module.funcs);

module.exports.add("_start", main);
Expand Down Expand Up @@ -333,7 +358,11 @@ impl Generator {
CodeGenType::Static => {
// Remove no longer necessary exports.
module.exports.remove("canonical_abi_realloc")?;
module.exports.remove("eval_bytecode")?;
// User plugins won't have an `eval_bytecode` function that
// Javy "owns".
if !self.plugin.is_user_plugin() {
module.exports.remove("eval_bytecode")?;
}
module.exports.remove("invoke")?;
module.exports.remove("compile_src")?;

Expand Down
15 changes: 14 additions & 1 deletion crates/cli/src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{js_config::JsConfig, option::OptionMeta, option_group, plugins::Plugin};
use anyhow::{anyhow, Result};
use anyhow::{anyhow, bail, Result};
use clap::{
builder::{StringValueParser, TypedValueParser, ValueParserFactory},
error::ErrorKind,
Expand Down Expand Up @@ -179,6 +179,7 @@ pub struct CodegenOptionGroup {
pub dynamic: bool,
pub wit: WitOptions,
pub source_compression: bool,
pub plugin: Option<PathBuf>,
}

impl Default for CodegenOptionGroup {
Expand All @@ -187,6 +188,7 @@ impl Default for CodegenOptionGroup {
dynamic: false,
wit: WitOptions::default(),
source_compression: true,
plugin: None,
}
}
}
Expand All @@ -206,6 +208,10 @@ option_group! {
/// Enable source code compression, which generates smaller WebAssembly
/// files at the cost of increased compile time.
SourceCompression(bool),
/// Optional path to Javy plugin Wasm module. Not supported for
/// dynamically linked modules. JavaScript config options are also not
/// supported when using this parameter.
Plugin(PathBuf),
}
}

Expand All @@ -223,10 +229,17 @@ impl TryFrom<Vec<GroupOption<CodegenOption>>> for CodegenOptionGroup {
CodegenOption::Wit(path) => wit = Some(path),
CodegenOption::WitWorld(world) => wit_world = Some(world),
CodegenOption::SourceCompression(enabled) => options.source_compression = *enabled,
CodegenOption::Plugin(path) => options.plugin = Some(path.clone()),
}
}

options.wit = WitOptions::from_tuple((wit.cloned(), wit_world.cloned()))?;

// This is temporary. So I can make the change for dynamic modules
// separately because it will be a breaking change.
if options.dynamic && options.plugin.is_some() {
bail!("Cannot use plugins for building dynamic modules");
}
Ok(options)
}
}
Expand Down
10 changes: 7 additions & 3 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,16 @@ fn main() -> Result<()> {
Command::Build(opts) => {
let js = JS::from_file(&opts.input)?;
let codegen: CodegenOptionGroup = opts.codegen.clone().try_into()?;
let plugin = match codegen.plugin {
Some(path) => Plugin::new_user_plugin(&path)?,
None => Plugin::Default,
};
let js_opts = JsConfig::from_group_values(&plugin, opts.js.clone())?;
let mut builder = CodeGenBuilder::new();
builder
.wit_opts(codegen.wit)
.source_compression(codegen.source_compression);

let js_opts = JsConfig::from_group_values(&Plugin::Default, opts.js.clone())?;
.source_compression(codegen.source_compression)
.plugin(plugin);
let mut gen = if codegen.dynamic {
builder.build(CodeGenType::Dynamic, js_opts)?
} else {
Expand Down
20 changes: 18 additions & 2 deletions crates/cli/src/plugins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use serde::Deserialize;
use std::{
fs,
io::{Read, Seek},
path::Path,
str,
};
use walrus::{ExportItem, ValType};
Expand Down Expand Up @@ -33,6 +34,8 @@ pub enum Plugin {
Default,
/// A plugin for use with the `compile` to maintain backward compatibility.
V2,
/// A plugin provided by the user.
User { bytes: Vec<u8> },
}

impl Default for Plugin {
Expand All @@ -42,11 +45,24 @@ impl Default for Plugin {
}

impl Plugin {
/// Creates a new user plugin.
pub fn new_user_plugin(path: &Path) -> Result<Self> {
Ok(Self::User {
bytes: fs::read(path)?,
})
}

/// Returns true if the plugin is a user plugin.
pub fn is_user_plugin(&self) -> bool {
matches!(&self, Plugin::User { .. })
}

/// Returns the plugin Wasm module as a byte slice.
pub fn as_bytes(&self) -> &[u8] {
match self {
Self::Default => PLUGIN_MODULE,
Self::V2 => QUICKJS_PROVIDER_V2_MODULE,
Self::User { bytes } => bytes,
}
}

Expand All @@ -59,7 +75,7 @@ impl Plugin {
pub fn import_namespace(&self) -> Result<String> {
match self {
Self::V2 => Ok("javy_quickjs_provider_v2".to_string()),
Self::Default => {
Self::Default | Self::User { .. } => {
let module = walrus::Module::from_buffer(self.as_bytes())?;
let import_namespace = module
.customs
Expand All @@ -81,7 +97,7 @@ impl Plugin {
/// The JS configuration properties supported by this plugin.
pub fn config_schema(&self) -> Result<Vec<JsConfigProperty>> {
match self {
Self::V2 => Ok(vec![]),
Self::V2 | Self::User { .. } => Ok(vec![]),
Self::Default => {
let engine = Engine::default();
let module = wasmtime::Module::new(&engine, self.as_bytes())?;
Expand Down
13 changes: 12 additions & 1 deletion crates/cli/tests/dynamic_linking_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use anyhow::Result;
use javy_runner::Builder;
use javy_runner::{Builder, Plugin};
use javy_test_macros::javy_cli_test;

#[javy_cli_test(dyn = true, root = "tests/dynamic-linking-scripts")]
Expand Down Expand Up @@ -129,3 +129,14 @@ fn javy_json_identity(builder: &mut Builder) -> Result<()> {

Ok(())
}

#[javy_cli_test(dyn = true, commands(not(Compile)))]
fn test_using_plugin_with_dynamic_build_fails(builder: &mut Builder) -> Result<()> {
let result = builder.plugin(Plugin::User).input("plugin.js").build();
let err = result.err().unwrap();
assert!(err
.to_string()
.contains("Cannot use plugins for building dynamic modules"));

Ok(())
}
28 changes: 27 additions & 1 deletion crates/cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use anyhow::{bail, Result};
use javy_runner::{Builder, Runner, RunnerError};
use javy_runner::{Builder, Plugin, Runner, RunnerError};
use std::{path::PathBuf, process::Command, str};
use wasi_common::sync::WasiCtxBuilder;
use wasmtime::{AsContextMut, Engine, Linker, Module, Store};
Expand Down Expand Up @@ -136,6 +136,32 @@ fn test_javy_json_disabled(builder: &mut Builder) -> Result<()> {
Ok(())
}

#[javy_cli_test(commands(not(Compile)))]
fn test_using_plugin_with_static_build(builder: &mut Builder) -> Result<()> {
let mut runner = builder.plugin(Plugin::User).input("plugin.js").build()?;

let result = runner.exec(&[]);
assert!(result.is_ok());

Ok(())
}

#[javy_cli_test(commands(not(Compile)))]
fn test_using_plugin_with_static_build_fails_with_runtime_config(
builder: &mut Builder,
) -> Result<()> {
let result = builder
.plugin(Plugin::User)
.simd_json_builtins(true)
.build();
let err = result.err().unwrap();
assert!(err
.to_string()
.contains("Property simd-json-builtins is not supported for runtime configuration"));

Ok(())
}

#[javy_cli_test]
fn test_readme_script(builder: &mut Builder) -> Result<()> {
let mut runner = builder.input("readme.js").build()?;
Expand Down
3 changes: 3 additions & 0 deletions crates/cli/tests/sample-scripts/plugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
if (plugin !== true) {
throw new Error("Not using the test_plugin");
}
1 change: 1 addition & 0 deletions crates/runner/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test_plugin.wasm
Loading

0 comments on commit 947dceb

Please sign in to comment.