Repository navigation
Compose compile-time builtins in memory - #15025
Conversation
github-actions
Bot
added
the
wasmtime:api
fitzgen
left a comment
fitzgen
left a comment
There was a problem hiding this comment.
Thanks! Overall looks good, but a few things we should address before merging below.
3312b32 to
4566eb6
Compare
The feedback has got me thinking, actually, and maybe this requires an issue to discuss and a 2nd PR, but what if builtins resolved transitively? Like, say an embedder splits builtins into layers that build upon each other, with "ns:host/buffers" and "ns:host/strings" building on "ns:host/memory", as it stands right now, if a guest uses the higher-level builtins on their own then the lower level "ns:host/memory" won't get composed in because it's not a direct guest import. If they resolved transitively then it just works for any import anywhere and works fine based on my local testing. |
This seems reasonable, but yeah should be a dedicated follow up. |
Build the composition with wasm-compose's `CompositionGraph` instead of writing the main Wasm and its builtins to a temporary directory for the file-based `ComponentComposer`. Import names no longer become file paths, so interface names work, and only the registered builtins can be composed in. Also validate with the engine's features, name the builtin in its parse errors, skip composition when no builtin is imported, and drop the `tempfile` dependency.
4566eb6 to
1313c21
Compare
|
The tempfile change triggered a lint in the merge queue on a misplaced tempfile dependency in |
Compose the main Wasm and its compile-time builtins directly with
wasm_compose::graph::CompositionGraphinstead of writing them to a temporary directory and running the file-basedComponentComposer. Only builtins registered on theCodeBuildercan be composed in.This also skips composition when the main Wasm imports no builtin, and drops the
tempfiledependency of thecompile-time-builtinsfeature.This resolves a security issue and fixes various filesystem path related bugs and also adds tests with more realistic name inputs.
Fixes #15024.