Build rust wasm data with Bazel #516
Conversation
Signed-off-by: shikugawa <[email protected]>
…o rust-bazel Signed-off-by: shikugawa <[email protected]>
Signed-off-by: shikugawa <[email protected]>
|
Looks good. Just need to fix the formatting. |
Signed-off-by: shikugawa <[email protected]>
Signed-off-by: shikugawa <[email protected]>
Signed-off-by: shikugawa <[email protected]>
Signed-off-by: shikugawa <[email protected]>
Signed-off-by: shikugawa <[email protected]>
Signed-off-by: shikugawa <[email protected]>
…o rust-bazel Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
…t-bazel Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
| @@ -6,44 +6,45 @@ use proxy_wasm; | |||
| #[no_mangle] | |||
| pub fn proxy_abi_version_0_1_0() {} | |||
|
|
|||
| use log::{debug, error, info, trace, warn}; | |||
| use log::*; | |||
There was a problem hiding this comment.
Maybe, log crate has private helper macros. In Rust 2018, we can available external macros with use syntax. But it works only there is no private helper macros. We should import them when we use log macros. But it will cause more complexity and confusion. In this case, we don't have any problem to use *. https://doc.rust-lang.org/edition-guide/rust-2018/macros/macro-changes.html#local-helper-macros
| srcs = glob(["*.wasm"]), | ||
| rust_library( | ||
| name = "proxy_wasm", | ||
| srcs = ["proxy_wasm.rs"], |
There was a problem hiding this comment.
I don't have a better place to comment on this, but this proxy_wasm.rs is NOT Proxy-Wasm.
You can copy the contents of that file into logging_rust.rs, since it implements the ABI directly... or at the very least use a different name to avoid confusion.
Ideally, we should rewrite this test to use proxy-wasm-rust-sdk, but that's probably too big change, and should be done in a separate PR.
There was a problem hiding this comment.
I will add TODO here and fix when proxy-wasm-rust-sdk bazel support is merged.
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
test/extensions/common/wasm/BUILD
Outdated
| # "skip_on_windows", | ||
| # # Precompiled WASM code which is required to compile with wee8_compiler | ||
| # # which is needed to build as position independent code. It is not supported on macos. | ||
| # "skip_on_macos", |
There was a problem hiding this comment.
- I don't think you should disable this test.
wee8_compilebuilds fine on macOS (at least it worked last time I checked).- Precompilation is only enabled on Linux-x86_64. This is guarded in
wee8_compiletool and in the V8 code loading the.wasmmodule, so this test should pass on macOS just fine.
There was a problem hiding this comment.
Also, you could just skip the precompile step on macOS.
| INSTANTIATE_TEST_SUITE_P(RuntimesAndLanguages, WasmTestMatrix, | ||
| testing::Combine(testing::Values("v8" | ||
| #if defined(ENVOY_WASM_WAVM) | ||
| , | ||
| "wavm" | ||
| #endif | ||
| ), | ||
| testing::Values("cpp", "rust"))); | ||
| testing::Values("cpp"))); |
There was a problem hiding this comment.
If this PR disables wasm_vm_test and removes rust from wasm_test, then there are no Rust tests executed at all...
There was a problem hiding this comment.
wasm_test is temporary disabled for my debugging of OOM killed in CI.
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
|
@PiotrSikora I fixed all build stuff, ptal. |
There was a problem hiding this comment.
It looks that this PR builds and precompiles the self-contained test/extensions/common/wasm/test_data/test_rust.wasm, but not the recently added tests written using Rust SDK.
I think it's fine to merge this now, and add the migration of the new tests to Bazel in a subsequent PR using cargo raze, but right now this PR adds some unnecessary files and breaks building the new tests, which should be fixed before merging.
test/extensions/filters/http/wasm/test_data/body_rust/Cargo.toml
Outdated
Show resolved
Hide resolved
Signed-off-by: Lizan Zhou <[email protected]>
|
@PiotrSikora I reverted those unnecessary changes |
Signed-off-by: Lizan Zhou <[email protected]>
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Description: Build rust wasm data with bazel. Note that we can't execute this with RBE, and we use private fork of rules_rust because we can't overcome the build problem (see bazelbuild/rules_rust#312) with current version of it.
Risk Level: Low
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]