-
Notifications
You must be signed in to change notification settings - Fork 139
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
Disable wat
feature from wasmer
and wasmtime
#2586
Conversation
Instead of only running the tests if the feature is enabled. Also enable the `wat` feature in preparation to remove the `sys-default` feature.
It doesn't seem to be needed.
Disable the default features on `wasmer-compiler-singlepass`.
Only enable the ones that are needed.
b3a5fb1
to
28edac0
Compare
@@ -404,7 +404,7 @@ impl<'bytecode> Sanitizer<'bytecode> { | |||
} | |||
} | |||
|
|||
#[cfg(all(test, with_wasmer))] | |||
#[cfg(test)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always test with Wasmer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we have to :/ Because otherwise we can't enable the wat
feature only for tests :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:(
Relevant discussion: rust-lang/cargo#1596
Motivation
We've recently had dependency version conflicts for
wat
betweenwasmer
andwasmtime
, which required releasing a new version of ourlinera-wasmer
work every time an update forwasmtime
was needed (#2427, #2530). That dependency onwat
isn't actually needed on either runtime, except for a couple of tests.Proposal
Disable the
wat
features and additional features fromwasmer
andwasmtime
.Because
dev-dependencies
can't have optional dependencies, nowlinera-execution
tests requirewasmer
so that it can enable thewat
feature.Test Plan
CI should catch regressions, since this just changes dependencies.
Release Plan
Links