Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow plugins when building static modules #813

Merged
merged 4 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
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!(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My plan is to relax this assertion a bit after we have released a new version of javy_quickjs_provider_v3.

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
Loading