Repository navigation
Make component target semver compatible - #2763
Conversation
| /// | ||
| /// Imports and exports of nested modules and components are not considered. | ||
| /// This does not validate `bytes`. | ||
| pub fn has_canonical_names(bytes: &[u8]) -> Result<bool> { |
There was a problem hiding this comment.
Would it be possible to skip this? I'm not sure the purpose of this function and it's kind of subtle only checking the root, and not for example instance exports of the root.
There was a problem hiding this comment.
Make sense. I was hoping to use this as an optimization that if the binary is already canonicalized, we can skip the reencode. Checking the root is okay for target, because subtyping allows more exports than the target world. But as a general function, it does miss those cases. I will remove it.
| // The merge above already ensures imports have unique canonical names, | ||
| // so this mainly catches semver-compatible duplicate exports. | ||
| check_duplicate_canonical_names(&test_resolve, test_world)?; |
There was a problem hiding this comment.
Could a test be added for this? I'm not entirely sure what purpose this part here is serving
There was a problem hiding this comment.
It's not strictly needed. It's mainly to exclude exports with both 0.1.0 and 0.1.1, but the encoder would catch that anyway.
chenyan2002
left a comment
chenyan2002
left a comment
There was a problem hiding this comment.
I also add some fix in the wasmparser to include full version when reporting errors.
| /// | ||
| /// Imports and exports of nested modules and components are not considered. | ||
| /// This does not validate `bytes`. | ||
| pub fn has_canonical_names(bytes: &[u8]) -> Result<bool> { |
There was a problem hiding this comment.
Make sense. I was hoping to use this as an optimization that if the binary is already canonicalized, we can skip the reencode. Checking the root is okay for target, because subtyping allows more exports than the target world. But as a general function, it does miss those cases. I will remove it.
| // The merge above already ensures imports have unique canonical names, | ||
| // so this mainly catches semver-compatible duplicate exports. | ||
| check_duplicate_canonical_names(&test_resolve, test_world)?; |
There was a problem hiding this comment.
It's not strictly needed. It's mainly to exclude exports with both 0.1.0 and 0.1.1, but the encoder would catch that anyway.
2 participants
Fix #2220
emit_canonical_namestosemver_compatible.semver_compatibleis enabled,component targetfirst checks if the binary is already using canonical names. If not, we decode the WIT, merge imports, re-encode WIT with canonical names. Then we can type check the WIT against the target world all based on canonical names.wasmparserto include full version when reporting errors.