Allow plugins when building static modules #813
Conversation
| // 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!( |
There was a problem hiding this comment.
My plan is to relax this assertion a bit after we have released a new version of javy_quickjs_provider_v3.
jeffcharles
force-pushed
the
jc.allow-static-plugins
branch
from
900a5cc to
e8f8452
Compare
crates/cli/src/codegen/mod.rs
Outdated
| // `eval_bytecode` is not present in user plugins and `remove` | ||
| // returns an error if the export is not present. | ||
| if module.exports.get_func("eval_bytecode").is_ok() { | ||
| module.exports.remove("eval_bytecode")?; | ||
| } |
There was a problem hiding this comment.
This is true because we know the plugin API surface, however, I don't think anything is preventing a plugin from exporting a function named eval_bytecode for their own purposes. Probably unlikely, but not impossible. I'd suggest checking if the plugin is not a user provided plugin before removing this export.
There was a problem hiding this comment.
I've updated the check
crates/cli/src/commands.rs
Outdated
|
|
||
| #[arg(short, long)] | ||
| /// Path of the plugin. | ||
| pub plugin: Option<PathBuf>, |
There was a problem hiding this comment.
Any reason why the plugin flag is not scoped? In the What are the changes? section of the RFC, the plugin handling is scoped under the -C option group, which is what I think makes most sense, since it's easier to validate the semantic meaning of the group as a whole (e.g., -C pluing=.. is required with -C dynamic)
There was a problem hiding this comment.
That was an oversight on my part
Description of the change
Add a
-por--pluginflag tojavy buildto support using a user-provided plugin. I've disabled allowing plugins on dynamically linked modules in this PR since I want to make that a required parameter and that would represent a breaking change. I also don't want a version of Javy shipped where it is an optional parameter.I've also added a test-plugin crate for use by the CLI tests so the tests can verify the plugin is being actually used by the generated Wasm module.
Why am I making this change?
Part of #768. This enables using plugins for statically linked modules.
I opted to compile the test plugin as part of the Makefile and CI steps but I'm open to switching it so we commit the final artifact if that would be preferrable.
Checklist
javy-cliandjavy-plugindo not require updating CHANGELOG files.