add flag to allow putting out binary whenever

[Shikugawa]

We had an opportunity to put out binary without specifying crate_type as bin.
In this case, we should add configurable crate_type and a flag to put out binary whenever.
With this patch, we can tackle this situation.

[damienmg]

Hi,

Thanks for contributing. Could you expand a bit more on the purpose of this PR? Maybe by giving an example?

Also the build is failing, you should be able to reproduce with bazel build @examples//....

[Shikugawa]

@damienmg Thanks. This functionality is needed on this PR.
envoyproxy/envoy-wasm#488

I'd like to build Rust code like this into wasm32-unknown-unknown.
https://github.com/Shikugawa/envoy-wasm-rust-playground/blob/master/example/src/lib.rs

This code is build with this build config.
https://github.com/Shikugawa/envoy-wasm-rust-playground/blob/master/example/Cargo.toml

We want to put out binary but not want to build with --cargo-type=lib but --cargo-type=cdylib. In the current implementation, we can't put out binary without using --cargo-type=bin. This was a critical problem for us so that patched this.

I'm considering to use this like this. This will put out test_rust.wasm with this command.

bazel build //:test_rust --platforms=@io_bazel_rules_rust//rust/platform:wasm

rust_binary(
    name = "test_rust",
    srcs = ["test_rust.rs"],
    crate_type = "cdylib",
    out_binary = True,
)

[damienmg]

Ok I am not really happy with the interface this PR offer but I cannot think of something better, maybe @mfarrugi or @acmcarther have a better opinion.

[acmcarther]

I don't feel particularly strongly about this. It does seem to solve a preexisting problem, though it seems this might be wasm-specific?

out_binary=true should be implicit for a rust_binary rule. Can we just infer this?

It's also hard to infer the purpose of crate_type on rust_binary if you're not steeped in the wasm context. In fact, I'm not even really sure what this does. Does it emit both a library and a binary?

Reading the relevant Cargo.toml it's not obvious to me that this is supposed to produce a "binary" at all. Does rust_library work for this?

I suspect I'm missing something critical here.

[mfarrugi]

It's also hard to infer the purpose of crate_type on rust_binary if you're not steeped in the wasm context. In fact, I'm not even really sure what this does. Does it emit both a library and a binary?
+1

I have no idea why it having rust_library build the cdylib doesn't work, and I can't guess what cdylib means for wasm. Does executable mean something special for wasm?

[Shikugawa]

@acmcarther @mfarrugi I think that this is not "binary" but "library" because we emit wasm module. But it seems that rust_library doesn't have this functionality, to emit wasm module. So we may have to add this to rust_library. WDYT?

it seems this might be wasm-specific?

I think that this is not so, but I have no idea to use this functionality on a normal binary.

I have no idea why it having rust_library build the cdylib doesn't work, and I can't guess what cdylib means for wasm. Does executable mean something special for wasm?

I also have no idea about this. Maybe cc. @PiotrSikora have some idea.

[PiotrSikora]

See previous discussion: #240 (comment)

rules_rust already set the precedent and has rust_{proto,grpc,wasm_bindgen}_library targets, so I think that we should add dedicated rust_wasm_library (binary vs library doesn't matter for Wasm, AFAIK).

[damienmg]

@mfarrugi @acmcarther was there a consensus on moving forward with that change?

[mfarrugi]

I can't really opine, don't know anything about wasm. To me this stuff looks a little grody, but it's not a lot of code.

[Shikugawa]

We may ought to implement rust_wasm_binary which spec has to put out code anytime. But It can be good a bit to rename out_binary to wasm_mode, which put out wasm module anytime if we set this True. out_binary may cause confusion.

[damienmg]

Sorry for taking so long to come back to this one. I don't see strong opinion either way. rust_wasm_binary is a good idea but in between I don't see this PR as harmful so LGTM.