Skip to content
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

Update to Wasmtime 11 #1678

Closed
wants to merge 2 commits into from
Closed

Conversation

tschneidereit
Copy link
Contributor

Mostly this update was uneventful. However there's a change to the WasiCtxBuilder for Preview2 that made using it much more annoying. @alexcrichton, maybe you have ideas for how to do this in a better way?

Signed-off-by: Till Schneidereit <till@tillschneidereit.net>
@rylev
Copy link
Collaborator

rylev commented Jul 25, 2023

@tschneidereit I think you'll need to update https://github.com/fermyon/spin-componentize and https://github.com/fermyon/wit-bindgen-backport as well. I'll take a closer look later today.

@alexcrichton
Copy link
Contributor

@alexcrichton, maybe you have ideas for how to do this in a better way?

I'll send a PR for updating the upstream APIs to be more amenable to patterns like this

assert_eq!(ctx_addr, std::ptr::addr_of!(*ctx));
dummy
}

/// A builder of a `WasiCtx` for all versions of Wasi
#[allow(clippy::large_enum_variant)]
enum WasiCtxBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conclusion of bytecodealliance/wasmtime#6770 was that this should probably use a WasiCtxBuilder in both arms to use the move-style everywhere instead of on just one half to avoid some of the weirdness here. Otherwise the other possible fix is to update to &mut-style builders but that has other API drawbacks that aren't a clear win at this time.

The major difference from preview1 is that the preview2 WasiCtx doesn't have the mutators that preview1's context has I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented over there, because I'm still not entirely convinced, but we can certainly live with things as they are now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not too sure how using a builder in both arms would actually make things easier, but I'm probably missing something there

Signed-off-by: Till Schneidereit <till@tillschneidereit.net>
@tschneidereit
Copy link
Contributor Author

@tschneidereit I think you'll need to update fermyon/spin-componentize and fermyon/wit-bindgen-backport as well. I'll take a closer look later today.

Thanks Ryan! I had updated the wit-bindgen backport, but missed spin-componentize. That's fermyon/spin-componentize#16 now.

@rylev
Copy link
Collaborator

rylev commented Sep 8, 2023

Closing this as I believe we want to wait until wasmtime 13 to upgrade. @tschneidereit let me know if that's not a mistake and we can reopen this.

@rylev rylev closed this Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants