Victor Adossi (Jul 17 2024 at 19:58): 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:
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)
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?
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
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.
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)...
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.
Victor Adossi (Jul 18 2024 at 14:44):Yeah I could definitely see/understand that. Then the ideal use case is generally:
0.2.1)0.2.0to 0.2.1 somehow0.2.1 contains @since(0.2.1) annotations and @since(0.2.0) annotationsI 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,bcan be used, butccannot. An important expectation set by the@sincegate 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?
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.
Alex Crichton (Jul 18 2024 at 14:48):This is probably something I should have raised on the original PR though
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"?
Alex Crichton (Jul 18 2024 at 14:56):Yes that sort of support is already implemented in Wasmtime and planned for other runtimes
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
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
Victor Adossi (Jul 18 2024 at 14:58):Ah yes -- I do remember seeing that in a PR (relatively recently?) IIRC!
Victor Adossi (Jul 18 2024 at 15:00):Here's another possibility -- Does it seem reasonable to make this a bug?
Victor Adossi (Jul 18 2024 at 15:01):i.e. error if a version number that is higher than the current package is used?
Alex Crichton (Jul 18 2024 at 15:03):I think that might be reasonable yeah
Alex Crichton (Jul 18 2024 at 15:03):help catch mistakes and/or raise more awareness of this issue on issue trackers
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:
@since on a future version 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).
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
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
Last updated: Nov 22 2024 at 16:03 UTC