build: manage `wit/deps` via `wit-deps`

[rvolosatovs]

Use https://github.com/bytecodealliance/wit-deps to manage WIT dependencies. Integrating wit-deps in the root of the repository will be done as a follow-up, it's not straight-forward as many dependencies are out-of-date and the implementation is not compatible with any upstream wasi-clocks version: upstream takes the clock ID as the first (out of 3) parameter, whereas the current implementation assumes 2 parameters.

Closes #134 Closes #133

[sunfishcode]

Fwiw, I have a PR syncing the upstream wasi-clocks with the change in preview2-prototyping: https://github.com/WebAssembly/wasi-clocks/pull/42. I'll land it in a few days if there are no comments.

[rvolosatovs]

Thanks, this looks like a good start.

One thing I am confused about is why depit::lock_sync! is a macro. I wasn't able to figure that out from reading the sources.

Instead of using build.rs, could we instead use a CI step and/or a #[test] to enforce that the deps are locked?

The reason for it to be a macro is to integrate with cargo and eliminate a need for installing an additional executable tool. All of the functionality is available either via the binary or the library, however, so there are multiple ways how it could be integrated. My initial hoping was to get to a point where /wit/deps would be gitignored and wit/deps.lock and wit/deps.toml would be enough for development. That would make build, in general case, dependent on networking though, which is less than ideal and may not work well with external tooling. More importantly, it effectively breaks the use case where the URLs are "dynamic", e.g. pointing to a branch like was done in https://github.com/WebAssembly/wasi-http/pull/23 See also example https://github.com/bytecodealliance/wit-deps/blob/1899423442c3e6f78de3227f2a168db2920e3e7b/examples/github/wit/deps.toml#L1-L15

In any case, this can be changed to a binary invocation in CI or a Rust test case, for example a test case could call https://docs.rs/wit-deps/latest/wit_deps/fn.lock.html and fail if Some(_) is returned. I still feel like build.rs integration is nicer, since it's impossible to "forget" to e.g. lock the deps after updating the manifest.

[rvolosatovs]

Thanks, this looks like a good start.

One thing I am confused about is why depit::lock_sync! is a macro. I wasn't able to figure that out from reading the sources.

Instead of using build.rs, could we instead use a CI step and/or a #[test] to enforce that the deps are locked?

I've added a CI check, since #[test] approach is mutually exclusive with https://github.com/bytecodealliance/preview2-prototyping/pull/136#discussion_r1162097885 (if there's no lock in the repository, there's nothing to validate the dep state against, other than using git diff, which IMO is something that belongs in a CI job step and not a general-purpose test)

Personally, I am not a fan of this approach, since developers are likely to forget to

$ cd wasi
$ wit-deps lock

on changes to /wit, not to mention a dependency on yet another binary, but whatever works

[rvolosatovs]

Note, that installing wit-deps-cli in CI takes a while. An alternative approach could be pulling in a binary release from e.g. https://github.com/bytecodealliance/wit-deps/releases/tag/v0.3.0, should I try that?

[pchickey]

Yes, I think installing from the binary release is appropriate.

Is it not possible for wit-deps to perform a diff of what it would have generated vs what is on the filesystem, without shelling out to git? Like how cargo fmt --check or similar tools work.

Also, just as a heads up, #140 re-organizes some wit in this repo.

[rvolosatovs]

Yes, I think installing from the binary release is appropriate.

Done, PTAL

Is it not possible for wit-deps to perform a diff of what it would have generated vs what is on the filesystem, without shelling out to git? Like how cargo fmt --check or similar tools work.

This is not supported at this moment, but would be a nice feature to add once https://github.com/bytecodealliance/wit-deps/issues/25 and https://github.com/bytecodealliance/wit-deps/issues/24 are done, since then it would be possible to just diff the Wasm

FTR, here's an example failure https://github.com/rvolosatovs/preview2-prototyping/actions/runs/4788291197/jobs/8514694951

[pchickey]

Thanks. I need a little bit more help understanding how this tool is integrated.

It looks like this is just adding a copy of all the wit deps to inside the wasi crate's tree. From the wasi/wit/deps.toml it looks like the canonical definition of them, called "preview", is up at the root of the crate under wit/.

When the tool is run, it generates a deps.lock file, but that file is gitignored. What is the purpose of that file if we aren't checking it in? We check in our Cargo.lock files to make sure each build of our Rust crates is reproducible, and git diffs to the Cargo.lock describe whatever updates happened to our deps. Should we do the same with this lockfile? or why not?

How will this tool support moving the canonical definitions out of wit/ and into the standards repositories? I thought that was part of the goal here, but it looks like that git history is force-pushed away. Can we do that now?

Also, we use the same wits inside host/ via path = ../wit in the proc macro, and use a symlink to copy some of the deps inside test-programs/reactor-tests/wit/ as well. Should we be managing copies of all of those with this tool as well?

[rvolosatovs]

It looks like this is just adding a copy of all the wit deps to inside the wasi crate's tree. From the wasi/wit/deps.toml it looks like the canonical definition of them, called "preview", is up at the root of the crate under wit/.

Yes, and this is what closes the https://github.com/bytecodealliance/preview2-prototyping/issues/133 issue I filed earlier. Relative path references in wit-bindgen invocations break dependencies utilizing cargo vendor.

When the tool is run, it generates a deps.lock file, but that file is gitignored. What is the purpose of that file if we aren't checking it in? We check in our Cargo.lock files to make sure each build of our Rust crates is reproducible, and git diffs to the Cargo.lock describe whatever updates happened to our deps. Should we do the same with this lockfile? or why not?

See https://github.com/bytecodealliance/preview2-prototyping/pull/136#discussion_r1162097885

Since all dependencies of wasi/wit are present within the repository, this is all fully reproducible. deps.lock is what wit-deps uses to determine if the contents of wit/deps are in sync. In cases where the deps.toml contains network URLs, especially references to branches, then this file would need to be checked in to ensure reproducibility. See this example https://github.com/bytecodealliance/wit-deps/tree/main/examples/github/wit

How will this tool support moving the canonical definitions out of wit/ and into the standards repositories? I thought that was part of the goal here, but it looks like that git history is force-pushed away. Can we do that now?

Not exactly sure what you mean about force-pushing the history away. It is indeed the goal, however, for this tool to evolve into a general-purpose wit tooling used by WIT interface developers (e.g. developers of WASI standards and/or Wasm host runtime developers). This tool would be language-agnostic and would mainly be used to validate the WIT definitions, but potentially could even e.g. "translate" the WIT definitions to arbitrary protocols, for example, it could produce a protocol buffer definition including a gRPC service corresponding to the WIT. cc @lukewagner

All of this is way further down the line, for now https://github.com/bytecodealliance/wit-deps/issues/24 is the next target, but it can only be done once wit-bindgen "speaks" Wasm for dependencies, which I believe is not the case today, as it only operated on raw WIT files last time I checked. So once WIT dependencies can be consumed as Wasm binaries, wit-deps can consume and produce Wasm. Next step would be fetching that Wasm from a registry.

To get a bit deeper into the details here, currently wit-deps relies on gzipped tarballs as the WIT package distribution format, that will be replaced by Wasm, at which point wit-deps should be fully capable to consume that Wasm from an arbitrary URL and as long as that Wasm can be just fetched directly via usual HTTP GET from WARG (is that the plan @Kylebrown9 ?), the Wasm registry should be immediately supported.

Also, we use the same wits inside host/ via path = ../wit in the proc macro, and use a symlink to copy some of the deps inside test-programs/reactor-tests/wit/ as well. Should we be managing copies of all of those with this tool as well?

I will update those as well

[pchickey]

Sorry, it took me a while to make time to come back to this. Understood, thanks for explaining better.

Lets land this, and then do a follow up where we convert the whole repository to use this tool for managing all wit definitions, and pull definitions from the standards repo tarballs whenever possible.