Stream: git-wasmtime

Topic: wasmtime / issue #7336 wasmtime 14 argument parsing chang...

Wasmtime GitHub notifications bot (Oct 23 2023 at 18:16):

dgryski added the bug label to Issue #7336.

Wasmtime GitHub notifications bot (Oct 23 2023 at 18:16):

dgryski opened issue #7336:

Go uses wasmtime as the default when running wasip1 binaries: https://github.com/golang/go/blob/master/misc/wasm/go_wasip1_wasm_exec . This script will break the default runner for users once they upgrade wasmtime.

Similarly, TinyGo constructs command lines to run wasm binaries. The changes to the argument parsing breaks that:
https://github.com/tinygo-org/tinygo/issues/3970 .

Wasmtime GitHub notifications bot (Oct 23 2023 at 18:21):

alexcrichton commented on issue #7336:

Thanks for the report! If you're curious more discussion about this can be found on https://github.com/bytecodealliance/wasmtime/pull/6925 and related links.

Is it possible to have these scripts to get updated to require Wasmtime 14? Or is it desired that any Wasmtime version in the environment suffices? In the case that any Wasmtime version would suffice I think that will unfortunately require version detection to select the right CLI.

Wasmtime GitHub notifications bot (Oct 23 2023 at 18:57):

ydnar commented on issue #7336:

Regardless of the merits of the redesigned CLI, I think having a compatibility shim with deprecation warnings might have been warranted, for at least one major version.

--max-wasm-stack: deprecated in wasmtime v14. Did you mean -W max-wasm-stack=N?

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:17):

alexcrichton commented on issue #7336:

I wanted to be sure to copy over some comments from here to this dedicated issue as well as I suspect others will land here in the future too.

First off, if this is catching folks by surprise, I'm sorry about that! The intent was to gradually roll this out the best we could to prevent catching anyone by surprise. The places this was previously discussed prior to the Wasmtime 14 release were:

• original issue
• Zulip discussion
• Discussed in a bi-weekly meeting

That being said if you're surprised by this change, that definitely wasn't sufficient communication! I would at least personally be interested in learning how to better communicate possible breaking changes like this to downstream consumers. Feel free to reach out to me (or other project members) either here, on email, or on Zulip, and we can continue discussion.

Now with that being there's two points that were discussed historically in the above discussions which I think are also important to reiterate here as well:

1. It was discussed and explicitly decided to not have a period of warnings or similar related to this change. This primarily arose from the difficulty of detecting when to warn due to the fact that CLI args not only changed syntax (e.g. --disable-cache to -Ccache=n) but additionally changed how they were parsed. For example previously wasmtime foo.wasm --disable-cache would be parsed as disabling the cache in Wasmtime, but after this change would be parsed as passing the --disable-cache flag to foo.wasm. This means that, for example, if foo.wasm actually took a --disable-cache it's difficult to determine whether the warning should be emitted or not. In the end it was decided that by combining both the syntax and parsing changes in the same release (as opposed to two different releases) that it would assist in alerting users to a change as opposed to silently breaking.

2. This is intended to be The Last major breaking change to the CLI. Emphasis was placed in multiple locations when talking about the change that this was basically the last opportunity to have changes made on this scale. Such a breaking change will not be allowed in the future. Minor changes may still happen but if they do they'll be required to go through a warning/deprecation/etc process.

I bring up these two points less to justify what happened but moreso to help highlight that these were discussed and to additionally summarize (to the best I can) the results of the discussions at the time. If folks are landing here then I'll reiterate again that it primarily means that the communication here wasn't handled as best as it could have. I (and I'm sure others) am interested in improving communication going forward, so please reach out of you have concerns and we can figure out how best to take into account more feedback and communicate more effectively.

Wasmtime GitHub notifications bot (Oct 25 2023 at 21:32):

ydnar commented on issue #7336:

Thanks for the additional context!

That being said if you're surprised by this change, that definitely wasn't sufficient communication! I would at least personally be interested in learning how to better communicate possible breaking changes like this to downstream consumers. Feel free to reach out to me (or other project members) either here, on email, or on Zulip, and we can continue discussion.

In the spirit of constructive feedback, I think a couple things could have been improved with the rollout. The first is comms, and the second is implementation.

Comms

I think it’s important to distinguish between internal project comms and external comms. Zulip, GH issues, PRs, and internal discussion are just that—internal. I don’t think it’s reasonable to expect your users (or their users, who might also be affected) to keep abreast of internal discussions.

• For breaking changes, it’s probably worth a blog post, a Tweet, and/or reaching out proactively to significant users ahead of shipping the change.
• Second, it might have been worth waiting until Monday to ship the change. Shipping a big release on a Friday with little to no public notice gave folks little time during the work week to adapt to the change.
  - Somewhat related: monthly semver-major releases do not signal stability. Recommendation: preserve monthly release cadence, but shift to semver-minor, with additional communication ahead of breaking semver-major releases on a lower frequency.
• Last, improved release documentation:
  - Publish a changelog on the website.
  - Document all of the CLI arguments in the documentation. Currently the only way to get complete CLI documentation is to install wasmtime or examine the source code.
  - Publish a migration guide for every CLI argument.
  - Add a changelog to the Wasmtime CLI page on crates.io.

Implementation

Given this was a breaking change for all users of the Wasmtime CLI, I think having some period of backwards compatibility is warranted.

• For N (<= 2?) major versions, support both the existing CLI arguments in a frozen v13 state, as well as the new v14+ argument structure.
• For each legacy argument, emit a deprecation warning with a translation from old → new so callers can adapt their programs quickly and easily.
• Consider whether a longer duration of backwards compatibility is warranted, e.g. for arguments whose names and function have changed entirely (--mapdir to --dir).

Hopefully some of this is useful, and happy to discuss here or live. Thanks for your consideration!

Wasmtime GitHub notifications bot (Oct 25 2023 at 21:40):

ydnar commented on issue #7336:

A couple other notes:

1. Shipping major changes like this behind a feature flag or as a pre-release version is another way to give notice users affected by the change.
2. A pre-release period or a period of backwards compatibility gives you a chance to refine the new CLI arguments (with further breaking changes) while existing users depend on the legacy CLI.
3. Changes like this inevitably create work for every stakeholder. I have no doubt the new CLI redesign was well-considered and probably the right plan over the long term. But I think the question worth asking is: what is the work we want our users to bear, versus what work can we take on as maintainers to promote usage of the product by making our users’ lives easier?

Wasmtime GitHub notifications bot (Oct 25 2023 at 23:09):

tschneidereit commented on issue #7336:

(Only quickly replying to two points before I sign off for the night)

Somewhat related: monthly semver-major releases do not signal stability. Recommendation: preserve monthly release cadence, but shift to semver-minor, with additional communication ahead of breaking semver-major releases on a lower frequency.

We have very thoroughly thought about this before landing on this approach, see the section on release cadence and the rationale in the RFC for the 1.0 release.

The underlying problem is that we're in a space that is rapidly evolving, and we have a number of different embeddings of wasmtime that we think should always be able to communicate the version number of wasmtime, not the embedding itself, as the most important aspect. This has fundamental tensions in it, and I still believe that quickly iterating major version numbers is the best of a range of not-great options.

I think the question worth asking is: what is the work we want our users to bear, versus what work can we take on as maintainers to promote usage of the product by making our users’ lives easier?

As I mentioned over in https://github.com/golang/go/issues/63718, and as Alex said above, we got this one wrong, and should've done a better job of communicating and rolling out this change. Nevertheless, please rest assured that we're very very aware of the fact that building a platform like wasmtime comes with great responsibility. And I think we're getting this right almost all of the time, in part because we put a lot of thought into how we go about both building and releasing wasmtime.

Wasmtime GitHub notifications bot (Oct 26 2023 at 17:06):

tschneidereit commented on issue #7336:

We just discussed this in the Wasmtime project call, and decided on a remediation plan.

Most importantly, work is underway right now to put out a patch release that restores support for the current CLI (while retaining support for the new one as well).

We also agreed on handling the CLI as a stable interface with strong compatibility guarantees, as well as the shape of a process for ensuring that we can keep to these guarantees, as well as how we'll communicate changes to the CLI. We'll publish an RFC detailing this, with ample opportunity to weigh in.

Note that we deem the changes to the new interface foundationally important for these stability guarantees, so the RFC and an accompanying blog post will also include details on how we'll fully transition, and eventually sunset support for the old interface.

Wasmtime GitHub notifications bot (Oct 26 2023 at 21:11):

alexcrichton commented on issue #7336:

I've opened https://github.com/bytecodealliance/wasmtime/pull/7385 which implements compatibility shims and warnings. Once that lands on the main branch I will backport it to the 14.0.x release, likely as 14.0.3. My guess as to the ETA on that is probably early next week. I've additionally opened a tracking issue which will be pointed to by warnings the CLI will now print. That tracking issue contains further information about translating CLI invocations between the old and the new.

Wasmtime GitHub notifications bot (Oct 30 2023 at 16:15):

alexcrichton commented on issue #7336:

Wasmtime 14.0.3 is now released, and it should address concerns raised here. If there are still issues with it please let us know!

Wasmtime GitHub notifications bot (Oct 31 2023 at 17:45):

alexcrichton closed issue #7336:

Go uses wasmtime as the default when running wasip1 binaries: https://github.com/golang/go/blob/master/misc/wasm/go_wasip1_wasm_exec . This script will break the default runner for users once they upgrade wasmtime.

Similarly, TinyGo constructs command lines to run wasm binaries. The changes to the argument parsing breaks that:
https://github.com/tinygo-org/tinygo/issues/3970 .

Wasmtime GitHub notifications bot (Oct 31 2023 at 17:45):

alexcrichton commented on issue #7336:

I'm going to close this now in favor of https://github.com/bytecodealliance/wasmtime/issues/7384 as I believe the concerns have been addressed. Please follow-up though if that's not the case.

Last updated: Nov 22 2024 at 16:03 UTC