-
Notifications
You must be signed in to change notification settings - Fork 435
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
Add support for wasm-bindgen. #240
Merged
Merged
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
6c97bb9
Add support for WebAssembly.
johnedmonds 56ff697
Add a comment explaining that the npm deps are for the wasm example.
johnedmonds e024335
Use transitions for web assembly.
johnedmonds 75a2af8
Merge branch 'master' into wasm
johnedmonds f13e29a
Merge branch 'master' into wasm
johnedmonds 0a65de2
Merge remote-tracking branch 'origin/master' into wasm
johnedmonds e68fb04
Upgrade WASM bindgen and make compatible with Typescript.
johnedmonds 4f9759a
Remove unnecessary setting.
johnedmonds 9b0fb32
Re-run cargo-raze.
johnedmonds ef0bf46
Add nodejs rules for the examples workspace.
johnedmonds 9e0be3c
Merge branch 'master' into wasm
johnedmonds b71caa8
Merge branch 'master' into wasm
johnedmonds 2fe4c61
Fix link_env missing.
johnedmonds 9a5295c
Bazel 1.0 compatibility.
johnedmonds b3af5e7
Exclude matrix_dylib_test on RBE due to https://github.com/bazelbuild…
johnedmonds 7305a12
Ignore the wasm test because rust-lld isn't available on RBE.
johnedmonds 7bd76e9
Use extra target triples.
johnedmonds 72c518c
Remove extra newline.
johnedmonds 742c0ab
Merge branch 'master' into wasm
johnedmonds File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
# The email address is not required for organizations. | ||
|
||
Google Inc. | ||
Spotify AB | ||
Damien Martin-Guillerez <[email protected]> | ||
David Chen <[email protected]> | ||
Florian Weikert <[email protected]> | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,3 +19,4 @@ Kristina Chodorow <[email protected]> | |
Philipp Wollermann <[email protected]> | ||
Ulf Adams <[email protected]> | ||
Justine Alexandra Roberts Tunney <[email protected]> | ||
John Edmonds <[email protected]> |
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package(default_visibility = ["//visibility:public"]) | ||
|
||
load( | ||
"@io_bazel_rules_rust//rust:rust.bzl", | ||
"rust_binary", | ||
) | ||
|
||
load( | ||
"@io_bazel_rules_rust//wasm_bindgen:wasm_bindgen.bzl", | ||
"rust_wasm_bindgen", | ||
) | ||
|
||
load( | ||
"@build_bazel_rules_nodejs//:defs.bzl", | ||
"jasmine_node_test", | ||
) | ||
|
||
rust_binary( | ||
name = "hello_world_wasm", | ||
srcs = ["main.rs"], | ||
edition = "2018", | ||
deps = ["@io_bazel_rules_rust//wasm_bindgen/raze:wasm_bindgen"], | ||
) | ||
|
||
rust_wasm_bindgen( | ||
name = "hello_world_wasm_bindgen", | ||
wasm_file = ":hello_world_wasm" | ||
) | ||
|
||
jasmine_node_test( | ||
name = "hello_world_wasm_test", | ||
srcs = [ | ||
"hello_world_wasm_test.js", | ||
], | ||
data = [ | ||
":hello_world_wasm_bindgen_bg.wasm", | ||
"@npm//jasmine", | ||
], | ||
) |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
const fs = require('fs'); | ||
const path = require('path'); | ||
|
||
describe('Calling WebAssembly code', () => { | ||
it('Should run WebAssembly code', async () => { | ||
const buf = fs.readFileSync(path.join(__dirname, 'hello_world_wasm_bindgen_bg.wasm')); | ||
const res = await WebAssembly.instantiate(buf); | ||
expect(res.instance.exports.hello_world(2)).toEqual(4); | ||
}) | ||
}); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
use wasm_bindgen::prelude::*; | ||
|
||
#[wasm_bindgen] | ||
pub fn hello_world(i: i32) -> i32 { | ||
i * 2 | ||
} | ||
|
||
fn main() { | ||
println!("Hello {}", hello_world(2)); | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"private": true, | ||
"devDependencies": { | ||
"jasmine": "3.4.0" | ||
}, | ||
"scripts": {} | ||
} |
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
load(":dummy_cc_toolchain.bzl", "dummy_cc_toolchain") | ||
dummy_cc_toolchain(name = "dummy_cc_wasm32") | ||
|
||
# When compiling Rust code for wasm32, we avoid linking to cpp code so we introduce a dummy cc | ||
# toolchain since we know we'll never look it up. | ||
# TODO([email protected]): Need to support linking C code to rust code when compiling for wasm32. | ||
toolchain( | ||
name = "dummy_cc_wasm32_toolchain", | ||
toolchain = ":dummy_cc_wasm32", | ||
toolchain_type = "@bazel_tools//tools/cpp:toolchain_type", | ||
target_compatible_with = ["//rust/platform:wasm32"] | ||
) | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
def _dummy_cc_toolchain_impl(ctx): | ||
return [platform_common.ToolchainInfo()] | ||
|
||
dummy_cc_toolchain = rule( | ||
implementation = _dummy_cc_toolchain_impl, | ||
attrs = {}, | ||
) | ||
|
Oops, something went wrong.
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.
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.
This is a great start, but I think that the requirement to set target platform is quite limiting.
WebAssembly is used outside of browsers / JavaScript host environments, and Wasm modules can be loaded by other build targets in the same Bazel workspace, and those build targets must be built using "native" platforms and not the
//rust:platform:wasm
pseudo-platform.For example, imagine such
BUILD
file:where Wasm module compiled using
rust_library
must be loaded by theruntime
written in C/C++.The ability to override
--target
is something that I have hacked in my local tree, but ideally we would have something likerust_wasm_library
that uses--crate-type=cdylib --target=wasm32-unknown-unknown
to compile Wasm modules.Basically, we need the ability to indicate Wasm target on a per build target basis, and not globally.
What do you think?
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.
@PiotrSikora Thanks for the comment! I think I had a similar problem where I wanted to include the compiled wasm (compiled as wasm32-unknown-unknown) with
include_bytes
and serve it from the binary (compiled as a native Rust binary). We allow it in this PR by using Bazel transitions. See https://github.com/bazelbuild/rules_rust/pull/240/files#diff-9a42a2c125ed40c511198be1665d0faeR346. This allows us to build the entire build with the default target platform (e.g. x86 + Linux) but temporarily transition to//rust:platform:wasm
when building the wasm code. We also handle transitioning to the host platform when buildingproc-macro
crates.The easiest way is to do this in practice is to make a
rust_wasm_bindgen
target which will force the transition for the dependentrust_library
(and transitively for all its dependencies). So I think what you want to do is this:With this you should be able to build
:runtime
as a native library and:wasm
will automatically get built as//rust:platform:wasm
.I have a note about this here https://github.com/bazelbuild/rules_rust/pull/240/files/7305a12fd276b0c21432d00d4e2ddd8b15115e13#diff-04c6e90faac2675aa89e2176d2eec7d8R31. As I read my note again I realize that perhaps it's a bit unclear since "transition" is a Bazel-specific word. Do you think it would be clearer if I referenced it in this paragraph? Or did I misunderstand your usecase?
Basically in this paragraph I was trying to explain how you could get the .wasm file if that's the only thing you wanted.
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, the transitions are great, I didn't realize that you could do it this way.
I think that
rust_wasm_bindgen
is close to what I want, but after reading that paragraph, I simply ignored it, because I don't want anything related towasm-bindgen
, I want pure Rust code that's compiled towasm32-unknown-unknown
.As such, I'm suggesting that we would add
rust_wasm_library
that uses the transitions that you've already added forrust_wasm_bindgen
, i.e.Alternatively, since you're already doing it for
proc-macro
, I think that we could usecrate_type = "wasm32"
and simply transition towasm32-unknown-unknown
in therust_library
, i.e.What do you think?
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.
Friendly ping before this gets merged.
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.
Do you think we should do it in a different PR? There are several ways to do this like:
rust_library
to enable the transitionrust_wasm_bindgen
.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.
@PiotrSikora
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.
I'm definitely against (4), since this feature has nothing to do with wasm-bindgen.
(3) would still require action from the person building the target (vs person writing
BUILD
file), so it's not much different than requiring extra--platforms=wasm32
flag, IMHO.I'm fine with either (1) or (2), with a slight preference for (1), since then you could easily add a target that extends existing
rust_library
to be also compiled as Wasm module (i.e. you could have both native and Wasm build targets built at the same time) by simply adding few lines withrust_wasm_library
(see the example above). You could achieve the same with (2), but then you'd need to duplicate the complete build target, so it would be more error prone, I think.Also, there is already a precedence for
rust_xxx_library
withrust_proto_library
andrust_grpc_library
.It's fine if you want to add it in a separate PR (i.e. I don't want to block this PR from getting merged), but could you rename it to "Add support for wasm-bindgen", since it's not a complete WebAssembly support?
What do you think?