-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
refactor: introduce p2
module
#10073
base: main
Are you sure you want to change the base?
Conversation
6d44773
to
815b81a
Compare
Signed-off-by: Roman Volosatovs <[email protected]>
815b81a
to
29b9cad
Compare
Signed-off-by: Roman Volosatovs <[email protected]>
cec5af9
to
94e95e6
Compare
Thanks for this! I'm going to continue the discussion from #10061 over here since this looks like it's going to be first. I'll note that whatever we end up doing here for p3 is a relatively big change to consider depending on how it ends up which sort of borders along the lines of "maybe this should have an RFC". For example upon reading reading Pat's comment I initially reacted thinking that we should instead do something else. After thinking more though this seems like a more reasonable approach. That being said though I'd at least personally still have thoughts on this, for example:
I'll also note though that I'm not necessarily saying this requires an RFC. I find though that it's not always the greatest medium to have a design discussion when there's a PR because it's easy to get into the mindset of "well the PR does it this way so I guess we'll just go with that". RFCs have their own downsides of course though. In the abstract though I think we should ideally design for where we want to end up a year or two from now. At that point WASIp3 is stable and will be the "primary" APIs that folks use. Given our destination end point first then I think we can work backwards and consider things like breaking changes, refactorings, migration paths, etc. I've historically found that only designing in incremental steps from where we are today, for example trying to minimize breaking changes, doesn't always result in the best design. |
I agree with all of Alex's broad strokes here. I also want to point out that landing PRs to support wasip3 in wasmtime
Agree, I don't like
Eventually to never would also be my prioritization. Consistency is nice in the abstract, but in this case I don't think its very important, since the interfaces being exposed by preview0 and 1 are for witx, and the rest are for components anyway. And its definitely not urgent to change this, since it would break existing users.
Agree - lets come up with a repeatable pattern that can be applied to other crates as needed, but lets start by only applying it to wasmtime-wasi now and apply it to others immediately before landing a 0.3-draft impl in those. That way, if we discover in the buildout of wasmtime-wasi that the pattern isnt quite right, we can course-correct with a minimum of thrash. |
Oh to clarify for the pub use personally I prefer names to only be in one location as opposed to multiple, so I would advocate for moving most of the preexisting crate to Roman would you be up for making that change and reverting other crates to their original state? That I think should create enough space to start experimenting with p3 in the main crate would be my hope. |
Introduce a
p2
module in WASI crates as suggested in #10061 (review)This will allow us to iterate on wasip3 without breaking changes
I tried to minimize the diff and only moved the WIT files, generated bindings and host implementations to
p2
modules.Abstractions (like
WasiCtx
) are left in place, since it's likely thatp3
modules will (mostly) reuse the same abstractions.Since
wasi-nn
seemed a bit different from other crates, I left it as-is. I'm also not aware of any plans of introducing a new version of it for wasip3.I also opted not to rename existing
preview1
andpreview0
modules/features to avoid breaking changes.cc @pchickey