Support up to wasmtime 27.0.0 #55
Conversation
Cargo.toml
Outdated
| rustc-demangle = "0.1.23" | ||
| spin_sleep = "1.1.1" | ||
| wasmtime = ">= 15.0.0, < 23.0.0" | ||
| wasmtime = ">= 15.0.0, < 28.0.0" |
There was a problem hiding this comment.
I'm not sure if there's value on keeping this range here. Perhaps we should pin wasmtime to a single version? e.g., 27
There was a problem hiding this comment.
Me neither 🤷 I suppose it could also be problematic since we don't explicitly test on every wasmtime version. I'm not a rust/wasmtime expert so I'll go with whatever you or others prefer!
There was a problem hiding this comment.
I suggest we pin it. The only real consumer of this library is function-runner, which is always on a specific wasmtime version.
There was a problem hiding this comment.
It's a published public crate so we can't know who other consumers may be. That said, there are no other crates on crates.io consuming this crate. So there may also be no other consumers at all.
There was a problem hiding this comment.
Even if there were, by specifying this dependency this way we're saying that this crate is compatible with that range of Wasmtime versions, which: we're not guaranteeing or committing to gurantee in any shape or form, especially since upstream doesn't guarantee compatibility across major versions.
|
The clippy errors are a bit unfortunate. I'm not sure if we should block the upgrade on those, as an alternative we could add clippy rules to disable them and come back to apply the right fix after the upgrade given that mutable statics are really discouraged. |
jeffcharles
left a comment
jeffcharles
left a comment
There was a problem hiding this comment.
The changes themselves look good to me!
The clippy errors are unrelated to the specifics of this PR and have more to do with the Rust stable update that happened in the background. I'm fine if we disable that specific check for this PR with a follow-up in the short-term to try to get rid of those mutable references to mutable statics. Also you can do a fix on main instead of in this PR if you want to try making that fix now.
|
Thanks for the suggestions! That sounds good to me. I allowed the clippy warnings and I'll work on fixing those separately |
This is unreleated to the wasmtime gem bump and will be tackled separately.
jacobsteves
force-pushed
the
js.wasmtime-27
branch
from
c019eaf to
ce97848
Compare
4 participants
Upgrading to latest wasmtime version to ensure function runner compatibility.
I tested generating some profiles on wasmtime 27 and everything looks fine.