Support lists of WITs in Resolve::select_world#2288
Support lists of WITs in Resolve::select_world#2288alexcrichton merged 5 commits intobytecodealliance:mainfrom
Resolve::select_world#2288Conversation
This extends `Resolve::select_world` to be usable for lists of WITs. One such example is wit-bindgen's `generate` macro's `path:` parameter, which can take a list of WIT paths. It currently uses a [custom `select_world` implementation], but with this patch in wit-parser, it would be able to use `Resolve::select_world` instead. I'm also considering extending the wit-bindgen CLI to accept multiple WIT paths; with this patch, it could use `Resolve::select_world` as well. [custom `select_world` implementation]: https://github.com/bytecodealliance/wit-bindgen/blob/0e2201b8629b4144f45867161de7bc1fe26133bb/crates/guest-rust/macro/src/lib.rs#L191 Specifically, this PR makes the package argument to `Resolve::select_world` an `Option`, and adds code to handle the case where it's `None`. A `None` means the caller has a list of WITs and as such doesn't have a single main package. This also rewrites the documentation for `select_world` to clean up already-obsolete references to the `packages` array argument, and to hopefully make it more clear how `select_world` makes its choice.
alexcrichton
requested review from
alexcrichton
and removed request for
fitzgen
|
I know that this function has a long and storied history of flip-flopping between various signatures and semantics, and I also don't remember everything as well. WIth this PR though it has a subtly different semantics than the Basically: do you think the "main package" argument should be a "main packages" list instead of |
|
Going by the git logs, the switch to using one main package was in #1700, which introduced the idea of requiring a single root package, which I think still makes sense within a given WIT directory. I think what's new now is that we're also supporting lists of independent WIT directories, and at this level, there may not be a single root package connecting them all. With that understanding, I think it makes sense to switch back to a list of main packages, so I've now updated the patch to do this. |
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks for digging that up!
|
Oh I should also mention:
Sounds reasonable to me! |
Accept multiple WIT paths in the wit-bindgen CLI, using the same logic as the multiple WIT paths accepted by the `path` parameter in the Rust bindgen macro. This depends on bytecodealliance/wasm-tools#2288, which is not yet released.
Accept multiple WIT paths in the wit-bindgen CLI, using the same logic as the multiple WIT paths accepted by the `path` parameter in the Rust bindgen macro. This depends on bytecodealliance/wasm-tools#2288, which is not yet released.
Accept multiple WIT paths in the wit-bindgen CLI, using the same logic as the multiple WIT paths accepted by the `path` parameter in the Rust bindgen macro. This depends on bytecodealliance/wasm-tools#2288, which is not yet released.
2 participants
This extends
Resolve::select_worldto be usable for lists of WITs. One such example is wit-bindgen'sgeneratemacro'spath:parameter, which can take a list of WIT paths. It currently uses a customselect_worldimplementation, but with this patch in wit-parser, it would be able to useResolve::select_worldinstead.I'm also considering extending the wit-bindgen CLI to accept multiple WIT paths; with this patch, it could use
Resolve::select_worldas well.Specifically, this PR makes the package argument to
Resolve::select_worldanOption, and adds code to handle the case where it'sNone. ANonemeans the caller has a list of WITs and as such doesn't have a single main package.This also rewrites the documentation for
select_worldto clean up already-obsolete references to thepackagesarray argument, and to hopefully make it more clear howselect_worldmakes its choice.