Delete the wasi-common crate from Wasmtime#13108
Delete the wasi-common crate from Wasmtime#13108alexcrichton wants to merge 2 commits intobytecodealliance:mainfrom
wasi-common crate from Wasmtime#13108Conversation
This crate has been languishing without maintenance within this
repository for a number of years now and I think it's time to remove it.
This commit proposes removing it without replacement.
Some history of the `wasi-common` crate:
* This crate is the original implementation of WASIp1 dating back to
2020 or so.
* The `wasmtime-wasi` crate was a reimplementation of WASIp1 and WASIp2
introduced with the component model. Upon introduction the
`wasmtime-wasi` crate was off-by-default for WASIp1.
* The `wasmtime` CLI eventually grew a `-Spreview2={y|n}` for
controlling which implementation to use. Perhaps a bit confusingly
`-Spreview2` affects the implementation of WASIp1 when running core
modules as well.
* Eventually `-Spreview2` was enabled by default meaning that core
modules in the `wasmtime` CLI used the `wasmtime-wasi` implementation
by default.
* Since then `wasi-common` has been on life support and has received
little-to-no updates. All maintenance and work is focused on
`wasmtime-wasi` while `wasi-common` is more-or-less ignored.
The main reason `wasi-common` has stuck around until now is that it's
the only way to use WASIp1 APIs when enabling the experimental
`wasi-threads` proposal. The `wasmtime-wasi` crate architecturally does
not support threads, so it cannot fill this niche.
This begs the question of: why remove the crate now? The rationale I
have is:
* The writing has been on the wall for `wasmtime-wasi` for quite a long
time. This is an acknowledgement that it's there.
* The `wasi-threads` proposal as-is is a dead-end and this has long been
known. Further work on that proposal is all expected to go down the
route of shared-everything-threads, which is a drastically different
approach than what it is today.
* The dependency burden of `wasi-common` is real and cannot be
overlooked. We've still had to update code in `wasi-common` for crate
updates and such and this, while small, is not trivial. I'm recently
trying to remove the `system-interface` dependency from Wasmtime and
it's deeply embedded within `wasi-common` as an example.
* Users being able to run at least cooperatively threaded code is
expected to be possible with WASIp3. This is expected to land this
year which means users will, eventually, have a path forward for "just
run code that uses pthreads".
* Wasmtime still supports this crate in the LTS releases. Wasmtime 36,
for example, is supported until August 2027 which means that we're
still maintaining it for another 1.5 years no matter what. This,
however, is also a reason to remove it before the next LTS to avoid
furthering its ongoing maintenance.
Overall I personally feel that the calculus has pointed towards the
removal of `wasi-common` for quite a long time now and we just haven't
gotten around to it. Thus, this commit gets around to it. The concrete
changes here in this commit are:
* The `wasi-common` crate is deleted.
* The `-Spreview2=n` flag on the `wasmtime` CLI now unconditionally
prints an error saying it's unsupported.
* The `-Sthreads` flag now requires that `-Scli=n` is passed
(wasi-threads is incompatible with the implementation of WASIp1)
* Tests are updated in a few locations as well.
This commit removes the `system-interface` dependency from `wasi-common`, reimplementing necessary functionality within the crate itself as appropriate. The goal of this commit is to trim our dependency tree. The `system-interface` crate has not received an update in over a year and continues to pull in an older `rustix` dependency for example. Additionally I've personally found it confusing and surprising in the past to trace through all the layers of abstractions from `wasmtime-wasi` to the OS, and I'd like to start slimming this down to be more local within Wasmtime rather than depending on a tree of crates. The `system-interface` crate is a relatively thin wrapper around `cap-std`-style crates providing a platform-agnostic API. This sometimes fits what WASI wants, and sometimes doesn't. For example all reads/writes to files within WASI currently require that Wasmtime maintains a file cursor itself meaning that the underlying OS file cursor doesn't actually matter. Reads and writes through the `system-interface` abstraction, however, keep the cursor up-to-date to have the same semantics across Unix and Windows which differ in the behavior of the underlying syscalls. This is unnecessarily adds overhead to Wasmtime's implementation of these APIs where they're otherwise not required. Effectively `system-interface` is not receiving much maintenance (old dependency on `rustix` has persisted for ~1 year), its an extra layer of abstraction to debug when issues arise, and its abstractions are not always the best fit for WASI's semantics meaning that it can add performance overhead. The replacement of inlining implementations within Wasmtime is not too too costly and, personally, I view as worth it. I'll note that this doesn't delete the `system-interface` crate entirely. That would require removing it from `wasi-common` as well, which is the subject of bytecodealliance#13108.
This commit removes the `system-interface` dependency from `wasi-common`, reimplementing necessary functionality within the crate itself as appropriate. The goal of this commit is to trim our dependency tree. The `system-interface` crate has not received an update in over a year and continues to pull in an older `rustix` dependency for example. Additionally I've personally found it confusing and surprising in the past to trace through all the layers of abstractions from `wasmtime-wasi` to the OS, and I'd like to start slimming this down to be more local within Wasmtime rather than depending on a tree of crates. The `system-interface` crate is a relatively thin wrapper around `cap-std`-style crates providing a platform-agnostic API. This sometimes fits what WASI wants, and sometimes doesn't. For example all reads/writes to files within WASI currently require that Wasmtime maintains a file cursor itself meaning that the underlying OS file cursor doesn't actually matter. Reads and writes through the `system-interface` abstraction, however, keep the cursor up-to-date to have the same semantics across Unix and Windows which differ in the behavior of the underlying syscalls. This is unnecessarily adds overhead to Wasmtime's implementation of these APIs where they're otherwise not required. Effectively `system-interface` is not receiving much maintenance (old dependency on `rustix` has persisted for ~1 year), its an extra layer of abstraction to debug when issues arise, and its abstractions are not always the best fit for WASI's semantics meaning that it can add performance overhead. The replacement of inlining implementations within Wasmtime is not too too costly and, personally, I view as worth it. I'll note that this doesn't delete the `system-interface` crate entirely. That would require removing it from `wasi-common` as well, which is the subject of bytecodealliance#13108.
cfallin
left a comment
There was a problem hiding this comment.
Removal looks OK to me technically!
As a policy decision this should probably be discussed at the next Wasmtime meeting just for completeness / formal approval (I personally agree completely with your rationale here but don't want to speak for the whole project).
| host.legacy_p1_ctx.as_mut().unwrap() | ||
| })?; | ||
| self.set_legacy_p1_ctx(store)?; | ||
| (Some(false), _) => { |
There was a problem hiding this comment.
Should we update the comment just above this to no longer refer to the "historical preview1 implementation"?
pchickey
left a comment
There was a problem hiding this comment.
As the one of the authors of wasi-common I strongly support this change - its definitely time to sunset this legacy code. I agree with Chris that we can discuss it at a Wasmtime project meeting just to give everyone time and opportunity to weigh in.
For anyone who still needs wasi-common, they can use it on the LTS releases until their support term is up, but removing this crate from the tree frees us from a real maintenance burden and is the right path for the project.
The -Sthreads flag now requires that -Scli=n is passed (wasi-threads is incompatible with the implementation of WASIp1)
I think that this means that, effectively, there are no more programs that can make use of the wasi-threads proposal in wasmtime.
wasi-threads was always proposed and used a superset of wasip1, and since now the implementation won't support the wasip1 imports at all, we should also be sunsetting the wasi-threads implementation at the same time as we sunset wasi-common.
As you said in the justification for removing wasi-common, the entire story behind wasi-threads has long been known to be a dead end, and with wasip3 we now have a cooperative threads implementation on track to land, and shared-everything threads will follow after that.
This commit removes the `system-interface` dependency from `wasi-common`, reimplementing necessary functionality within the crate itself as appropriate. The goal of this commit is to trim our dependency tree. The `system-interface` crate has not received an update in over a year and continues to pull in an older `rustix` dependency for example. Additionally I've personally found it confusing and surprising in the past to trace through all the layers of abstractions from `wasmtime-wasi` to the OS, and I'd like to start slimming this down to be more local within Wasmtime rather than depending on a tree of crates. The `system-interface` crate is a relatively thin wrapper around `cap-std`-style crates providing a platform-agnostic API. This sometimes fits what WASI wants, and sometimes doesn't. For example all reads/writes to files within WASI currently require that Wasmtime maintains a file cursor itself meaning that the underlying OS file cursor doesn't actually matter. Reads and writes through the `system-interface` abstraction, however, keep the cursor up-to-date to have the same semantics across Unix and Windows which differ in the behavior of the underlying syscalls. This is unnecessarily adds overhead to Wasmtime's implementation of these APIs where they're otherwise not required. Effectively `system-interface` is not receiving much maintenance (old dependency on `rustix` has persisted for ~1 year), its an extra layer of abstraction to debug when issues arise, and its abstractions are not always the best fit for WASI's semantics meaning that it can add performance overhead. The replacement of inlining implementations within Wasmtime is not too too costly and, personally, I view as worth it. I'll note that this doesn't delete the `system-interface` crate entirely. That would require removing it from `wasi-common` as well, which is the subject of bytecodealliance#13108.
This commit removes the `system-interface` dependency from `wasi-common`, reimplementing necessary functionality within the crate itself as appropriate. The goal of this commit is to trim our dependency tree. The `system-interface` crate has not received an update in over a year and continues to pull in an older `rustix` dependency for example. Additionally I've personally found it confusing and surprising in the past to trace through all the layers of abstractions from `wasmtime-wasi` to the OS, and I'd like to start slimming this down to be more local within Wasmtime rather than depending on a tree of crates. The `system-interface` crate is a relatively thin wrapper around `cap-std`-style crates providing a platform-agnostic API. This sometimes fits what WASI wants, and sometimes doesn't. For example all reads/writes to files within WASI currently require that Wasmtime maintains a file cursor itself meaning that the underlying OS file cursor doesn't actually matter. Reads and writes through the `system-interface` abstraction, however, keep the cursor up-to-date to have the same semantics across Unix and Windows which differ in the behavior of the underlying syscalls. This is unnecessarily adds overhead to Wasmtime's implementation of these APIs where they're otherwise not required. Effectively `system-interface` is not receiving much maintenance (old dependency on `rustix` has persisted for ~1 year), its an extra layer of abstraction to debug when issues arise, and its abstractions are not always the best fit for WASI's semantics meaning that it can add performance overhead. The replacement of inlining implementations within Wasmtime is not too too costly and, personally, I view as worth it. I'll note that this doesn't delete the `system-interface` crate entirely. That would require removing it from `wasi-common` as well, which is the subject of bytecodealliance#13108.
* Remove `system-interface` from `wasi-common` This commit removes the `system-interface` dependency from `wasi-common`, reimplementing necessary functionality within the crate itself as appropriate. The goal of this commit is to trim our dependency tree. The `system-interface` crate has not received an update in over a year and continues to pull in an older `rustix` dependency for example. Additionally I've personally found it confusing and surprising in the past to trace through all the layers of abstractions from `wasmtime-wasi` to the OS, and I'd like to start slimming this down to be more local within Wasmtime rather than depending on a tree of crates. The `system-interface` crate is a relatively thin wrapper around `cap-std`-style crates providing a platform-agnostic API. This sometimes fits what WASI wants, and sometimes doesn't. For example all reads/writes to files within WASI currently require that Wasmtime maintains a file cursor itself meaning that the underlying OS file cursor doesn't actually matter. Reads and writes through the `system-interface` abstraction, however, keep the cursor up-to-date to have the same semantics across Unix and Windows which differ in the behavior of the underlying syscalls. This is unnecessarily adds overhead to Wasmtime's implementation of these APIs where they're otherwise not required. Effectively `system-interface` is not receiving much maintenance (old dependency on `rustix` has persisted for ~1 year), its an extra layer of abstraction to debug when issues arise, and its abstractions are not always the best fit for WASI's semantics meaning that it can add performance overhead. The replacement of inlining implementations within Wasmtime is not too too costly and, personally, I view as worth it. I'll note that this doesn't delete the `system-interface` crate entirely. That would require removing it from `wasi-common` as well, which is the subject of #13108. * Portability fixes prtest:full * More portability fixes * Allow some more errors on Windows * Try to fix test on Windows * Fix test issue * Limit pwritev2 to linux
This crate has been languishing without maintenance within this repository for a number of years now and I think it's time to remove it. This commit proposes removing it without replacement.
Some history of the
wasi-commoncrate:wasmtime-wasicrate was a reimplementation of WASIp1 and WASIp2 introduced with the component model. Upon introduction thewasmtime-wasicrate was off-by-default for WASIp1.wasmtimeCLI eventually grew a-Spreview2={y|n}for controlling which implementation to use. Perhaps a bit confusingly-Spreview2affects the implementation of WASIp1 when running core modules as well.-Spreview2was enabled by default meaning that core modules in thewasmtimeCLI used thewasmtime-wasiimplementation by default.wasi-commonhas been on life support and has received little-to-no updates. All maintenance and work is focused onwasmtime-wasiwhilewasi-commonis more-or-less ignored.The main reason
wasi-commonhas stuck around until now is that it's the only way to use WASIp1 APIs when enabling the experimentalwasi-threadsproposal. Thewasmtime-wasicrate architecturally does not support threads, so it cannot fill this niche.This begs the question of: why remove the crate now? The rationale I have is:
wasi-commonfacing end-of-life for quite a long time. This is an acknowledgement that it's there.wasi-threadsproposal as-is is a dead-end and this has long been known. Further work on that proposal is all expected to go down the route of shared-everything-threads, which is a drastically different approach than what it is today.wasi-commonis real and cannot be overlooked. We've still had to update code inwasi-commonfor crate updates and such and this, while small, is not trivial. I'm recently trying to remove thesystem-interfacedependency from Wasmtime and it's deeply embedded withinwasi-commonas an example.Overall I personally feel that the calculus has pointed towards the removal of
wasi-commonfor quite a long time now and we just haven't gotten around to it. Thus, this commit gets around to it. The concrete changes here in this commit are:wasi-commoncrate is deleted.-Spreview2=nflag on thewasmtimeCLI now unconditionally prints an error saying it's unsupported.-Sthreadsflag now requires that-Scli=nis passed (wasi-threads is incompatible with the implementation of WASIp1)