Skip to content
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 19 commits into from
Oct 31, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use transitions for web assembly.
  • Loading branch information
johnedmonds committed Jul 22, 2019
commit e02433516958eba44454288ec9a3c0d05e94a078
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ This repository provides rules for building [Rust][rust] projects with [Bazel](h

#### WebAssembly

All `rust_binary` rules also produce an optional `.wasm` file which can be requested like `@examples//hello_world_wasm:hello_world_wasm.wasm`.
To build a `rust_binary` for wasm32-unknown-unknown add the `--platforms=//rust/platform:wasm` flag.
Copy link
Contributor

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:

cc_library(
    name = "runtime",
    srcs = "runtime.cc",
    data = [
        ":wasm",
    ],
)

rust_library(
    name = "wasm",
    srcs = ["wasm.rs"],
    edition = "2018",
    crate_type = "cdylib",
    rustc_flags = ["--target=wasm32-unknown-unknown"],
)

where Wasm module compiled using rust_library must be loaded by the runtime 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 like rust_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?

Copy link
Contributor Author

@johnedmonds johnedmonds Oct 14, 2019

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 building proc-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 dependent rust_library (and transitively for all its dependencies). So I think what you want to do is this:

cc_library(
    name = "runtime",
    srcs = "runtime.cc",
    data = [
        ":wasm_bindgen_bg.wasm",
    ],
)

rust_wasm_bindgen(
    name = "wasm_bindgen",
    wasm_file = ":wasm"
)

rust_library(
    name = "wasm",
    srcs = ["wasm.rs"],
    edition = "2018",
)

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.

Copy link
Contributor

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 to wasm-bindgen, I want pure Rust code that's compiled to wasm32-unknown-unknown.

As such, I'm suggesting that we would add rust_wasm_library that uses the transitions that you've already added for rust_wasm_bindgen, i.e.

cc_library(
    name = "runtime",
    srcs = "runtime.cc",
    data = [
        ":wasm",
    ],
)

rust_wasm_library(
    name = "wasm",
    deps = ":library"
)

rust_library(
    name = "library",
    srcs = ["library.rs"],
    edition = "2018",
)

Alternatively, since you're already doing it for proc-macro, I think that we could use crate_type = "wasm32" and simply transition to wasm32-unknown-unknown in the rust_library, i.e.

cc_library(
    name = "runtime",
    srcs = "runtime.cc",
    data = [
        ":library",
    ],
)

rust_library(
    name = "library",
    srcs = ["library.rs"],
    crate_type = "wasm32",
    edition = "2018",
)

What do you think?

Copy link
Contributor

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.

Copy link
Contributor Author

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:

  1. The method you proposed of introducing a rule to force a transition
  2. Adding a setting to the rust_library to enable the transition
  3. Change Bazel to make it easier to do transitions on demand.
  4. Existing workaround via rust_wasm_bindgen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@PiotrSikora PiotrSikora Oct 30, 2019

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 with rust_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 with rust_proto_library and rust_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?


bazel build @examples//hello_world_wasm --platforms=//rust/platform:wasm

`rust_wasm_bindgen` will automatically transition to the wasm platform and can be used when
building wasm code for the host target.

### Protobuf
<div class="toc">
Expand Down
2 changes: 1 addition & 1 deletion examples/hello_world_wasm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ rust_binary(

rust_wasm_bindgen(
name = "hello_world_wasm_bindgen",
wasm_file = ":hello_world_wasm.wasm"
wasm_file = ":hello_world_wasm"
)

jasmine_node_test(
Expand Down
11 changes: 0 additions & 11 deletions proto/proto.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -146,33 +146,24 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, grpc,

# And simulate rust_library behavior
output_hash = repr(hash(lib_rs.path))
wasm_output_hash = output_hash + ".wasm"
rust_lib = ctx.actions.declare_file("%s/lib%s-%s.rlib" % (
output_dir,
crate_name,
output_hash,
))
rust_wasm_lib = ctx.actions.declare_file("%s/lib%s-%s.rlib" % (
output_dir,
crate_name,
wasm_output_hash,
))
result = rustc_compile_action(
ctx = ctx,
toolchain = find_toolchain(ctx),
wasm_toolchain = ctx.toolchains["@io_bazel_rules_rust//rust:wasm_toolchain"],
crate_info = CrateInfo(
name = crate_name,
type = "rlib",
root = lib_rs,
srcs = srcs,
deps = compile_deps,
output = rust_lib,
wasm_output = rust_wasm_lib,
edition = proto_toolchain.edition,
),
output_hash = output_hash,
wasm_output_hash = wasm_output_hash,
)
return result

Expand Down Expand Up @@ -232,7 +223,6 @@ rust_proto_library = rule(
toolchains = [
"@io_bazel_rules_rust//proto:toolchain",
"@io_bazel_rules_rust//rust:toolchain",
"@io_bazel_rules_rust//rust:wasm_toolchain",
"@bazel_tools//tools/cpp:toolchain_type",
],
doc = """
Expand Down Expand Up @@ -293,7 +283,6 @@ rust_grpc_library = rule(
toolchains = [
"@io_bazel_rules_rust//proto:toolchain",
"@io_bazel_rules_rust//rust:toolchain",
"@io_bazel_rules_rust//rust:wasm_toolchain",
"@bazel_tools//tools/cpp:toolchain_type",
],
doc = """
Expand Down
4 changes: 0 additions & 4 deletions rust/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ toolchain_type(
name = "toolchain",
)

toolchain_type(
name = "wasm_toolchain",
)

bzl_library(
name = "rules",
srcs = glob(["**/*.bzl"]),
Expand Down
8 changes: 8 additions & 0 deletions rust/platform/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,11 @@ package(default_visibility = ["//visibility:public"])
load(":platform.bzl", "declare_config_settings")

declare_config_settings()

package_group(
name = "function_transition_whitelist",
packages = [
"//...",
],
)

14 changes: 4 additions & 10 deletions rust/platform/platform.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,14 @@ def declare_config_settings():
constraint_values = triple_to_constraint_set(triple),
)

native.constraint_setting(name = "extra_target_platform_cpu")
native.constraint_setting(name = "extra_target_platform_system")
native.constraint_value(
name = "constraint_cpu_wasm32",
constraint_setting = ":extra_target_platform_cpu"
)
native.constraint_value(
name = "constraint_system_unknown",
constraint_setting = ":extra_target_platform_system"
name = "wasm32",
constraint_setting = "@platforms//cpu"
)

native.platform(
name = "wasm",
constraint_values = [
"@io_bazel_rules_rust//rust/platform:constraint_cpu_wasm32",
"@io_bazel_rules_rust//rust/platform:constraint_system_unknown",
"@io_bazel_rules_rust//rust/platform:wasm32",
]
)
5 changes: 3 additions & 2 deletions rust/platform/triple_mappings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ _CPU_ARCH_TO_BUILTIN_PLAT_SUFFIX = {
"i686": "x86_32",
"s390x": "s390x",
"asmjs": None,
"wasm32": None,
"i386": None,
"i586": None,
"powerpc64": None,
Expand All @@ -29,7 +28,6 @@ _SYSTEM_TO_BUILTIN_SYS_SUFFIX = {
"ios": "ios",
"android": "android",
"emscripten": None,
"unknown": None,
"nacl": None,
"bitrig": None,
"dragonfly": None,
Expand Down Expand Up @@ -125,6 +123,9 @@ def triple_to_constraint_set(triple):
if len(component_parts) == 4:
abi = component_parts[3]

if cpu_arch == "wasm32":
return ["@io_bazel_rules_rust//rust/platform:wasm32"]

constraint_set = []
constraint_set += cpu_arch_to_constraints(cpu_arch)
constraint_set += vendor_to_constraints(vendor)
Expand Down
13 changes: 13 additions & 0 deletions rust/private/dummy_cc_toolchain/BUILD
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(jedmonds@spotify.com): 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"]
)

8 changes: 8 additions & 0 deletions rust/private/dummy_cc_toolchain/dummy_cc_toolchain.bzl
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 = {},
)

42 changes: 11 additions & 31 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

load("@io_bazel_rules_rust//rust:private/rustc.bzl", "CrateInfo", "rustc_compile_action")
load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain")
load("@io_bazel_rules_rust//rust:private/transitions.bzl", "wasm_transition")

_OLD_INLINE_TEST_CRATE_MSG = """
--------------------------------------------------------------------------------
Expand Down Expand Up @@ -98,7 +99,6 @@ def _rust_library_impl(ctx):
lib_rs = _crate_root_src(ctx)

toolchain = find_toolchain(ctx)
wasm_toolchain = ctx.toolchains["@io_bazel_rules_rust//rust:wasm_toolchain"]

# Determine unique hash for this rlib
output_hash = _determine_output_hash(lib_rs)
Expand All @@ -111,56 +111,39 @@ def _rust_library_impl(ctx):
)
rust_lib = ctx.actions.declare_file(rust_lib_name)

# proc-macro crates must be built with "host" configuration because they need to be run.
# Therefore we reuse the proc macro crate built for the host configuration.
if ctx.attr.crate_type != "proc-macro":
wasm_output_hash = output_hash + ".wasm"

rust_wasm_lib_name = _determine_lib_name(
ctx.attr.name,
ctx.attr.crate_type,
toolchain,
wasm_output_hash,
)
rust_wasm_lib = ctx.actions.declare_file(rust_wasm_lib_name)
else:
wasm_output_hash = output_hash
rust_wasm_lib = rust_lib

return rustc_compile_action(
ctx = ctx,
toolchain = toolchain,
wasm_toolchain = wasm_toolchain,
crate_info = CrateInfo(
name = ctx.label.name,
type = ctx.attr.crate_type,
root = lib_rs,
srcs = ctx.files.srcs,
deps = ctx.attr.deps,
output = rust_lib,
wasm_output = rust_wasm_lib,
edition = _get_edition(ctx, toolchain),
),
output_hash = output_hash,
wasm_output_hash = wasm_output_hash,
)

def _rust_binary_impl(ctx):
toolchain = find_toolchain(ctx)
wasm_toolchain = ctx.toolchains["@io_bazel_rules_rust//rust:wasm_toolchain"]

if (toolchain.target_arch == "wasm32"):
output = ctx.actions.declare_file(ctx.label.name + ".wasm")
else:
output = ctx.actions.declare_file(ctx.label.name)

return rustc_compile_action(
ctx = ctx,
toolchain = toolchain,
wasm_toolchain = wasm_toolchain,
crate_info = CrateInfo(
name = ctx.label.name,
type = "bin",
root = _crate_root_src(ctx, "main.rs"),
srcs = ctx.files.srcs,
deps = ctx.attr.deps,
output = ctx.actions.declare_file(ctx.label.name),
wasm_output = ctx.actions.declare_file(ctx.label.name + ".wasm"),
output = output,
edition = _get_edition(ctx, toolchain),
),
)
Expand All @@ -186,7 +169,6 @@ def _rust_test_common(ctx, test_binary):
srcs = crate.srcs + ctx.files.srcs,
deps = crate.deps + ctx.attr.deps,
output = test_binary,
wasm_output = None,
edition = crate.edition,
)
elif len(ctx.attr.deps) == 1 and len(ctx.files.srcs) == 0:
Expand All @@ -211,7 +193,6 @@ def _rust_test_common(ctx, test_binary):
return rustc_compile_action(
ctx = ctx,
toolchain = toolchain,
wasm_toolchain = None,
crate_info = target,
rust_flags = ["--test"],
)
Expand Down Expand Up @@ -339,6 +320,9 @@ _rust_library_attrs = {
"""),
default = "rlib",
),
"_whitelist_function_transition": attr.label(
default = "//tools/whitelists/function_transition_whitelist",
),
}

_rust_test_attrs = {
Expand All @@ -359,9 +343,9 @@ rust_library = rule(
_rust_library_attrs.items()),
fragments = ["cpp"],
host_fragments = ["cpp"],
cfg = wasm_transition,
toolchains = [
"@io_bazel_rules_rust//rust:toolchain",
"@io_bazel_rules_rust//rust:wasm_toolchain",
"@bazel_tools//tools/cpp:toolchain_type"
],
doc = """
Expand Down Expand Up @@ -448,12 +432,8 @@ rust_binary = rule(
host_fragments = ["cpp"],
toolchains = [
"@io_bazel_rules_rust//rust:toolchain",
"@io_bazel_rules_rust//rust:wasm_toolchain",
"@bazel_tools//tools/cpp:toolchain_type"
],
outputs = {
"wasm": "%{name}.wasm",
},
doc = """
Builds a Rust binary crate.

Expand Down
Loading