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>
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 nestedstruct
s).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.</details>
jameysharp commented on issue #6119:
@saulecabrera, you have the necessary GitHub permissions to merge this, right?
saulecabrera commented on issue #6119:
Yes @jameysharp, I wasn't sure if you wanted to merge it yourself. But just did, thanks!
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 ofcfg(compiler)
isn't enough: Many of the places in thewasmtime
crate that use the#[cfg(compiler)]
attribute also have a#[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))]
attribute which needs to be updated now.
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 thecomponent_model
feature. We've tried updating thepackage.metadata.docs.rs
features in thewasmtime/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.
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
andwinch-environ
was not correct. I've addressed that in https://github.com/bytecodealliance/wasmtime/pull/6119/commits/7d83eec78e18bc73435b61ebbd519bf8728a2f8e
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.
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.
KevinRizzoTO commented on issue #6119:
@saulecabrera thank you for getting this over the line!
Last updated: Dec 23 2024 at 12:05 UTC