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

Cherry-pick #3063 to the stable branch #3079

Closed
wants to merge 1 commit into from
Closed

Cherry-pick #3063 to the stable branch #3079

wants to merge 1 commit into from

Conversation

ulan
Copy link
Contributor

@ulan ulan commented Jul 13, 2021

  • Restore POSIX signal handling on MacOS behind a feature flag

As described in Issue #3052, the switch to Mach Exception handling
removed unix::StoreExt from the public API of crate on MacOS.
That is a breaking change and makes it difficult for some
application to upgrade to the current stable Wasmtime.

As a workaround this PR introduces a feature flag called
posix-signals-on-macos that restores the old behaviour on MacOS.
The flag is disabled by default.

  • Fix test guard

  • Fix formatting in the test

* Restore POSIX signal handling on MacOS behind a feature flag

As described in Issue #3052, the switch to Mach Exception handling
removed `unix::StoreExt` from the public API of crate on MacOS.
That is a breaking change and makes it difficult for some
application to upgrade to the current stable Wasmtime.

As a workaround this PR introduces a feature flag called
`posix-signals-on-macos` that restores the old behaviour on MacOS.
The flag is disabled by default.

* Fix test guard

* Fix formatting in the test
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Jul 13, 2021
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@alexcrichton
Copy link
Member

Thanks for this! AFAIK though we don't really have any guidelines about what's backported and what historical versions are maintained or not. So far we have only backported and done a point release once, and that was for a security fix.

Is upgrading to the current wasmtime main branch possible for you? It should be easier to make a new release for us than to make another older release.

@ulan
Copy link
Contributor Author

ulan commented Jul 13, 2021

Right, I was also wondering if there is a process for backporting.

Is upgrading to the current wasmtime main branch possible for you?

Yep, we can update to it although it will take some time to adjust our code to the API redesign that happened in 0.28.0.

My only concern is the stability of that version. My understanding was that 0.26.1 is the current stable version suitable for production whereas the newer versions may have less coverage and may be unstable. Is this not correct?

@alexcrichton
Copy link
Member

If anything this is the process of backporting, we just haven't written it down anywhere or really exercised it that much. (we also don't have any process yet to determine what changes should be backported, a security fix was just sort of "obviously correct" to backport)

In terms of stability and production-ready-ness all recent Wasmtime versions should suffice. We mostly make new releases for releasing new features. Currently we don't have a notion of a "stable release" or similar. We hope to signal this soon with a 1.0 release that additionally signals a level of API stability, but in the meantime the quality of the implementation of 0.26 should be the same as 0.28 and beyond.

@ulan
Copy link
Contributor Author

ulan commented Jul 14, 2021

Thank you for the clarification! That sounds good. I assumed too much based on the branch name stable-v0.26.

I'll start working on updating our codebase to the main branch of wasmtime.

@ulan ulan closed this Jul 14, 2021
@alexcrichton
Copy link
Member

FWIW if this is urgent and updating to 0.28 in a reasonable timeframe isn't feasible I don't think we have a policy against backports like this, we'd just probably need to discuss it with a few more folks other than just me

@ulan
Copy link
Contributor Author

ulan commented Jul 15, 2021

@alexcrichton thanks, I'll keep that in mind and may come back to it if other options do not work out.

ulan added a commit to dfinity-lab/wasmtime that referenced this pull request Jul 15, 2021
This patch is taken from
bytecodealliance#3079
which in turn a cherry-pick of bytecodealliance#3063 adjusted for v0.26:
bytecodealliance#3063

- Restore POSIX signal handling on MacOS behind a feature flag

As described in Issue bytecodealliance#3052, the switch to Mach Exception handling
removed unix::StoreExt from the public API of crate on MacOS.
That is a breaking change and makes it difficult for some
application to upgrade to the current stable Wasmtime.

As a workaround this PR introduces a feature flag called
posix-signals-on-macos that restores the old behaviour on MacOS.
The flag is disabled by default.

- Fix test guard

- Fix formatting in the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants