Fix some issues with future encodings #1270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently there's a few encoding changes in-flight in wasm-tools. Two relevant ones here are: * How WIT packages are encoded (`WIT_COMPONENT_ENCODING_V2`, bytecodealliance#1252) * How type information is embedded in core wasm files (`WIT_COMPONENT_NEW_ENCODE`, bytecodealliance#1260) Currently these don't end up playing well together. If the new WIT package encoding is used then it breaks when combined with the new metadata encoding. This commit seeks to rectify this situation. Currently this wasn't previously tested on CI due to this particular combination of flags not being on in the tests. The fixes here largely amount to refactoring to delete some duplicated code. Previously the v1/v2 split introduced an `encode_world` that was defined in both modules, but the output was different for both modules and only one would work when fed to decoding. This commit removes this split and leaves only one canonical `encode_world` function since it should be the same for both as well. This then additionally updates `WIT_COMPONENT_NEW_ENCODE=1` to preserve a lookalike format of a WIT package. Furthermore `decode_world` is updated to expect this structure as well. The intention is that the encoded metadata is as-if there was a single world in a WIT package. These refactorings fix the prior buggy behavior between the two new encodings. Note that this was only exposed through `WIT_COMPONENT_NEW_ENCODE=1` which was only very recently added. Previous builds using only `WIT_COMPONENT_ENCODING_V2=1` should continue to work fine and produce the same output as before.
pchickey
approved these changes
Oct 31, 2023
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
Nov 6, 2023
Going to pull bytecodealliance#1274, bytecodealliance#1270, and bytecodealliance#1269 into Wasmtime.
Merged
alexcrichton
added a commit
that referenced
this pull request
Nov 6, 2023
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
Nov 27, 2023
This commit enables the support for the new format of type information embedded in custom sections for `wit-component` first implemented in bytecodealliance#1260. The new format minimizes type information to only that which was bound for the `world` selected as opposed to all packages and interfaces regardless of whether they're referenced or not. A bug with this format was fixed with bytecodealliance#1270 and shipped in Wasmtime 15 so there's at least one stable release that supports the old and the new without bugs. This commit turns on the new format by default while leaving in an environment variable to turn it off. This isn't expected to have any practical breakage at this time.
alexcrichton
added a commit
that referenced
this pull request
Nov 28, 2023
This commit enables the support for the new format of type information embedded in custom sections for `wit-component` first implemented in #1260. The new format minimizes type information to only that which was bound for the `world` selected as opposed to all packages and interfaces regardless of whether they're referenced or not. A bug with this format was fixed with #1270 and shipped in Wasmtime 15 so there's at least one stable release that supports the old and the new without bugs. This commit turns on the new format by default while leaving in an environment variable to turn it off. This isn't expected to have any practical breakage at this time.
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
Jan 9, 2024
This commit is a follow-up to bytecodealliance#1260, bytecodealliance#1270, and bytecodealliance#1308. This is the next step in the transition to updating how type information is encoded in object files in core WebAssembly binaries when transforming them into a component. The old format has been enabled by default since the 1.0.54 release of `wasm-tools` in late November. and has been supported since the 1.0.51 release in early November. Support has been preserved for continuing to emit the old format in case any tooling accidentally hadn't updated but now feels like the right time to remove the ability to emit the old format. The old format can still be parsed if historical object files are in use and the hope is that it's not too bad to leave that piece sticking around for awhile.
alexcrichton
added a commit
that referenced
this pull request
Jan 9, 2024
This commit is a follow-up to #1260, #1270, and #1308. This is the next step in the transition to updating how type information is encoded in object files in core WebAssembly binaries when transforming them into a component. The old format has been enabled by default since the 1.0.54 release of `wasm-tools` in late November. and has been supported since the 1.0.51 release in early November. Support has been preserved for continuing to emit the old format in case any tooling accidentally hadn't updated but now feels like the right time to remove the ability to emit the old format. The old format can still be parsed if historical object files are in use and the hope is that it's not too bad to leave that piece sticking around for awhile.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently there's a few encoding changes in-flight in wasm-tools. Two relevant ones here are:
WIT_COMPONENT_ENCODING_V2, Change the encoding of wit definitions #1252)WIT_COMPONENT_NEW_ENCODE, Minimize bindgen type information #1260)Currently these don't end up playing well together. If the new WIT package encoding is used then it breaks when combined with the new metadata encoding. This commit seeks to rectify this situation. Currently this wasn't previously tested on CI due to this particular combination of flags not being on in the tests.
The fixes here largely amount to refactoring to delete some duplicated code. Previously the v1/v2 split introduced an
encode_worldthat was defined in both modules, but the output was different for both modules and only one would work when fed to decoding. This commit removes this split and leaves only one canonicalencode_worldfunction since it should be the same for both as well.This then additionally updates
WIT_COMPONENT_NEW_ENCODE=1to preserve a lookalike format of a WIT package. Furthermoredecode_worldis updated to expect this structure as well. The intention is that the encoded metadata is as-if there was a single world in a WIT package.These refactorings fix the prior buggy behavior between the two new encodings. Note that this was only exposed through
WIT_COMPONENT_NEW_ENCODE=1which was only very recently added. Previous builds using onlyWIT_COMPONENT_ENCODING_V2=1should continue to work fine and produce the same output as before.