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

Build Failure with Inline WIT with wit-bindgen #188

Open
tqwewe opened this issue Nov 9, 2023 · 6 comments
Open

Build Failure with Inline WIT with wit-bindgen #188

tqwewe opened this issue Nov 9, 2023 · 6 comments

Comments

@tqwewe
Copy link

tqwewe commented Nov 9, 2023

Description

There's an inconsistency when building a project with inline WIT definitions using cargo component build. It fails under cargo component but succeeds when manually building with cargo and wasm-tools with wit-bindgen given the following lib.rs:

wit_bindgen::generate!({
    inline: r#"
        package component:guest;

        world example {
            export example: interface {
                hello-world: func() -> string;
            }
        }
    "#,
    exports: {
        "example": Component
    }
});

use exports::example::Guest;

struct Component;

impl Guest for Component {
    fn hello_world() -> String {
        "Hello, World!".to_string()
    }
}

Reproduction Steps

Manual Build with Cargo and wasm-tools

  1. Build for wasm32-wasi target:
    cargo build --package guest --target wasm32-wasi
  2. Create the component with wasm-tools:
    wasm-tools component new ./target/wasm32-wasi/debug/guest.wasm -o ./target/wasm32-wasi/debug/guest.wasm --adapt ./wasi_snapshot_preview1.wasm
  3. Run the host package and observe successful Hello, World! message:
    cargo run --package host

Build with Cargo Component

  1. Clean previous builds:
    cargo clean
  2. Build with cargo component:
    cargo component build --package guest
  3. Observe failure when running the host package:
    cargo run --package host

Expected Behavior

Both build methods should successfully compile and run the project, displaying Hello, World! regardless if we use a wit directory with cargo_component_bindings or wit-bindgen.

Actual Behavior

The build succeeds with manual steps but fails with cargo component.

Additional Information

Repository Link: GitHub Repository

Is this intended behaviour?
I think it would be ideal if we can just use cargo component with wit-bindgen as an easy way of building wasm components, though it seems like we need to stick to the structure of having a wit directory along side our src directory.

@peterhuene
Copy link
Member

Could you provide the build error you're encountering?

@peterhuene
Copy link
Member

The guest project is missing the package metadata that marks it as a component project; currently cargo-component does not attempt to componentize just any wasm target outputs.

If you add [package.metadata.component] to your guest's Cargo.toml, does that workaround the issue?

@tqwewe
Copy link
Author

tqwewe commented Nov 10, 2023

Could you provide the build error you're encountering?

Ah sorry I forgot to add this 🤦
I get an error from wasmtime loading the wasm module:

thread 'main' panicked at host/src/main.rs:27:92:
called `Result::unwrap()` on an `Err` value: failed to parse WebAssembly module

Caused by:
    attempted to parse a wasm module with a component parser
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

If you add [package.metadata.component] to your guest's Cargo.toml, does that workaround the issue?

It does! However, would cargo component be willing to change this behaviour and add the component metadata no matter if the [package.metadata.component] is present?

The reason being is that:

  • If someone is compiling their cargo crate with cargo component build, then they'd definitely have the intention of building it as a wasm component.

  • I'm building a runtime which uses wasmtime, and users of my runtime will compile their modules as cargo components. However telling them that they need to add an empty [package.metadata.component] to get it working, despite them already using cargo component is just one extra gotchya that seems not too necessary?

    Ideally, they could just use cargo new, and then add crate-type = ["cdylib"], and my macro thalo::export_aggregate!(Foo) and it should work with cargo component build

@esoterra
Copy link
Collaborator

esoterra commented Nov 13, 2023

Why wouldn't they use cargo component new which sets both the crate-type and adds the required [package.metadata.component] section? As a bonus, it also gives them the opportunity to configure different cargo component values with arguments (e.g. package name, editor, vcs).

Alternatively, they could just use a template you create so you can embed your thalo macro call for them.

@tqwewe
Copy link
Author

tqwewe commented Nov 13, 2023

Why wouldn't they use cargo component new

It would indeed be nice, however it also generates the wit directory, and adds cargo-component-rust to the dependencies, etc. These things aren't needed in my case. I don't want them to be concerned with wit files or anything like that, as my library handles it all.

Alternatively, they could just use a template you create so you can embed your thalo macro call for them.

Also a nice idea, but I wanted to keep things as simple as possible for them. So it's just a regular rust crate, and they need thalo as a dependency. The only complicated thing would be that they need to add the crate-type, but that's just a rust thing with wasm.

As I mentioned, isn't it a little strange to have the behaviour of the cargo component build silently not generate a wasm component if the meta is not defined?
If someone uses the cargo component build command then I'd say it's safe to assume they're definitely expecting the resulting wasm to be a valid wasm component

@peterhuene
Copy link
Member

peterhuene commented Mar 2, 2024

@tqwewe

With cargo-component 0.9.0, it now componentizes any core module that has bindings type information present (where previously it required [package.metadata.component] to be present in Cargo.toml for it to componentize).

Please review the release notes as there were quite a few breaking changes relating to bindings in that release.

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 a pull request may close this issue.

3 participants