-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Promote the wasm32-wasip2
target to Tier 2
#126967
Conversation
rustbot has assigned @Mark-Simulacrum. Use |
I'll note that rust-lang/compiler-team#760 is not complete so this shouldn't merge yet. I'm opening this up ahead of time to get feedback on the more technical side of things as opposed to the MCP-side of things. One specific question I'd have is that this PR is adding more exceptions to the license check for crates that have an
Also I realize that licensing decisions are probably best not made in PRs, so this can also be considered as mostly highlighting the issue and if you know a better forum/location to discuss this I'm happy to raise something there. |
This comment has been minimized.
This comment has been minimized.
My sense is that we're fine with extra dependencies with that license, especially in the same pool (i.e., wasm-related crates). cc @wesleywiser @davidtwco -- does that align with your expectations? Can you help drive the licensing story with the Foundation's help if needed? I'm not sure whether re-licensing is worth the trouble or not as such. Though long-term it does likely hurt our ability to move code across the boundary of these crates, so if there's expectations that might be warranted it might be worth investing while there's less people involved (and so it's easier).
Can you say how big the extra binary is? I don't remember the rationale back then, but it feels plausible that the linker should be part of the wasm target component... maybe I'm missing something obvious? (lld is distinct in that lots of targets use it -- at least now -- whereas this one seems like that may not be true for). |
☔ The latest upstream changes (presumably #127133) made this pull request unmergeable. Please resolve the merge conflicts. |
Ok sounds good on the licensing front. I opened bytecodealliance/wasm-tools#1637 and bytecodealliance/wit-bindgen#987 earlier anyway to start the process and see generally if it can be done, so that's progressing in parallel too. As for binary sizes the linker locally is ~5M (4.6 on macos and 5.2 on linux). Making this part of the wasm component would require having a binary for each supported host architeture in the component as well which I think would be pretty difficult build-wise and also would make the wasm component significantly larger. IIRC we wrestled with this at the time when adding rust-lld as well and the decision was to ship it with all host components as that was the simplest solution at the time. Now that was also guided by the fact that we wanted to eventually use LLD for more targets one day (as has since happened) and this won't ever be used for any other target. I can look into size-optimizing the binary if 5M is too large. |
Yes, I think that's ok. Obviously, it would be more ideal if we had the standard licensing applied so relicensing would still be preferable but not a blocker. Thanks @alexcrichton for starting that conversation in the bytecodealliance projects! |
This commit promotes the `wasm32-wasip2` Rust target to tier 2 as proposed in rust-lang/compiler-team#760. There are two major changes in this PR: 1. The `dist-various-2` container, which already produces the other WASI targets, now has an extra target added for `wasm32-wasip2`. 2. A new `wasm-component-ld` binary is added to all host toolchains when LLD is enabled. This is the linker used for the `wasm32-wasip2` target. This new linker is added for all host toolchains to ensure that all host toolchains can produce the `wasm32-wasip2` target. This is similar to how `rust-lld` was originally included for all host toolchains to be able to produce WebAssembly output when the targets were first added. The new linker is developed [here][wasm-component-ld] and is pulled in via a crates.io-based dependency to the tree here. [wasm-component-ld]: https://github.com/bytecodealliance/wasm-component-ld
Reuse preexisting macro and switch it to a "bootstrap tool" to try to resolve build issues.
824002b
to
1afdd45
Compare
This comment has been minimized.
This comment has been minimized.
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
Ok I've redone a bit how the build is done to fix the build error from before, notably unconditionally using the stage0 compiler for the wasm-component-ld linker to ensure that it avoids the nightly issue with the ahash dependency. The FCP has finished here and otherwise the relicensing efforts are underway and should hopefully be complete in a month or so with new published versions at which point I'll update this to remove the exceptions. In the meantime this should otherwise be ready for review and I'm happy to dig further into the binary size if that's a concern. The easiest win is to probably compile with |
@bors r+ |
Let's land this. I think we can update the licensing and consider further improvements in binary size in the future, no need to block on that. |
… r=Mark-Simulacrum Promote the `wasm32-wasip2` target to Tier 2 This commit promotes the `wasm32-wasip2` Rust target to tier 2 as proposed in rust-lang/compiler-team#760. There are two major changes in this PR: 1. The `dist-various-2` container, which already produces the other WASI targets, now has an extra target added for `wasm32-wasip2`. 2. A new `wasm-component-ld` binary is added to all host toolchains when LLD is enabled. This is the linker used for the `wasm32-wasip2` target. This new linker is added for all host toolchains to ensure that all host toolchains can produce the `wasm32-wasip2` target. This is similar to how `rust-lld` was originally included for all host toolchains to be able to produce WebAssembly output when the targets were first added. The new linker is developed [here][wasm-component-ld] and is pulled in via a crates.io-based dependency to the tree here. [wasm-component-ld]: https://github.com/bytecodealliance/wasm-component-ld
… r=Mark-Simulacrum Promote the `wasm32-wasip2` target to Tier 2 This commit promotes the `wasm32-wasip2` Rust target to tier 2 as proposed in rust-lang/compiler-team#760. There are two major changes in this PR: 1. The `dist-various-2` container, which already produces the other WASI targets, now has an extra target added for `wasm32-wasip2`. 2. A new `wasm-component-ld` binary is added to all host toolchains when LLD is enabled. This is the linker used for the `wasm32-wasip2` target. This new linker is added for all host toolchains to ensure that all host toolchains can produce the `wasm32-wasip2` target. This is similar to how `rust-lld` was originally included for all host toolchains to be able to produce WebAssembly output when the targets were first added. The new linker is developed [here][wasm-component-ld] and is pulled in via a crates.io-based dependency to the tree here. [wasm-component-ld]: https://github.com/bytecodealliance/wasm-component-ld
This commit updates the support for the `wasm-component-ld` tool from rust-lang#126967 to conditionally build it rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means: * the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform. * a `"wasm-component-ld"` entry in the `tools` section of `config.toml`. Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool.
This is an accidental omission of mine from rust-lang#126967 which means that `rustup target add wasm32-wasip2` isn't working on today's nightlies.
This commit updates the support for the `wasm-component-ld` tool from rust-lang#126967 to conditionally build it rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means: * the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform. * a `"wasm-component-ld"` entry in the `tools` section of `config.toml`. Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool.
This is an accidental omission of mine from rust-lang#126967 which means that `rustup target add wasm32-wasip2` isn't working on today's nightlies.
…t-ld-by-default, r=onur-ozkan Conditionally build `wasm-component-ld` This commit updates the support for the `wasm-component-ld` tool from rust-lang#126967 to conditionally build rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means: * the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform. * a `"wasm-component-ld"` entry in the `tools` section of `config.toml`. Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool.
…t-ld-by-default, r=onur-ozkan Conditionally build `wasm-component-ld` This commit updates the support for the `wasm-component-ld` tool from rust-lang#126967 to conditionally build rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means: * the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform. * a `"wasm-component-ld"` entry in the `tools` section of `config.toml`. Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool.
Rollup merge of rust-lang#127866 - alexcrichton:disable-wasm-component-ld-by-default, r=onur-ozkan Conditionally build `wasm-component-ld` This commit updates the support for the `wasm-component-ld` tool from rust-lang#126967 to conditionally build rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means: * the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform. * a `"wasm-component-ld"` entry in the `tools` section of `config.toml`. Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool.
…dist-manifest, r=Mark-Simulacrum Add `wasm32-wasip2` to `build-manifest` tool This is an accidental omission of mine from rust-lang#126967 which means that `rustup target add wasm32-wasip2` isn't working on today's nightlies.
Rollup merge of rust-lang#127867 - alexcrichton:add-wasm32-wasip2-to-dist-manifest, r=Mark-Simulacrum Add `wasm32-wasip2` to `build-manifest` tool This is an accidental omission of mine from rust-lang#126967 which means that `rustup target add wasm32-wasip2` isn't working on today's nightlies.
This is another accidental omission from rust-lang#126967 (in addition to rust-lang#127867) which fixes an issue where `wasm-component-ld` isn't distributed via rustup just yet because while it's present in the sysroot it's not present in the tarballs.
…t-ld-for-real-this-time-maybe-let-see-after-this-merges, r=onur-ozkan Fix inclusion of `wasm-component-ld` in dist artifacts This is another accidental omission from rust-lang#126967 (in addition to rust-lang#127867) which fixes an issue where `wasm-component-ld` isn't distributed via rustup just yet because while it's present in the sysroot it's not present in the tarballs.
Rollup merge of rust-lang#128060 - alexcrichton:include-wasm-component-ld-for-real-this-time-maybe-let-see-after-this-merges, r=onur-ozkan Fix inclusion of `wasm-component-ld` in dist artifacts This is another accidental omission from rust-lang#126967 (in addition to rust-lang#127867) which fixes an issue where `wasm-component-ld` isn't distributed via rustup just yet because while it's present in the sysroot it's not present in the tarballs.
It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * rust-lang#126967 - this made it into Rust 1.81 * rust-lang#127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * rust-lang#126967 - this made it into Rust 1.81 * rust-lang#127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
…release-notes, r=pietroalbini Remove wasm32-wasip2's tier 2 status from release notes It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * rust-lang#126967 - this made it into Rust 1.81 * rust-lang#127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
…release-notes, r=pietroalbini Remove wasm32-wasip2's tier 2 status from release notes It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * rust-lang#126967 - this made it into Rust 1.81 * rust-lang#127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
…release-notes, r=pietroalbini Remove wasm32-wasip2's tier 2 status from release notes It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * rust-lang#126967 - this made it into Rust 1.81 * rust-lang#127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
# `wasm-component-ld` | ||
|
||
This wrapper is a wrapper around the [`wasm-component-ld`] crates.io crate. That | ||
crate. That crate is itself a thin wrapper around two pieces: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an incomplete sentence here (and still seems to be there on master).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh oops, I'll work on fixing this.
EDIT: #130034
Rollup merge of rust-lang#129995 - alexcrichton:remove-wasm32-wasip2-release-notes, r=pietroalbini Remove wasm32-wasip2's tier 2 status from release notes It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * rust-lang#126967 - this made it into Rust 1.81 * rust-lang#127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
…nt-ld-comments, r=onur-ozkan Fix enabling wasm-component-ld to match other tools It was [pointed out recently][comment] that enabling `wasm-component-ld` as a host tool is different from other host tools. This commit refactors the logic to match by deduplicating selection of when to build other tools and then using the same logic for `wasm-component-ld`. While here I also fixed a typo pointed out in rust-lang#126967 (review) [comment]: rust-lang#127866 (comment)
…nt-ld-comments, r=onur-ozkan Fix enabling wasm-component-ld to match other tools It was [pointed out recently][comment] that enabling `wasm-component-ld` as a host tool is different from other host tools. This commit refactors the logic to match by deduplicating selection of when to build other tools and then using the same logic for `wasm-component-ld`. While here I also fixed a typo pointed out in rust-lang#126967 (review) [comment]: rust-lang#127866 (comment)
…nt-ld-comments, r=onur-ozkan Fix enabling wasm-component-ld to match other tools It was [pointed out recently][comment] that enabling `wasm-component-ld` as a host tool is different from other host tools. This commit refactors the logic to match by deduplicating selection of when to build other tools and then using the same logic for `wasm-component-ld`. While here I also fixed a typo pointed out in rust-lang#126967 (review) [comment]: rust-lang#127866 (comment)
Rollup merge of rust-lang#130034 - alexcrichton:fix-some-wasm-component-ld-comments, r=onur-ozkan Fix enabling wasm-component-ld to match other tools It was [pointed out recently][comment] that enabling `wasm-component-ld` as a host tool is different from other host tools. This commit refactors the logic to match by deduplicating selection of when to build other tools and then using the same logic for `wasm-component-ld`. While here I also fixed a typo pointed out in rust-lang#126967 (review) [comment]: rust-lang#127866 (comment)
…ments, r=onur-ozkan Fix enabling wasm-component-ld to match other tools It was [pointed out recently][comment] that enabling `wasm-component-ld` as a host tool is different from other host tools. This commit refactors the logic to match by deduplicating selection of when to build other tools and then using the same logic for `wasm-component-ld`. While here I also fixed a typo pointed out in rust-lang/rust#126967 (review) [comment]: rust-lang/rust#127866 (comment)
This commit promotes the
wasm32-wasip2
Rust target to tier 2 as proposed in rust-lang/compiler-team#760. There are two major changes in this PR:dist-various-2
container, which already produces the other WASI targets, now has an extra target added forwasm32-wasip2
.wasm-component-ld
binary is added to all host toolchains when LLD is enabled. This is the linker used for thewasm32-wasip2
target.This new linker is added for all host toolchains to ensure that all host toolchains can produce the
wasm32-wasip2
target. This is similar to howrust-lld
was originally included for all host toolchains to be able to produce WebAssembly output when the targets were first added. The new linker is developed here and is pulled in via a crates.io-based dependency to the tree here.