Skip to content

Re-add support for building Wasm libraries as executables. #626

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

Merged
merged 3 commits into from
Mar 15, 2021

Conversation

PiotrSikora
Copy link
Contributor

The ability to build Wasm libraries as executables is needed to support
WASI reactors (Wasm executables with multiple entrypoints).

This is a temporary workaround, and we should be able to use crate-type
"bin" when a proper support for WASI reactors (rust-lang/rust#79997) is
stabilised is Rust.

This feature was added in #312, and most recently broken in #592.

Signed-off-by: Piotr Sikora [email protected]

The ability to build Wasm libraries as executables is needed to support
WASI reactors (Wasm executables with multiple entrypoints).

This is a temporary workaround, and we should be able to use crate-type
"bin" when a proper support for WASI reactors (rust-lang/rust#79997) is
stabilised is Rust.

This feature was added in bazelbuild#312, and most recently broken in bazelbuild#592.

Signed-off-by: Piotr Sikora <[email protected]>
@google-cla google-cla bot added the cla: yes label Mar 6, 2021
Copy link
Contributor Author

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

@hlopko PTAL

Signed-off-by: Piotr Sikora <[email protected]>
@hlopko hlopko self-requested a review March 9, 2021 07:42
@hlopko
Copy link
Member

hlopko commented Mar 9, 2021

I'm sorry for breaking this /o. If this is urgent I'm happy to approve the PR, but I'd like to continue discussing the options.

So if I understand things you want to use rust_binary rule for reactors, but pass --crate-type=lib? I deduce that from rust-lang/rust#79997 (comment). But then from #312 it seems you want to use --crate-type=cdylib. Is it an option to use rust_shared_library instead of rust_binary?

What exactly is going to change after this feature is stabilized in rustc? Won't we still need to have some way of telling the rules that they are building the reactor and that they'll have to set the DefaultInfo.executable?

@PiotrSikora
Copy link
Contributor Author

I'm sorry for breaking this /o. If this is urgent I'm happy to approve the PR, but I'd like to continue discussing the options.

Not urgent, but it would be great if it worked, i.e. a few days is fine, a few months is not.

So if I understand things you want to use rust_binary rule for reactors, but pass --crate-type=lib? I deduce that from rust-lang/rust#79997 (comment).

You can build WASI reactors using --crate-type=bin --target=wasm32-wasi and RUSTFLAGS="-Z wasi-exec-model=reactor", but that's an unstable nightly feature, so for now we create a "fake" WASI reactor by building a Wasm library with _initialize function.

But then from #312 it seems you want to use --crate-type=cdylib. Is it an option to use rust_shared_library instead of rust_binary?

rust_shared_library doesn't work for .wasm files (i.e. when using --platforms=@rules_rust//rust/platform:wasi):

Traceback (most recent call last):
        File ".../rules_rust/rust/private/rust.bzl", line 179, column 32, in _rust_shared_library_impl
                return _rust_library_common(ctx, "cdylib")
        File ".../rules_rust/rust/private/rust.bzl", line 221, column 32, in _rust_library_common
                return rustc_compile_action(
        File ".../rules_rust/rust/private/rustc.bzl", line 614, column 29, in rustc_compile_action
                return establish_cc_info(ctx, crate_info, toolchain, cc_toolchain, feature_configuration) + [
        File ".../rules_rust/rust/private/rustc.bzl", line 671, column 59, in establish_cc_info
                library_to_link = cc_common.create_library_to_link(
Error in create_library_to_link: 'libhello_world-665202032.wasm' does not have any of the allowed extensions .so, .dylib or .dll

You can use the repro steps from the previous time it was broken (#386), and adjust accordingly.

But I think it would be better to restore the previously working behavior, otherwise users need to switch from existing:

rust_binary(
    name = "foo",
    crate_type="cdylib",
    out_binary=True,
)

to:

rust_shared_library(
    name = "foo",
)

only to switch back again to rust_binary, once the WASI reactors are stabilised:

rust_binary(
    name = "foo",
    rustc_flags = "-Z wasi-exec-model=reactor",
) 

What exactly is going to change after this feature is stabilized in rustc? Won't we still need to have some way of telling the rules that they are building the reactor and that they'll have to set the DefaultInfo.executable?

I think this would work just fine (WASI reactors use --crate-type=bin):

rust_binary(
    name = "foo",
    rustc_flags = "-Z wasi-exec-model=reactor",
) 

Note, that ideally we would have dedicated rust_wasm_{binary,library} targets that automatically transition to a Wasm platform (either @rules_rust//rust/platform:wasm or @rules_rust//rust/platform:wasi), and maybe they would be aware of the WASI exec models (command and reactor).

Right now, we're using "regular" rust_{binary,library} targets and set --platform accordingly to build Wasm files, but then the whole workspace transitions to Wasm, and you cannot build mixed projects this way (e.g. have a native rust_binary that loads .wasm file generated from Wasm rust_binary).

@hlopko
Copy link
Member

hlopko commented Mar 15, 2021

Thank you for the thorough explanation! Yeah let's merge this.

@hlopko hlopko merged commit 76c4420 into bazelbuild:main Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants