Stream: wasm

Topic: Missing impl for @since in `Resolve::include_stability()`


view this post on Zulip Victor Adossi (Jul 17 2024 at 19:58):

Hey all, want to make sure I'm not missing anything obvious before I submit a PR, but is @since actually just missing implementation here:

https://github.com/bytecodealliance/wasm-tools/blob/263b697a442273ef9848e73546c770f5b7f6e2a9/crates/wit-parser/src/resolve.rs#L1334

Is this just not finished yet? It should actually be checking the since and feature members of Stability::Stable but I think this isn't implemented yet (and needs to be)

CLI and Rust libraries for low-level manipulation of WebAssembly modules - bytecodealliance/wasm-tools

view this post on Zulip Alex Crichton (Jul 17 2024 at 20:05):

Are you thinking that the packages version would be checked against @since and the item would be omitted if the package's version was too small?

view this post on Zulip Alex Crichton (Jul 17 2024 at 20:05):

Otherwise though it's intentional that currenly the version number on @since is ignored there, but I may be missing a use case

view this post on Zulip Victor Adossi (Jul 18 2024 at 04:23):

Yup that's exactly what I was thinking! Given a file like this:

package test:feature-gates@0.2.1;

interface foo {
  a: func();

  @since(version = 0.2.1)
  b: func();

  @since(version = 0.2.2)
  c: func();

  @unstable(feature = fancier-foo)
  d: func();
}

I was thinking that both c and d should be missing.

view this post on Zulip Victor Adossi (Jul 18 2024 at 04:25):

Ah the full context might be more useful -- I'm working on adding feature gate support to jco and this is exactly my test case.

@unstable works great, but what I was finding was that c still shows up (when I'd expect it not to). Though I do think that the case is a bit contrived -- normally the user would get an entirely new WIT (with c included, where as in the past it wasn't present)...

This PR updates jco to support the @since, @after and @unstable annotations as implemented by the upstream Rust tooling. Currently there are a few issues left to address: We likely need an upstre...

view this post on Zulip Alex Crichton (Jul 18 2024 at 14:29):

I could personally go either way on this. On one hand it seems fine to add. On the other hand I'm doubtful this will show up in practice. Using @since with a future version number seems akin to retroactively modifying a release which generally isn't great practice.

view this post on Zulip Victor Adossi (Jul 18 2024 at 14:44):

Yeah I could definitely see/understand that. Then the ideal use case is generally:

I think what makes me think this usecase is intended is when reading the WIT design:

Thus, when building a component targeting, e.g., 0.2.1, b can be used, but c cannot. An important expectation set by the @since gate is that, once applied to an item, the item is not modified incompatibly going forward (according to general semantic versioning rules).

How would anyone targeting 0.2.1 ever even see c(in that example), unless it was added to the WIT for 0.2.1 itself? Is there a targeting mechanism I'm missing?

Repository for design and specification of the Component Model - WebAssembly/component-model

view this post on Zulip Alex Crichton (Jul 18 2024 at 14:48):

I think this is something that I didn't fully understand when reading over the original PR for this. To me targetting an older version is done by checking out the WIT files as they were during that release. That's different than taking the latest WIT files and setting a version of "ignore everything above this version".

To me it's much more robust and reliable to use the older WIT files rather than always using the newest WIT files in a situation like this.

view this post on Zulip Alex Crichton (Jul 18 2024 at 14:48):

This is probably something I should have raised on the original PR though

view this post on Zulip Victor Adossi (Jul 18 2024 at 14:51):

To me it's much more robust and reliable to use the older WIT files rather than always using the newest WIT files in a situation like this.

Yeah so that definitely makes sense to me -- I think that's the sort of... PL/toolchain default -- you can't use the new features by virtue of them not being present. I think this is a more natural flow.

That's different than taking the latest WIT files and setting a version of "ignore everything above this version".

Do we think there could be a different way in the future to do this targeting? For example if the host knows of 0.2.2, and the component only knows of 0.2.1, is this a way (unfortunately somewhat dependent on people using @since quite religiously and correctly) for the same WIT file to support both interfaces for "free"?

view this post on Zulip Alex Crichton (Jul 18 2024 at 14:56):

Yes that sort of support is already implemented in Wasmtime and planned for other runtimes

view this post on Zulip Alex Crichton (Jul 18 2024 at 14:57):

It's not fully documented in the component-model itself but with this section the idea is that if two version numbers canonicalize to the same string then that's what the runtime will support

Repository for design and specification of the Component Model - WebAssembly/component-model

view this post on Zulip Alex Crichton (Jul 18 2024 at 14:57):

so 0.2.1 and 0.2.2 both canonicalize to "0.2" so the runtime would support both

view this post on Zulip Victor Adossi (Jul 18 2024 at 14:58):

Ah yes -- I do remember seeing that in a PR (relatively recently?) IIRC!

view this post on Zulip Victor Adossi (Jul 18 2024 at 15:00):

Here's another possibility -- Does it seem reasonable to make this a bug?

view this post on Zulip Victor Adossi (Jul 18 2024 at 15:01):

i.e. error if a version number that is higher than the current package is used?

view this post on Zulip Alex Crichton (Jul 18 2024 at 15:03):

I think that might be reasonable yeah

view this post on Zulip Alex Crichton (Jul 18 2024 at 15:03):

help catch mistakes and/or raise more awareness of this issue on issue trackers

view this post on Zulip Victor Adossi (Jul 18 2024 at 15:03):

Thinking that maybe @Yoshua Wuyts and @Guy Bedford might want to weigh in on this as well. Seems like we could really go any one of the following ways:

view this post on Zulip Victor Adossi (Jul 18 2024 at 15:07):

Yep, I'm also fine with making it a bug -- I can't imagine a use case yet where a package at x would include code that targeted x + 1... Feels like in that case people should just be using x + 1, as we've discussed...

The only thing that sort of occurs to me is monkey/hot patching, but seems like a shaky rationale (given that if you can change the existing WIT file, then you can probably upgrade it).

view this post on Zulip Alex Crichton (Jul 18 2024 at 15:35):

yeah I think it's best to start more conservatively with a strict error here and it can always be relaxed in the future if necessary

view this post on Zulip Victor Adossi (Jul 18 2024 at 17:44):

OK I've got a first cut at the PR here:

https://github.com/bytecodealliance/wasm-tools/pull/1689

This commit introduces some extra checking (and panicking) for feature gates that are stable (i.e. Stability::Stable) AKA @since with a version and/or features specified. There are two primary chan...

Last updated: Dec 23 2024 at 12:05 UTC