Stream: git-wasmtime

Topic: wasmtime / PR #10073 refactor: introduce `p2` module

Wasmtime GitHub notifications bot (Jan 22 2025 at 10:50):

rvolosatovs opened PR #10073 from rvolosatovs:feat/p2-module to bytecodealliance:main:

Introduce a p2 module in WASI crates as suggested in https://github.com/bytecodealliance/wasmtime/pull/10061#pullrequestreview-2565638530

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 that p3 modules will reuse the same abstractions.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:00):

rvolosatovs updated PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:02):

rvolosatovs edited PR #10073:

Introduce a p2 module in WASI crates as suggested in https://github.com/bytecodealliance/wasmtime/pull/10061#pullrequestreview-2565638530

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 that p3 modules will (mostly) reuse the same abstractions.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:03):

rvolosatovs updated PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:05):

rvolosatovs updated PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:07):

rvolosatovs updated PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:08):

rvolosatovs has marked PR #10073 as ready for review.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:08):

rvolosatovs requested fitzgen for a review on PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:08):

rvolosatovs requested wasmtime-core-reviewers for a review on PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:08):

rvolosatovs requested wasmtime-default-reviewers for a review on PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:08):

rvolosatovs edited PR #10073:

Introduce a p2 module in WASI crates as suggested in https://github.com/bytecodealliance/wasmtime/pull/10061#pullrequestreview-2565638530

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 that p3 modules will (mostly) reuse the same abstractions.

cc @pchickey

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:12):

rvolosatovs edited PR #10073:

Introduce a p2 module in WASI crates as suggested in https://github.com/bytecodealliance/wasmtime/pull/10061#pullrequestreview-2565638530

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 that p3 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.
cc @pchickey

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:44):

rvolosatovs updated PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:46):

rvolosatovs updated PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 11:48):

rvolosatovs edited PR #10073:

Introduce a p2 module in WASI crates as suggested in https://github.com/bytecodealliance/wasmtime/pull/10061#pullrequestreview-2565638530

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 that p3 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 and preview0 modules/features to avoid breaking changes.
cc @pchickey

Wasmtime GitHub notifications bot (Jan 22 2025 at 17:34):

alexcrichton requested alexcrichton for a review on PR #10073.

Wasmtime GitHub notifications bot (Jan 22 2025 at 17:41):

alexcrichton commented on PR #10073:

Thanks for this! I'm going to continue the discussion from https://github.com/bytecodealliance/wasmtime/pull/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'm not sure if we want to keep a pub use p2::* reexport myself.
• This probably wants to (eventually, not necessarily here), come with a rename of the preview0 and preview1 modules to p0 and p1.
• We might want to hold off on changing other wasmtime-wasi-* crates for now until APIs are ready for those. Basically pave a path with the core wasmtime-wasi crate but otherwise defer the actual changes to future crates to when we've shaken out all the issues here.

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.

Wasmtime GitHub notifications bot (Jan 22 2025 at 21:53):

pchickey commented on PR #10073:

I agree with all of Alex's broad strokes here.

I also want to point out that landing PRs to support wasip3 in wasmtime main doesn't feel urgent to me. In the corresponding point in the p2 development process, we forked off a prototyping repo where we could thrash things around a bunch and not worry about compatibility as we iterated on the specs and implementations towards working code. So, aside from benefiting from more discussion of our desired end state and working backwards to figure out the changes and migrations to get there, I personally would benefit from seeing a more fleshed out implementation of p3 looks like. Currently it is spread among several different authors, repos, and PRs. Additionally, right now my employer is asking me to solve a totally different set of problems, so I don't have the bandwidth to engage with the p3 implementation process deep enough to collect all that context.

I'm not sure if we want to keep a pub use p2::* reexport myself.
Agree, I don't like use *. Please list all of the identifiers re-exported. (Is that the aspect @alexcrichton objects to?)

This probably wants to (eventually, not necessarily here), come with a rename of the preview0 and preview1 modules to p0 and p1.
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.

We might want to hold off on changing other wasmtime-wasi-* crates for now until APIs are ready for those. Basically pave a path with the core wasmtime-wasi crate but otherwise defer the actual changes to future crates to when we've shaken out all the issues here.

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.

Wasmtime GitHub notifications bot (Jan 22 2025 at 21:53):

pchickey edited a comment on PR #10073:

I agree with all of Alex's broad strokes here.

I also want to point out that landing PRs to support wasip3 in wasmtime main doesn't feel urgent to me. In the corresponding point in the p2 development process, we forked off a prototyping repo where we could thrash things around a bunch and not worry about compatibility as we iterated on the specs and implementations towards working code. So, aside from benefiting from more discussion of our desired end state and working backwards to figure out the changes and migrations to get there, I personally would benefit from seeing a more fleshed out implementation of p3 looks like. Currently it is spread among several different authors, repos, and PRs. Additionally, right now my employer is asking me to solve a totally different set of problems, so I don't have the bandwidth to engage with the p3 implementation process deep enough to collect all that context.

I'm not sure if we want to keep a pub use p2::* reexport myself.

Agree, I don't like use *. Please list all of the identifiers re-exported. (Is that the aspect @alexcrichton objects to?)

This probably wants to (eventually, not necessarily here), come with a rename of the preview0 and preview1 modules to p0 and p1.

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.

We might want to hold off on changing other wasmtime-wasi-* crates for now until APIs are ready for those. Basically pave a path with the core wasmtime-wasi crate but otherwise defer the actual changes to future crates to when we've shaken out all the issues here.

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.

Wasmtime GitHub notifications bot (Jan 23 2025 at 22:18):

alexcrichton commented on PR #10073:

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 pub mod p2 and leaving out the top level re-export.

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.

Last updated: Jan 24 2025 at 00:11 UTC