Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Component and WIT import/export syntax #1027

Merged
merged 13 commits into from
May 25, 2023

Conversation

alexcrichton
Copy link
Member

This PR is an implementation of WebAssembly/component-model#193 organized as a series of commits to "sets of crates" within this repository. Each modified crate's tests pass with each successive commit, but the entire repo's tests and fuzzers only pass on the final commit. Like with many WIT changes prior to this one I will preemptively apologize for the size and scale of this commit. Review-wise it's probably best to stick to the core crate of wasmparser and otherwise mostly review tests.

A sumary of changes made in this PR are:

  • The component model binary format is updated, and its major version is bumped. "0xc is dead long live 0xd". This is due to the new binary form of imports and exports and the removal of URLs.
  • URLs are removed entirely from the binary format and WIT. They're now supplanted with "IDs" which look like foo:bar/baz. In the text format of a component this is (interface "foo:bar/baz").
  • WIT syntax has seen a significant number of updates:
    • The concept of a "document" is gone. A WIT package is now always representable with a single file.
    • WIT supports top-level use to name remote interfaces locally or rename local interfaces to a different name in the local scope. This use is purely a package organizational tool.
    • The concept of default is now gone. All interfaces and worlds are referred to by "id" which is inferrable from a package's name.
    • All packages must have a package ... directive indicating the name of the package.
    • The syntax of use has been replaced. Now use foo.{...} refers to the name foo in scope, and use foo/bar.{...} is additionally a means of importing from foreign packages.
    • The syntax of imports/exports in worlds now reflects the component model. Kebab-names are not required for interface imports/exports (and no longer allowed). Anonymous interfaces still use the prior kebab-name syntax.

This list of changes isn't necessarily large but the impact of the change is quite large as can be seen from the scale of this commit. This touched quite a lot of fundamental structures and assumptions about how packages/components look. I've done my best to be judicious about all the little design decisions along the way and I'm hopeful that it's all in a pretty good spot to move forward. In terms of correctness I'm feeling pretty confident about this after updating fuzz-test-case-generation and generating a slew of each-more-interesting-than-the-last bugs.

In terms of logistics of landing this, I would like to work on getting Wasmtime and wit-bindgen updated before landing this here to account for any new features/bugs/etc that need ironing out. My hope is to land everything sort of all-at-once to try to keep everything in sync and avoid this repo getting out of date relative to others.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request May 16, 2023
This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request May 17, 2023
This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request May 17, 2023
This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.
@peterhuene peterhuene self-requested a review May 17, 2023 19:02
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request May 17, 2023
This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.
Copy link
Member

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic and I'm really happy with how this is shaping up.

Just a few minor issues and nits.

crates/wasmparser/src/validator/component.rs Outdated Show resolved Hide resolved
crates/wasmparser/src/validator/component.rs Outdated Show resolved Hide resolved
crates/wast/src/component/binary.rs Outdated Show resolved Hide resolved
crates/wast/src/component/import.rs Outdated Show resolved Hide resolved
crates/wit-component/src/decoding.rs Outdated Show resolved Hide resolved
crates/wit-parser/src/ast/resolve.rs Outdated Show resolved Hide resolved
crates/wit-parser/src/ast/resolve.rs Outdated Show resolved Hide resolved
crates/wit-parser/src/resolve.rs Outdated Show resolved Hide resolved
crates/wit-parser/src/resolve.rs Outdated Show resolved Hide resolved
crates/wit-parser/tests/ui/parse-fail/bad-pkg4.wit.result Outdated Show resolved Hide resolved
@peterhuene
Copy link
Member

peterhuene commented May 18, 2023

With my initial review now completed, I've been thinking more about what it means to be able to specify external dependency versions in WIT directly and how that would integrate with registry-aware tooling.

What's supported in cargo-component today is Cargo.toml can have a table like:

[package.metadata.component.target.dependencies]
# Resolve external dependency with name `a` to version 1.0 of WIT package `foo:bar`
a = "foo:bar@1.0"

and in the target WIT file:

world foo {
   import foo: a.foo
}

and that worked because we assumed a was an external dependency with the current syntax and it was up to the tooling to resolve that symbolic name to a package and version.

Now that we can encode the dependency information directly in the WIT, eliminating the target.dependencies table entirely from Cargo.toml 👍

Regarding versions in external dependency ids, I expect that cargo-component will error on a missing version, as an implicit * feels like the wrong thing to do.

It does make me wonder if we should require a version number for external dependencies when parsing the WIT, though. Given that deps is a workaround in lieu of a functioning registry, it doesn't seem like much of a burden, at first glance, to require that you refer to those deps with a version.

Edit: see next review comment.

@alexcrichton
Copy link
Member Author

Thanks for taking the time to review this! I know it's a lot ...

Copy link
Member

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one unresolved comment remaining, otherwise an enthusiastic 👍 from me (I'll be updating cargo-component with this asap).

@alexcrichton
Copy link
Member Author

Thanks again for the review! When WebAssembly/component-model#193 gets closer to merging I'll start the process for merging this and propagating it outwards.

@alexcrichton
Copy link
Member Author

Ok one feature added with the latest commit is that now, thoeretically, world bindgen is now possible without actually defining your own world. For example wit-bindgen --world wasi:http/proxy should now work. This should help use of worlds define in foreign packages.

@Mossaka Mossaka mentioned this pull request May 23, 2023
* Remove the old URL fields.
* Change the binary format, updating the major version, to have a
  discriminator byte.
* Implement validation of ID names including parsing and uniqueness.
* Update existing tests for new changes.
* Add text-parsing support
* Add text-printing support
* Add encoding support
This commit implements the changes outlined in
WebAssembly/component-model#193 for the `wit-parser` crate. Namely this
updates all parsing, lexing, and resolution of a WIT package. The
largest change is that the concept of a "document" has been removed.
Additionally most tests needed an update to have a `package foo` header.

Intra-package resolution is also a bit trickier now and required a
restructuring of the AST resolution pass, but nothing too too radical
for what it's doing.
This is a very large commit which gets the wit-component crate's tests
working again. The main changes this accounts for are:

* URLs are removed.
* Documents are removed and instead interfaces/worlds are directly in a
  package.
* The encoding structure of a package was slightly updated to reflect this.
* A few minor bugs were fixed now that the ID format is more expressive
  than the prior format.
This is mostly dealing with the fallout of removing URLs.
This removed a few now-obsolete options to `wasm-tools component wit`
(yay!) as they're contextually no longer needed given the new structure
of WIT.
These may get split again in the future, but for now they're the same.
Add support for syntax to select any world within the `Resolve`,
avoiding the need for the `WorldId` to be defined within the package
specified.
Add support for full semver versions on interface strings
@alexcrichton
Copy link
Member Author

Ok given what appears to be high-level consensus on WebAssembly/component-model#198 I'm going to merge this and get the ball rolling on releaseing this. There are still some changes being discussed but they're relatively minor and should be easy to roll out as incremental changes ideally.

@alexcrichton alexcrichton merged commit a636906 into bytecodealliance:main May 25, 2023
@alexcrichton alexcrichton deleted the wit-changes branch May 25, 2023 19:01
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request May 25, 2023
This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.
github-merge-queue bot pushed a commit to bytecodealliance/wasmtime that referenced this pull request May 26, 2023
* Update Wasmtime for upcoming WIT changes

This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.

* Update wasmtime-wasi crate with new WIT

* Add wit-bindgen override for the updated version

* Officially disable wasi-http tests/building

* Move test-reactor WIT into the main WIT files

Don't store duplicates with the rest of the WASI WIT files we have.

* Remove adapter's copy of WIT files

* Disable default features for wit-bindgen

* Plumb disabling wasi-http tests a bit more

* Fix reactor tests and adapter build

* Remove no-longer-needed feature

* Update adapter verification script

* Back out some wasi-http hacks

* Update vet and some dependency sources

* Move where wit-bindgen comes from

Make it a more "official" location which is also less likely to be
accidentally deleted in the future.
github-merge-queue bot pushed a commit to bytecodealliance/wasmtime that referenced this pull request May 26, 2023
* Update Wasmtime for upcoming WIT changes

This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.

* Update wasmtime-wasi crate with new WIT

* Add wit-bindgen override for the updated version

* Officially disable wasi-http tests/building

* Move test-reactor WIT into the main WIT files

Don't store duplicates with the rest of the WASI WIT files we have.

* Remove adapter's copy of WIT files

* Disable default features for wit-bindgen

* Plumb disabling wasi-http tests a bit more

* Fix reactor tests and adapter build

* Remove no-longer-needed feature

* Update adapter verification script

* Back out some wasi-http hacks

* Update vet and some dependency sources

* Move where wit-bindgen comes from

Make it a more "official" location which is also less likely to be
accidentally deleted in the future.
github-merge-queue bot pushed a commit to bytecodealliance/wasmtime that referenced this pull request May 26, 2023
* Update Wasmtime for upcoming WIT changes

This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.

* Update wasmtime-wasi crate with new WIT

* Add wit-bindgen override for the updated version

* Officially disable wasi-http tests/building

* Move test-reactor WIT into the main WIT files

Don't store duplicates with the rest of the WASI WIT files we have.

* Remove adapter's copy of WIT files

* Disable default features for wit-bindgen

* Plumb disabling wasi-http tests a bit more

* Fix reactor tests and adapter build

* Remove no-longer-needed feature

* Update adapter verification script

* Back out some wasi-http hacks

* Update vet and some dependency sources

* Move where wit-bindgen comes from

Make it a more "official" location which is also less likely to be
accidentally deleted in the future.

* Don't document wasi-http-tests
rvolosatovs pushed a commit to rvolosatovs/wasi-io that referenced this pull request Aug 24, 2023
)

* Update Wasmtime for upcoming WIT changes

This PR integrates bytecodealliance/wasm-tools#1027 into Wasmtime. The
main changes here are:

* WIT syntax is updated with WebAssembly/component-model#193
* Generated bindings in the `bindgen!` macro have been updated to
  reflect the new structure of WIT.
* The accepted component model binary format has been updated to account
  for changes.

This PR disables wasi-http tests and the on-by-default feature because
the WIT syntax has been updated but the submodule containing the WITs
has not been updated yet so there's no way to get that building
temporarily. Once that's updated then this can be reenabled.

* Update wasmtime-wasi crate with new WIT

* Add wit-bindgen override for the updated version

* Officially disable wasi-http tests/building

* Move test-reactor WIT into the main WIT files

Don't store duplicates with the rest of the WASI WIT files we have.

* Remove adapter's copy of WIT files

* Disable default features for wit-bindgen

* Plumb disabling wasi-http tests a bit more

* Fix reactor tests and adapter build

* Remove no-longer-needed feature

* Update adapter verification script

* Back out some wasi-http hacks

* Update vet and some dependency sources

* Move where wit-bindgen comes from

Make it a more "official" location which is also less likely to be
accidentally deleted in the future.

* Don't document wasi-http-tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants