Daniel Vigovszky (Jun 06 2023 at 14:42): Daniel Vigovszky (Jun 06 2023 at 14:42):Hi! I'm trying to use the latest cargo-component (and everything else), including the new WIT syntax with packages etc. I used to have some example components built with cargo-component for testing some WASI preview2 host implementations, which I did by just putting the relevant WIT files in the deps directory. This worked as expected but no longer because the latest cargo-component no longer calls wit-parser's Resolve.push_dir: https://github.com/bytecodealliance/cargo-component/pull/83/files#diff-c030113407a164cbd1ad3500c613f59abd4346ba13ee5b653c56f393c937b0a8L373-R363 Because of this change the deps subdirectory of the component's wit dir never gets parsed and the only way to add dependencies is to depend on _components_ either using a registry or a local path but not just by providing WITs. My question is whether this is intentional or not?
Bailey Hayes (Jun 06 2023 at 15:14):@Peter Huene is the best person to answer this. He wrote a document describing this design. The way I interpret it and the last example at the end, is that a registry is not necessarily required and a wit directory can be passed in directly: https://github.com/bytecodealliance/cargo-component/blob/main/docs/design/registries.md#exporting-only-functions-from-a-component
Daniel Vigovszky (Jun 06 2023 at 15:21):Yes I read this. The wit directory can be passed directly and it parses it as a wit package, but it no longer reads the deps subdir (because that is implemented in wit-parser and only happens if a _directory_ is pushed to the resolver, but after the latest change this is no longer the case - instead UnresolvedPackage::parse_dir get called which is not doing this recursive loading of dependencies. It feels like a bug to me but of course it is fully possible I'm missing something about the direction. If it is a bug though, I'm happy to try and submit a fix
Peter Huene (Jun 06 2023 at 15:49):This wasn't an intentional break, I forgot that deps is only checked that way.
I have a fix in mind that I'll try to get up today.
Daniel Vigovszky (Jun 06 2023 at 15:49):Oh nice thank you!
Peter Huene (Jun 06 2023 at 18:25):So unfortunately my fix may require a little bit of change on your end to make work, but with my fix it should be possible to replicate the same deps/ behavior.
With the fix, you should be able to point cargo-component to WIT packages on disk as the dependencies to your WIT. The WIT packages themselves can have deps/ directories in them, but your top-level dependencies in the WITs you are authoring need entries in Cargo.toml:
[package.metadata.component.target]
path = "wit"
[package.metadata.component.target.dependencies]
"foo:bar" = { path = "wit/deps/foo-bar" }
"baz:jam" = { path = "wit/deps/baz-jam" }
...
This is an unfortunate consequence of supporting both dependencies from a registry and local; wit-parser doesn't know anything about resolving foreign dependencies other than an explicit Resolve::push_dir which will _only_ resolve dependencies from deps/, which prevents cargo-component to also support the registry.
I hope this isn't too big of a hurdle for you.
Daniel Vigovszky (Jun 07 2023 at 07:02):I actually tried exactly doing this in Cargo.toml before digging into the code and understanding how it (does not) works :) So I think it is a good compromise
Daniel Vigovszky (Jun 07 2023 at 17:01):@Peter Huene I tried the fix and I think we have one more problem - the listed dependencies are resolved in isolation so they cannot refer to each other. For example if I the wasi WITs from the wasmtime repo in the deps and try to depend on wasi:http, it referes to wasi:io and wasi:poll in the http module, and adding those to Cargo.toml does not help. A workaround would be to duplicate all these cross-references in deps withinhttp etc but I think it should work by adding all these listed dependencies to a shared world.
Peter Huene (Jun 07 2023 at 17:03):Good point, yeah, let's see if we can improve this; otherwise I'll just do what Dan suggested in the PR and use push_dir if there's a deps directory detected and just do what we did prior (can't mix-and-match registry dependencies with local dependencies)
Peter Huene (Jun 07 2023 at 17:16):I think the plan of attack here is to duplicate what push_dir is doing internally, namely the topological sort and visiting of the dependencies (thus, the ordering in Cargo.toml doesn't matter), and merge them into a singular shared resolve.
The chief difference between the two is that cargo-component won't scan the file system.
Peter Huene (Jun 08 2023 at 01:53):I put up https://github.com/bytecodealliance/cargo-component/pull/86 with that fix; let me know if that improves things
Daniel Vigovszky (Jun 08 2023 at 04:50):Looks good, I will try it out today!
Daniel Vigovszky (Jun 08 2023 at 07:29):Tried it out, it works well in my examples! Thank you!
Notification Bot (Jun 08 2023 at 08:32):Daniel Vigovszky has marked this topic as resolved.
Last updated: Nov 22 2024 at 16:03 UTC