WebAssembly / component-model

Repository for design and specification of the Component Model
Other
897 stars 75 forks source link

Describe "explicit"-style WIT package grammar #340

Closed azaslavsky closed 1 month ago

azaslavsky commented 2 months ago

This enables the inclusion of multiple package definitions in a single .wit file, as discussed in https://github.com/WebAssembly/component-model/issues/313.

alexcrichton commented 2 months ago

An interesting consequence of being able to intermix files that have package foo:bar; and files that have package foo:baz { ... } is that we may no longer need the deps directory in wit-parser. You could, for example, have wasi-*.wit in the wit folder directly perhaps. Not that this is necessarily a good idea, but it's an idea this would open up.

ydnar commented 2 months ago

This is great—it would allow round-trip serialization of a Resolve back into WIT in a single consumable file.

lukewagner commented 1 month ago

Does this look good to merge, or would we like to wait for any additional implementation feedback?

azaslavsky commented 1 month ago

I'm happy to merge - do y'all do contributor-triggered merges, or maintainer?

On Fri, May 10, 2024, 07:58 Luke Wagner @.***> wrote:

Does this look good to merge, or would we like to wait for any additional implementation feedback?

— Reply to this email directly, view it on GitHub https://github.com/WebAssembly/component-model/pull/340#issuecomment-2104760227, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4JX6POMR2FZQSGQ36QUB3ZBTOARAVCNFSM6AAAAABGDPSG5GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBUG43DAMRSG4 . You are receiving this because you authored the thread.Message ID: @.***>

lukewagner commented 1 month ago

Mostly the latter. For WIT changes, I've generally been trying to keep WIT.md reflective of what is actually in the WIT toolchains (or, if not, gating the feature via a unicode emoji), so I was mostly curious if the implementation of this feature has been implemented and merged.

alexcrichton commented 1 month ago

This hasn't yet been merged into wasm-tools (or others as far as I know). That being said I think it's reasonable to merge here 👍

lukewagner commented 1 month ago

Ah thanks. Do you think it might be merged soonish, or would it make sense to emoji-gate this feature?

alexcrichton commented 1 month ago

I've not been working on it myself, but @azaslavsky I recall you were working on an initial implementation I think?

azaslavsky commented 1 month ago

@alexcrichton I am. Its fallen a bit on my priority list due to $dayjob concerns, but I should have the revised PR up by end of week.

azaslavsky commented 1 month ago

https://github.com/bytecodealliance/wasm-tools/pull/1577 implementing this has merged, so I think this can land as well?

lukewagner commented 1 month ago

Awesome, thanks again!