-
Notifications
You must be signed in to change notification settings - Fork 83
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
Support inline packages #313
Comments
Yep, that use case and syntax makes sense to me. I think we can even encode the WIT syntax you wrote today as: (component
(type (export "wasi:http/[email protected]") ...)
(type (export "wasi:http/[email protected]") ...)
(type (export "wasi:http/[email protected]") ...)
(type (export "wasi:random/[email protected]") ...)
(type (export "wasi:random/[email protected]") ...)
(type (export "wasi:random/[email protected]") ...)
(type (export "wasi:random/[email protected]") ...)
) Rendering that back to WIT would then just group Another closely-related use case that I think wants the same feature is "show me a single self-contained (no deps) .wit for a given component". Right now this isn't possible because you can't put multiple packages into a single Let's say I'm rendering the WIT from a ;; component.wasm
(component
(import "wasi:http/outgoing-handler" ...)
(import "custom-callback" (func (param "s" string) (result string)))
...
(export "wasi:http/incoming-handler" ...)
) where I'm importing or exporting interfaces that aren't published anywhere and thus I'm targeting a world that isn't published anywhere. While we could just force the tool to invent a synthetic package and world name, we've generally tried hard not to have to invent spurious names. Instead, it seems like what I'd like to see is roughly (modulo bikeshedding): package wasi:http {
interface outgoing-handler { ... }
interface incoming-handler { ... }
}
world {
import wasi:http/outgoing-handler;
import callback: func(s: string) -> string;
export wasi:http/incoming-handler;
} The idea is to allow a single unnamed And then, going the other direction: when I'm authoring a custom component, this is the syntax I'd be able to write and feed into, e.g., WDYT? |
I think it's a good goal to make names optional myself, and I agree that an optional-names-package, if it existed, would be perfect for "print the WIT of this component". There's at least a bit to figure out how it all interacts though because at some point a interface foo {}
world {
import foo; // what's the ID of this interface?
} Also having an unnamed world within a |
Yeah, it seems to me like this feature would bring WIT more up to parity with the binary encoding. Regarding the world name, that seems like a bit of an orthogonal question? It seems like that problem exists with the current wasm-to-wit output just the same. And regardless, maybe we can treat it as orthogonal by for now having tools use the |
Yes, I think it's a good idea to say you can have a When using an unnamed world, I think the ground truth that we need to reflect in the WIT syntax is: there are no Thus, as a starting point, I think we could say that it's an error to have a named package wasi:http {
interface incoming-handler { ... }
}
world {
import wasi:http/incoming-handler;
import foo: interface {
...
}
} Assuming we extend However, this would force repetition in cases where WAT could define the Incidentally, this question also came up in a WAC setting, and the idea there was to use let foo = interface { ... }
world {
import bar: foo;
import baz: foo;
} and to be clear, this would be the type for a component: (component
(type (instance ...))
(import "bar" (instance (type 0)))
(import "baz" (instance (type 0)))
...
) Incidentally, the WDYT? |
I'd personally be tempted to not bite off too much here. Once we're in the realm of deduplicating types that feels pretty far afield of the OP here of inline packages, and AFAIK there's not really an issue with duplicating types today anyway. (I'd also agree that a single anonymous world is a bit orthogonal from inline packages too) So going back to inline packages and pondering a bit, some questions I can come up with (perhaps with obvious answers but figure I should list them out):
That's at least the initial questions I can think of. This sort of gives rise to a first-class concept of merging packages. Right now the Rust tooling already handles that as it comes up in a number of other places, for example if you've got two object files both with bindings they might both refer to WASI things. More commonly the main module may refer to WASI but the adapter also refers to WASI. Right now the package merging step is pretty dumb and probably not the algorithm we'd want. With WIT packages being able to be defined inline we'd probably want to formally define what it means to merge packages. |
Yes, I'm quite happy to not focus on de-duplicating types; I mostly just wanted to preemptively address the issue with the solution to the question of what to do with interfaces in the context of anonymous worlds. Anonymous worlds, do seem rather timely and closely related and enabled by nested packages, but I'm also fine if we want to punt on them as well.
Given that what really matters is the distinctness of the names of the individual
If we use the encoding I mentioned above, the following two WIT documents would encode to the exact same thing: package ns:foo;
interface i {}
package ns:bar {
interface j {}
} package whocares:whatever;
package ns:foo {
interface i {}
}
package ns:bar {
interface j {}
} so why require the vestigial
I don't think it'd provide any value, so I'd go with no.
Agreed. This all sortof goes along with the "there can be multiple package definitions for the same package name as long as they don't intersect" suggestion I made in my first answer above. |
Oh I didn't know that C++ worked like that! But yeah I think what you say makes sense, and especially if we define what it means to merge packages then it sort of naturally falls out without too much work.
That sounds reasonable to me, yeah |
Hi! I've been attempting to implement the narrow version of this (that is, no anonymous worlds, no attempt at addressing type duplication issues, etc) over the last few evenings. It's been fairly tractable so far, though one consequence of this change is that the This spiders outward pretty quickly: by my rough count there are transitively a three dozen or so call sites that would need to change to accept a list of packages rather than a single instance. My work so far (most, though not all, tests passing) is at https://github.com/azaslavsky/wasm-tools/tree/component-model-313. The basic setup right now has two "modes": explicit mode, where the top-level of the file is solely Addressing @alexcrichton's questions above:
I haven't altered this. "Implicit" mode packages have the same rules as before, and "explicit" mode of course don't need a standalone
Not in the current draft. This felt like it would add a lot complexity.
I went the other way on this: package definitions must be in a single block in the current draft. I found it hard to think of examples (besides the aforementioned C++ namespaces) where blocks in language syntax work this way, and it felt very unnatural to me. Also, it makes implementation easier to not worry about it during resolution, and just treat every package tree as its own, self-contained whole. If folks feel strongly about going the other way, I don't think the current setup blocks adding this capability later.
This is admittedly untested in the current draft, but I think it just goes in the I'm going to polish up the above branch by resolving the remaining TODOs and adding some more tests, particularly of the |
This enables the inclusion of multiple package definitions in a sngle `.wit` file, as discussed in WebAssembly#313.
Thanks for working on this @azaslavsky! In terms of implementation I might recommend replacing As for all the consequences of your implementation, they sound good to me 👍. Only allowing one package block is at least a conservative starting point and we can always relax it later as necessary.
I think that's ok yeah, I mostly want to make sure that these sorts of cases are handled and at the very least don't ignore what's written in WIT files. Whether it unions or errors I'm ambivalent about myself but sounds like it'll get handled one way or another so I think that's fine. |
This enables the inclusion of multiple package definitions in a sngle `.wit` file, as discussed in #313.
Closing, since I think this has all since merged and landed; feel free to reopen if I missed something, though. |
@alexcrichton brought up the idea of supporting packages in-line in a file in addition to the
package
statement. This would help with bundling world definitions into single files as part of things like runtime / language SDKs, which is often very useful.As a concrete example, wasmtime is considering to include binary representations of the two worlds it implements instead of the
deps
folder to ease artifact management. That however makes handling these artifacts more difficult in other ways, including simple human inspection.If we were able to bundle all the wit files into a single one, containing inline packages, that'd be a much better solution.
The syntax for this seems pretty straightforward to me, but I might be missing something:
The text was updated successfully, but these errors were encountered: