Stream: git-wasmtime

Topic: wasmtime / PR #8928 Use wit-deps to manage all wit depend...

Wasmtime GitHub notifications bot (Jul 10 2024 at 04:33):

iawia002 opened PR #8928 from iawia002:wit-deps to bytecodealliance:main:

This patch uses the wit-deps package to manage all wit dependencies (except wasi-nn for now):
https://github.com/bytecodealliance/wasmtime/blob/7caca1624a3c68dd4135b3941e7c87d94eb4c7db/ci/vendor-wit.sh#L40-L43

I believe this is a more understandable and automated approach compared to the bash script, as it will automatically synchronize these wit dependencies during the build process.

Wasmtime GitHub notifications bot (Jul 10 2024 at 04:33):

iawia002 requested wasmtime-default-reviewers for a review on PR #8928.

Wasmtime GitHub notifications bot (Jul 10 2024 at 04:33):

iawia002 requested fitzgen for a review on PR #8928.

Wasmtime GitHub notifications bot (Jul 10 2024 at 04:33):

iawia002 requested wasmtime-core-reviewers for a review on PR #8928.

Wasmtime GitHub notifications bot (Jul 10 2024 at 04:42):

iawia002 updated PR #8928.

Wasmtime GitHub notifications bot (Jul 10 2024 at 04:51):

iawia002 updated PR #8928.

Wasmtime GitHub notifications bot (Jul 10 2024 at 15:10):

alexcrichton commented on PR #8928:

Thanks for the PR! While I don't disagree that this is better than bash in terms of readability this has a downside of depending on wit-deps at build time which is a bit of a hefty dependency to add to the build. For now I think it would be best to keep the same style of build where the wit files are checked in CI to be correct but otherwise there's no extra build dependencies for downstream users.

Wasmtime GitHub notifications bot (Jul 10 2024 at 15:18):

juntyr commented on PR #8928:

Thanks for the PR! While I don't disagree that this is better than bash in terms of readability this has a downside of depending on wit-deps at build time which is a bit of a hefty dependency to add to the build. For now I think it would be best to keep the same style of build where the wit files are checked in CI to be correct but otherwise there's no extra build dependencies for downstream users.

What I’m doing in a project of mine at the moment is to use wit-deps in a test to check that everything is up to date (but to also keep the wit field checked in … well technically my solution uses git submodules to avoid checking the files in directly, but that doesn’t matter for the test) - this makes the dependency a dev-dependency only so that it should not burden any downstream users

Wasmtime GitHub notifications bot (Jul 10 2024 at 17:28):

fitzgen commented on PR #8928:

Seconding what Alex said: we should run this in CI, not on every single build of the wasmtime crate.

Wasmtime GitHub notifications bot (Jul 11 2024 at 02:05):

iawia002 commented on PR #8928:

Thank all for your feedback. I understand your concerns and have made some improvements. I will no longer use build.rs to update dependencies. The process will remain as it is now, but developers will need to install the wit-deps CLI tool to update wit files locally.

Going back to the original reason why I wanted to make this change:

1. I noticed that the current bash script keeps the dependencies of the wasi and wasi-http crates in sync, but in fact, the wasi crate does not need the http world: https://github.com/bytecodealliance/wasmtime/blob/0e5048e20a24f6da19b3794eeb94111f052dc414/crates/wasi/src/bindings.rs#L148-L150
   In the bash script, achieving this differentiation requires more code. If this patch is accepted, I will submit another PR to do some cleanup.

2. When I tried to implement wasi-runtime-config, since it is still a draft version, its tag is different from cli and http etc. To manage its wit dependencies, I had to add some code to the bash script to download files individually. If there are more different versions of wit dependencies in the future, the bash script will become very complex.

PTAL if this change is reasonable.

Wasmtime GitHub notifications bot (Jul 11 2024 at 02:06):

iawia002 updated PR #8928.

Wasmtime GitHub notifications bot (Jul 11 2024 at 06:57):

iawia002 updated PR #8928.

Wasmtime GitHub notifications bot (Jul 11 2024 at 14:47):

alexcrichton commented on PR #8928:

Functionally this is much better, thanks! Personally though I'd at least personally prefer to stick to the bash script for now. I'm worried to pick up another tool as part of our build which much of us don't understand and it's not buying us much over a bash script. I don't disagree that the bash script is going to get a bit more complicated over time but I think the complexity is relatively manageable. For example it's not really an issue that HTTP WIT files are part of the wasmtime-wasi crate. Adding support for downloading another version of things to a different location shouldn't greatly increase the complexity of the script either.

Some other reasons I'd prefer to stick to a script for now:

• Installation of wit-deps is yet-another-tool to manage in CI. For example cargo install here doesn't use --locked to prevent errors over time with mistakes in publishing to crates.io. It also doesn't specify a version to install so we'd be susceptible to breakage in terms of if wit-depschanges. This is all fixable but is adding more overhead relative to a script.
• We can't exclusively use wit-deps because wasi-nn doesn't support it. That means we still need a script to do things. Additionally the script is required to matter what to pass the right arguments to wit-deps.

Overall I think it may just not be the right time to switch away from bash scripts. If the shell script could go away entirely I think that's the right time to migrate to a different tool, but I don't think that we're there yet.

Wasmtime GitHub notifications bot (Jul 17 2024 at 15:17):

alexcrichton closed without merge PR #8928.

Last updated: Dec 06 2025 at 06:05 UTC