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.

Sign up for GitHub

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

Jump to bottom

Support list of paths in component bindgen macro #9249

Open
lwansbrough opened this issue Sep 15, 2024 · 3 comments · May be fixed by #9288
Open

Support list of paths in component bindgen macro #9249

lwansbrough opened this issue Sep 15, 2024 · 3 comments · May be fixed by #9288

Comments

@lwansbrough
Copy link

wit-bindgen includes the option to provide a list of paths for the wit sources. This makes it more practical to satisfy the requirements of imports from other packages. Enabling this behaviour for the component bindgen macro would be helpful.

wit-bindgen docs:

// Path to parse WIT and its dependencies from. Defaults to the `wit`
// folder adjacent to your `Cargo.toml`.
//
// This parameter also supports the form of a list, such as:
// ["../path/to/wit1", "../path/to/wit2"]
// Usually used in testing, our test suite may want to generate code
// from wit files located in multiple paths within a single mod, and we
// don't want to copy these files again.
path: "../path/to/wit",
@alexcrichton
Copy link
Member

Thanks for the report! To link things here this was implemented in wit-bindgen in bytecodealliance/wit-bindgen#1003 and I think it'd be reasonable to copy over a similar structure into Wasmtime as well. @lwansbrough would you be interested in creating a PR perhaps?

@lwansbrough
Copy link
Author

@alexcrichton I'll see what I can do.

I was looking through that commit and this looks like a bug to me. https://github.com/bytecodealliance/wit-bindgen/blob/3e3877da85daf1ed98bd1aee7007e496461c3bd1/crates/guest-rust/macro/src/lib.rs#L215

Line 237 passes root.join("wit") which would mean root is joined to root.

@alexcrichton
Copy link
Member

Good catch! Semantically that'll work out ok since "/a".join("/b") yields "/b", so the end result is the same. That being said it's unnecessarily complex, and just passing in "wit" instead of root.join("wit") should also work and be cleaner.

@lwansbrough lwansbrough linked a pull request Sep 19, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: component bindgen: add support for multiple wit paths lwansbrough/wasmtime
2 participants
@alexcrichton @lwansbrough