Stream: git-wasmtime

Topic: wasmtime / issue #6119 winch: Initial integration with wa...

Wasmtime GitHub notifications bot (Mar 29 2023 at 19:45):

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

Subscribe to Label Action

cc @peterhuene, @saulecabrera

<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:config", "winch"

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

• peterhuene: wasmtime:api
• saulecabrera: winch

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Mar 29 2023 at 19:45):

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

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

• [ ] If you added a new Config method, you wrote extensive documentation for
  it.

  <details>

  Our documentation should be of the following form:

  ```text
  Short, simple summary sentence.

  More details. These details can be multiple paragraphs. There should be
  information about not just the method, but its parameters and results as
  well.

  Is this method fallible? If so, when can it return an error?

  Can this method panic? If so, when does it panic?

  Example

  Optional example here.
  ```

  </details>

• [ ] If you added a new Config method, or modified an existing one, you
  ensured that this configuration is exercised by the fuzz targets.

  <details>

  For example, if you expose a new strategy for allocating the next instance
  slot inside the pooling allocator, you should ensure that at least one of our
  fuzz targets exercises that new strategy.

  Often, all that is required of you is to ensure that there is a knob for this
  configuration option in [wasmtime_fuzzing::Config][fuzzing-config] (or one
  of its nested structs).

  Rarely, this may require authoring a new fuzz target to specifically test this
  configuration. See [our docs on fuzzing][fuzzing-docs] for more details.

  </details>

• [ ] If you are enabling a configuration option by default, make sure that it
  has been fuzzed for at least two weeks before turning it on by default.

[fuzzing-config]: https://github.com/bytecodealliance/wasmtime/blob/ca0e8d0a1d8cefc0496dba2f77a670571d8fdcab/crates/fuzzing/src/generators.rs#L182-L194
[fuzzing-docs]: https://docs.wasmtime.dev/contributing-fuzzing.html

---

<details>

To modify this label's message, edit the <code>.github/label-messager/wasmtime-config.md</code> file.

To add new label messages or remove existing label messages, edit the
<code>.github/label-messager.json</code> configuration file.

Learn more.

</details>

Wasmtime GitHub notifications bot (Mar 30 2023 at 16:10):

jameysharp commented on issue #6119:

@saulecabrera, you have the necessary GitHub permissions to merge this, right?

Wasmtime GitHub notifications bot (Mar 30 2023 at 16:23):

saulecabrera commented on issue #6119:

Yes @jameysharp, I wasn't sure if you wanted to merge it yourself. But just did, thanks!

Wasmtime GitHub notifications bot (Mar 30 2023 at 17:21):

jameysharp commented on issue #6119:

CI failed during the doc build job:

error[E0046]: not all trait items implemented, missing: `component_compiler`
  --> crates/winch/src/compiler.rs:43:1

Also I've learned that changing the build.rs definition of cfg(compiler) isn't enough: Many of the places in the wasmtime crate that use the #[cfg(compiler)] attribute also have a #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] attribute which needs to be updated now.

Wasmtime GitHub notifications bot (Mar 30 2023 at 18:20):

KevinRizzoTO commented on issue #6119:

@jameysharp So me and @saulecabrera tried to fix this but we are not quite sure where the doc features get added :thinking: On the winch compiler we copied the Cranelift approach to gate the component_compiler method with the component_model feature. We've tried updating the package.metadata.docs.rs features in the wasmtime/Cargo.toml file, but this doesn't seem to fix anything locally.

The one thing that worked was updating the command that is run by Github actions to include the winch feature. But I'm not sure if this is the approach we want to take here.

Also, good call about the doc attribute, I can add those for winch as well.

Wasmtime GitHub notifications bot (Apr 04 2023 at 18:55):

saulecabrera commented on issue #6119:

The docs fix worked -- there was another failure regarding the order of crates in publish.rs: the relationship between wasmtime-winch and winch-environ was not correct. I've addressed that in https://github.com/bytecodealliance/wasmtime/pull/6119/commits/7d83eec78e18bc73435b61ebbd519bf8728a2f8e

Wasmtime GitHub notifications bot (Apr 05 2023 at 00:12):

saulecabrera commented on issue #6119:

Just one minor (hopefully final) change in https://github.com/bytecodealliance/wasmtime/pull/6119/commits/f307452a581cc02896f2e492621bf90457dc5f2c to only run tests winch tests when in unix and x86_64, since there's no support for the fastcall calling convention yet.

Wasmtime GitHub notifications bot (Apr 05 2023 at 00:12):

saulecabrera edited a comment on issue #6119:

Just one minor (hopefully final) change in https://github.com/bytecodealliance/wasmtime/pull/6119/commits/f307452a581cc02896f2e492621bf90457dc5f2c to only run winch tests when in unix and x86_64, since there's no support for the fastcall calling convention yet.

Wasmtime GitHub notifications bot (Apr 05 2023 at 16:09):

KevinRizzoTO commented on issue #6119:

@saulecabrera thank you for getting this over the line!

Last updated: Oct 23 2024 at 20:03 UTC