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

"hello world" example with the wasi crate doesn't work #236

Closed
yoshuawuyts opened this issue Feb 26, 2024 · 5 comments · Fixed by #240
Closed

"hello world" example with the wasi crate doesn't work #236

yoshuawuyts opened this issue Feb 26, 2024 · 5 comments · Fixed by #240
Labels
bug Something isn't working

Comments

@yoshuawuyts
Copy link
Member

Repro

If we run the following commands to build the wasi crate:

cargo install cargo-component
git clone https://github.com/bytecodealliance/wasi
cd wasi
cargo component build --example hello-world

This results in the following output:

yosh@MacBook-Pro wasi % cargo component run --example hello-world
   Compiling wit-bindgen v0.19.1
   Compiling bitflags v2.4.2
   Compiling wasi v0.12.1+wasi-0.2.0 (/Users/yosh/Code/wasi)
    Finished dev [unoptimized + debuginfo] target(s) in 1.76s
Error: failed to run main module `/Users/yosh/Code/wasi/target/wasm32-wasi/debug/examples/hello-world.wasm`

Caused by:
    0: failed to instantiate "/Users/yosh/Code/wasi/target/wasm32-wasi/debug/examples/hello-world.wasm"
    1: unknown import: `wasi:io/[email protected]::[resource-drop]output-stream` has not been defined

Manual build instructions

The wasi crate suggests the following instructions to get it to build correctly:

# build the example for WASI 0.1
cargo build --target wasm32-wasi --example hello-world

# fetch the WASI 0.1 -> WASI 0.2 adapter
curl -LO https://github.com/bytecodealliance/wasmtime/releases/download/v17.0.0/wasi_snapshot_preview1.command.wasm

# convert the example into a WASI 0.2 component
cargo install wasm-tools
wasm-tools component new target/wasm32-wasi/example/hello-world.wasm \
    --adapt ./wasi_snapshot_preview1.command.wasm \
    -o hello-world.wasm

# run the component
wasmtime run hello-world.wasm

This results in the following output:

Hello, world!

Expected behavior

From the outset I was expecting this to just work? Is it that cargo run --example isn't implemented yet? It does seem to build the wasm32-wasi example correctly. Or is something else perhaps not working when it should?

@peterhuene
Copy link
Member

peterhuene commented Feb 26, 2024

To prevent cargo-component from componentizing any ol' wasm32 target it encounters, it expects Cargo.toml to have the [package.metadata.component] section to treat it as a component rather than a module. You'll notice it is missing the Creating component message in the cargo component build output.

If you add [package.metadata.component] to Cargo.toml, you'll get errors when parsing the WIT, as cargo-component does not do WIT dependency discovery via a deps directory, instead expects entries in [package.metadata.component.target.dependencies].

So Cargo.toml would need:

[package.metadata.component.target.dependencies]
"wasi:http" = { path = "wit/deps/http" }
"wasi:random" = { path = "wit/deps/random" }
"wasi:cli" = { path = "wit/deps/cli" }
"wasi:io" = { path = "wit/deps/io" }
"wasi:clocks" = { path = "wit/deps/clocks" }
"wasi:filesystem" = { path = "wit/deps/filesystem" }
"wasi:sockets" = { path = "wit/deps/sockets" }

However, there appears to be breaking changes in the latest wit-bindgen with regards to runtime types, so you'd need to drop the version down to 0.18.0 to be compatible with the latest cargo-component (I should do a cargo-component release to bump wit-bindgen to latest).

With the above in place, you should now see the following for cargo component run --example hello-world:

Error: failed to run main module `/Users/peterhuene/tmp/wasi/target/wasm32-wasi/debug/examples/hello-world.wasm`

Caused by:
    exported instance `wasi:cli/[email protected]` not present

This, finally, is a bug in cargo-component, where it is not treating an example as a bin, and thus is using the built-in reactor adapter rather than the command adapter.

It's possible to workaround that with:

[package.metadata.component]
adapter = "wasi_snapshot_preview1.command.wasm"

But it should work out of the box.

@peterhuene peterhuene added the bug Something isn't working label Feb 26, 2024
@alexcrichton
Copy link
Member

it expects Cargo.toml to have the [package.metadata.component] section to treat it as a component rather than a module

In this case with no WIT internally as it's all coming from the wasi crate itself could this logic change to: if no [package.metadata.component] is specified then parse the output wasm module and if any component-type* custom section is found then do the componentization process as-if an empty section was specified?

However, there appears to be breaking changes in the latest wit-bindgen with regards to runtime types

Oh dear, do you mean breakage from 0.18-to-0.19 or something that needs a yank/republish on crates.io?

(also I'm happy to cc you on more changes to wit-bindgen if you'd like for possible breakage with cargo-component)

@peterhuene
Copy link
Member

peterhuene commented Feb 26, 2024

In this case with no WIT internally as it's all coming from the wasi crate itself could this logic change to: if no [package.metadata.component] is specified then parse the output wasm module and if any component-type* custom section is found then do the componentization process as-if an empty section was specified?

I agree that componetizing a module based on the presence of the component-type custom section rather than the presence of [package.metadata.component] makes more sense; I can make that change.

Right now, if there are no settings under [package.metadata.component.target], cargo-component will probe for a wit directory to generate bindings automatically; hence why it's generating bindings for the bindings world in wit/wasi-crate.wit here.

I'm not entirely sure I would want to change that behavior, as most of the time the presence of the wit directory indicates a local target for the component being built.

Oh dear, do you mean breakage from 0.18-to-0.19 or something that needs a yank/republish on crates.io?

A breakage from 0.18 to 0.19; it's just cargo-component that's out of date, as it uses 0.18.0 to generate bindings ahead-of-time, but Cargo.toml might reference 0.19.1 solely for the runtime types defined in wit-bindgen.

An example breakage from 0.18.0 to 0.19.1:

error[E0599]: no function or associated item named `into_handle` found for struct `Resource<_>` in the current scope
     --> src/bindings.rs:14337:40
      |
14337 |             wit_bindgen::rt::Resource::into_handle(self.handle)
      |                                        ^^^^^^^^^^^
      |                                        |
      |                                        function or associated item not found in `Resource<_>`
      |                                        help: there is an associated function with a similar name: `handle`

Ideally it would be nice if we had a mode where we could generate the runtime types (and cabi_* exports) as part of bindings generation so that the user's Cargo.toml doesn't depend on wit-bindgen at all.

@alexcrichton
Copy link
Member

Ah I see good point!

One suggestion would be to update cargo component to generate wit-bindgen = "0.18.0" in the manifest, or whatever matches the built-in generator of cargo component. That being said that won't play well with upgrades over time, for example taking a very new cargo component and building an older project.

One fix for that is to then do what you mention, generate Reource<T> and bits into the file itself. That has problems though we just discovered at least with cabi_realloc where we want multiple wit-bindgen-using crates to link together and they can't all define cabi_realloc so we need to use weak symbols, and then that's not stable in Rust.

I think the best solution might be to define "perma stable" APIs, like how the runtime links cabi_realloc and the run_ctors_once thing. Less stable things, like Resource<T> should go into the generated bindings.

So how about this:

  • As a near-term fix, cargo component updates to 0.19.x for wit-bindgen generation
  • As a long-term fix, wit-bindgen generates the Resource<T> wrapper and any "less stable" bits into the generated bindings
  • As a long-term fix, wit-bindgen considers its API surface are "perma stable" as future versions of wit-bindgen runtime should work with old versions of wit-bindgen generators

@peterhuene
Copy link
Member

Your plan sounds great to me!

peterhuene added a commit to peterhuene/cargo-component that referenced this issue Feb 27, 2024
This commit includes the following improvements to `cargo-component`:

* `cargo-component` now componentizes based off the presence of component type
  information custom sections rather than always requiring a
  `[package.metadata.component]` section be present in `Cargo.toml`.
* Bindings generation will now occur if a `wit` directory is present and
  `[package.metadata.component]` is not present in `Cargo.toml`.
* Fix a panic in `cargo component add` if it encounters expected items that are
  not tables.
* Fix not setting implicit tables for intermediate tables for the `cargo
  component add` command.
* Fix not displaying anything when `--help` is present for commands passed
  directly to `cargo`.
* `cargo-component` now uses the command adapter for binary examples and
  test/bench artifacts.
* `cargo-component` now displays the same "Running" message that `cargo` does
  when using the `run`, `test`, and `bench` commands.
* Some paths displayed in status messages have been prefix-stripped based on
  the CWD.

This commit also contains a bit of refactoring to `run_cargo_command`, as it
was getting long and unwieldy.

Fixes bytecodealliance#236.
peterhuene added a commit to peterhuene/cargo-component that referenced this issue Feb 27, 2024
This commit includes the following improvements to `cargo-component`:

* `cargo-component` now componentizes based off the presence of component type
  information custom sections rather than always requiring a
  `[package.metadata.component]` section be present in `Cargo.toml`.
* Bindings generation will now occur if a `wit` directory is present and
  `[package.metadata.component]` is not present in `Cargo.toml`.
* Fix a panic in `cargo component add` if it encounters expected items that are
  not tables in `Cargo.toml`.
* Fix not setting implicit tables for intermediate tables for the `cargo
  component add` command.
* Fix not displaying anything when `--help` is present for commands passed
  directly to `cargo`.
* `cargo-component` now uses the command adapter for binary examples and
  test/bench artifacts.
* `cargo-component` now displays the same "Running" message that `cargo` does
  when using the `run`, `test`, and `bench` commands.
* Some paths displayed in status messages have been prefix-stripped based on
  the CWD.

This commit also contains a bit of refactoring to `run_cargo_command`, as it
was getting long and unwieldy.

Fixes bytecodealliance#236.
peterhuene added a commit that referenced this issue Feb 28, 2024
* Various improvements to `cargo-component`.

This commit includes the following improvements to `cargo-component`:

* `cargo-component` now componentizes based off the presence of component type
  information custom sections rather than always requiring a
  `[package.metadata.component]` section be present in `Cargo.toml`.
* Bindings generation will now occur if a `wit` directory is present and
  `[package.metadata.component]` is not present in `Cargo.toml`.
* Fix a panic in `cargo component add` if it encounters expected items that are
  not tables in `Cargo.toml`.
* Fix not setting implicit tables for intermediate tables for the `cargo
  component add` command.
* Fix not displaying anything when `--help` is present for commands passed
  directly to `cargo`.
* `cargo-component` now uses the command adapter for binary examples and
  test/bench artifacts.
* `cargo-component` now displays the same "Running" message that `cargo` does
  when using the `run`, `test`, and `bench` commands.
* Some paths displayed in status messages have been prefix-stripped based on
  the CWD.

This commit also contains a bit of refactoring to `run_cargo_command`, as it
was getting long and unwieldy.

Fixes #236.

* Remove `Option` from component section in metadata.

This makes using the section more ergonomic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants