Stream: git-wasmtime

Topic: wasmtime / issue #5087 wiggle: allow disable tracing in W...

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 21:12):

alexcrichton commented on issue #5087:

Seems reasonable to me! Instead of a crate feature, though, could this be added to CodegenSettings?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 21:36):

github-actions[bot] commented on issue #5087:

Subscribe to Label Action

cc @kubkon

<details>
This issue or pull request has been labeled: "wasi"

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 21:41):

abrown commented on issue #5087:

Ok, pushed a version using CodegenSettings; the parent crates still have Cargo features, though, since this is how the wasmtime feature is enabled.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 21:48):

alexcrichton commented on issue #5087:

I don't remember enough of the rationale here to remember for sure, but can the features be removed entirely? I think it's reasonable to leave tracing off-by-default and turn it back on for wasi-common to preserve its current feature set.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 21:52):

abrown commented on issue #5087:

Not sure I understand how you want it to be turned on-off by the users of from_witx! and wasmtime_integration!...

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 22:00):

abrown commented on issue #5087:

In fact... I think there might even be an additional level of feature propagation needed up to crates/wasi, crates/wasi-common, etc. Those users of wiggle are the places where I want to turn tracing on or off and the only way I see to do that is through Cargo features (just like the Wasmtime integration).

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 23:20):

abrown commented on issue #5087:

Oh never mind me! I think I understand now that you mean use the parse-able configuration of the macro. The latest commit attempts that. Unfortunately, when I add tracing: false to a from_witx! macro, I see build errors like:

wasmtime/crates/wasi-common$ cargo build
   Compiling wasi-common v3.0.0 (/.../wasmtime/crates/wasi-common)
error[E0728]: `await` is only allowed inside `async` functions and blocks
  --> crates/wasi-common/src/snapshots/preview_1.rs:21:1
   |
21 | / wiggle::from_witx!({
22 | |     witx: ["$WASI_ROOT/phases/snapshot/witx/wasi_snapshot_preview1.witx"],
23 | |     errors: { errno => Error },
24 | |     // Note: not every function actually needs to be async, however, nearly all of them do, and
...  |
29 | |     wasmtime: false
30 | | });
   | |  ^
   | |  |
   | |__only allowed inside `async` functions and blocks
   |    this is not `async`
   |
   = note: this error originates in the macro `wiggle::from_witx` (in Nightly builds, run with -Z macro-backtrace for more info)

e
...

So somehow the macro now thinks it should async when it shouldn't?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2022 at 23:20):

abrown edited a comment on issue #5087:

Oh never mind me! I think I understand now that you mean use the parse-able configuration of the macro. The latest commit attempts that. Unfortunately, when I add tracing: false to a from_witx! macro, I see build errors like:

wasmtime/crates/wasi-common$ cargo build
   Compiling wasi-common v3.0.0 (/.../wasmtime/crates/wasi-common)
error[E0728]: `await` is only allowed inside `async` functions and blocks
  --> crates/wasi-common/src/snapshots/preview_1.rs:21:1
   |
21 | / wiggle::from_witx!({
22 | |     witx: ["$WASI_ROOT/phases/snapshot/witx/wasi_snapshot_preview1.witx"],
23 | |     errors: { errno => Error },
24 | |     // Note: not every function actually needs to be async, however, nearly all of them do, and
...  |
29 | |     wasmtime: false
30 | | });
   | |  ^
   | |  |
   | |__only allowed inside `async` functions and blocks
   |    this is not `async`
   |
   = note: this error originates in the macro `wiggle::from_witx` (in Nightly builds, run with -Z macro-backtrace for more info)

e
...

So somehow the macro now thinks it should async when it shouldn't or viceversa?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 21 2022 at 14:20):

alexcrichton commented on issue #5087:

Unfortunately wiggle hasn't really seen much bug fixing and battle-testing over its lifespan, so what you're seeing may be a bug in wiggle. CI isn't failing, though, so is this with an individual build rather than an always-happens things?

As for the wasmtime feature, wiggle-of-old was used for both Lucet and Wasmtime so it had a feature to avoid using Wasmtime, but nowadays I don't think there's necessarily any need for it and we should probably eventually remove that. No need to do that here of course, mostly just that the prior design decisions of wiggle aren't necessarily great guides for what to do with it going forward since times have changed.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2022 at 17:18):

abrown commented on issue #5087:

Closing in favor of the rebased #5146.

Last updated: Oct 23 2024 at 20:03 UTC