envoy icon indicating copy to clipboard operation
envoy copied to clipboard

Wasm coverage disabled due to `rules_rust` update

Open phlax opened this issue 3 years ago • 21 comments

Not sure why CI didnt fail in presubmit, but the recent update to rules_rust has broken ci (#22253 )

Fail is:

# Execution platform: @local_config_platform//:host

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
error[E0463]: can't find crate for `profiler_builtins`
  |
  = note: the compiler may have been built without the profiler runtime

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
INFO: Elapsed time: 181.252s, Critical Path: 45.34s
INFO: 11009 processes: 5405 remote cache hit, 5297 internal, 307 processwrapper-sandbox.
//test/extensions/bootstrap/wasm:wasm_test                      FAILED TO BUILD

https://dev.azure.com/cncf/envoy/_build/results?buildId=121396&view=logs&j=bbe4b42d-86e6-5e9c-8a0b-fea01d818a24&s=4f7d954b-a765-565f-91a2-c04870dab43f&t=e00c5a13-c6dc-5e9a-6104-69976170e881&l=147

phlax avatar Nov 23 '22 08:11 phlax

cc @PiotrSikora

phlax avatar Nov 23 '22 08:11 phlax

possibly related https://github.com/rust-lang/rust/issues/81684

phlax avatar Nov 23 '22 08:11 phlax

seems we may need to add --linkopt=-noprofilelib somewhere

https://github.com/bazelbuild/rules_rust/pull/1324#issuecomment-1120467263

phlax avatar Nov 23 '22 09:11 phlax

test pr here - #24168

phlax avatar Nov 23 '22 10:11 phlax

I am not sure how to disable some specified test cases.

But it wouldn't be hard to add additional linkopts to some test cases.

wbpcode avatar Nov 23 '22 10:11 wbpcode

But it wouldn't be hard to add additional linkopts to some test cases.

im grepping around but not sure where to add - i have tried adding to the wasm binary rule in my test PR but not sure that is correct

phlax avatar Nov 23 '22 10:11 phlax

im grepping around but not sure where to add - i have tried adding to the wasm binary rule in my test PR but not sure that is correct

Hmmm, I just readed the error log. Seems that it's error from the rust runtime. But you added opts to the normal c++ wasm.

wbpcode avatar Nov 23 '22 10:11 wbpcode

Hmmm, I just readed the error log. Seems that it's error from the rust runtime. But you added opts to the normal c++ wasm.

clutching at straws 8/

can you point me to where it should go ?

phlax avatar Nov 23 '22 10:11 phlax

i considered the rust_binary but couldnt see a linkopts there http://bazelbuild.github.io/rules_rust/flatten.html#rust_binary

phlax avatar Nov 23 '22 10:11 phlax

perhaps in test/extensions/common/wasm/test_data/BUILD

phlax avatar Nov 23 '22 10:11 phlax

hmm - not sure about ^^ - they just seem to point to the rules i already hacked (which didnt work incidentally)

phlax avatar Nov 23 '22 10:11 phlax

In fact, I am also searching rust rule for now and find nothing about the linkopts 🤣

wbpcode avatar Nov 23 '22 10:11 wbpcode

rustc_flags perhaps

phlax avatar Nov 23 '22 10:11 phlax

its this that is failing

 13 wasm_rust_binary(
 14     name = "test_rust.wasm",
 15     srcs = ["test_rust.rs"],
 16     rustc_flags = ["-Clink-arg=-zstack-size=32768"],
 17     precompile = envoy_select_wasm_v8_bool(),
 18 )

in test/extensions/common/wasm/test_data/BUILD

altho i guess it would fail elsewhere so if this fixes - i think we want to put it in the wasm.bzl file

phlax avatar Nov 23 '22 10:11 phlax

testing this on test PR

diff --git a/bazel/wasm/wasm.bzl b/bazel/wasm/wasm.bzl
index 9350189d8b..1cb9b68f11 100644
--- a/bazel/wasm/wasm.bzl
+++ b/bazel/wasm/wasm.bzl
@@ -78,7 +78,7 @@ def envoy_wasm_cc_binary(name, additional_linker_inputs = [], linkopts = [], tag
         **kwargs
     )
 
-def wasm_rust_binary(name, tags = [], wasi = False, precompile = False, **kwargs):
+def wasm_rust_binary(name, tags = [], wasi = False, precompile = False, rustc_flags = [], **kwargs):
     wasm_name = "_wasm_" + name.replace(".", "_")
     kwargs.setdefault("visibility", ["//visibility:public"])
 
@@ -87,6 +87,7 @@ def wasm_rust_binary(name, tags = [], wasi = False, precompile = False, **kwargs
         edition = "2018",
         crate_type = "cdylib",
         out_binary = True,
+        rustc_flags = rustc_flags + ["--linkopt=-noprofilelib"],
         tags = ["manual"],
         **kwargs
     )

phlax avatar Nov 23 '22 10:11 phlax

I don't know how to disable the whole test case. But seems that the rust wasm is an optional data input of test case, we can just disable this data input for coverage test to fix the ci first. Maybe we can give it a try?

def envoy_select_wasm_rust_tests(xs):
    return select({
        "@envoy//bazel:wasm_disabled": [],
        "@envoy//bazel:coverage_build": [],
        "//conditions:default": xs,
    })

wbpcode avatar Nov 23 '22 10:11 wbpcode

cool - if my hack above doent work then lets do that

phlax avatar Nov 23 '22 10:11 phlax

for ref rustc_flags implementation is here

https://github.com/bazelbuild/rules_rust/blob/61b99cdd18dc918c9023e02431b40496b229e67c/rust/private/rustc.bzl#L1835-L1845

phlax avatar Nov 23 '22 11:11 phlax

I'm also meet the same issue: https://dev.azure.com/cncf/envoy/_build/results?buildId=121458&view=logs&j=bbe4b42d-86e6-5e9c-8a0b-fea01d818a24&t=e00c5a13-c6dc-5e9a-6104-69976170e881&l=224

wangfakang avatar Nov 24 '22 03:11 wangfakang

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] avatar Dec 24 '22 04:12 github-actions[bot]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

github-actions[bot] avatar Dec 31 '22 08:12 github-actions[bot]

Hi @phlax, @mpwarres, @wbpcode, I tried to re-enable it with some explorations but failed the exam ;-(. Do you have more insights and want to take a look?

It looks like the rust profile build is not supported in wasm32-unkown-unkown, and we need to disable the profile build. Here is a summary:

  1. In addition to what @phlax tried and I also tried add rustc_flags = ["--cfg=disable_profiler"] + rustc_flags, but it doesn't build
  2. Tried the global RUSTFLAGS with# build --action_env=RUSTFLAGS="-C profile-generate=no", but it failed with some external build.
  3. We can try to use
build:coverage --instrumentation_filter="^//source(?!/common/quic/platform)[/:],^//envoy[/:],^//contrib(?!/.*/test)[/:], //(?!test/extensions/filters/http/wasm/test_data)[/:]", //(?!test/extensions/filters/http/wasm/test_data)[/:]"

or -@rules_rust[/:],-@proxy_wasm_rust_sdk[/:] to filter out the instrument build, maybe my regex is wrong, but got

ERROR: /usr/local/google/home/boteng/.cache/bazel/_bazel_boteng/cc869c547587616e74431ebbdbd7f03c/external/crates_vendor__log-0.4.22/BUILD.bazel:13:13: Compiling Rust rlib log v0.4.22 (10 files) failed: (Exit 1): process_wrapper failed: error executing command (from target @crates_vendor__log-0.4.22//:log) 
  (cd /usr/local/google/home/boteng/.cache/bazel/_bazel_boteng/cc869c547587616e74431ebbdbd7f03c/execroot/envoy && \
  exec env - \
    BAZEL_COMPILER=clang \
    BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 \
    BAZEL_LINKLIBS=-l%:libstdc++.a \
    BAZEL_LINKOPTS=-lm \
    BAZEL_USE_LLVM_NATIVE_COVERAGE=1 \
    CARGO_CFG_TARGET_ARCH=wasm32 \
    CARGO_CFG_TARGET_OS=wasi \
    CARGO_CRATE_NAME=log \
    CARGO_MANIFEST_DIR='${pwd}/external/crates_vendor__log-0.4.22' \
    CARGO_PKG_AUTHORS='' \
    CARGO_PKG_DESCRIPTION='' \
    CARGO_PKG_HOMEPAGE='' \
    CARGO_PKG_NAME=log \
    CARGO_PKG_VERSION=0.4.22 \
    CARGO_PKG_VERSION_MAJOR=0 \
    CARGO_PKG_VERSION_MINOR=4 \
    CARGO_PKG_VERSION_PATCH=22 \
    CARGO_PKG_VERSION_PRE='' \
    CC=clang \
    CXX=clang++ \
    CXXFLAGS='-stdlib=libc++' \
    GCOV=llvm-profdata \
    LDFLAGS='-stdlib=libc++' \
    PATH=/bin:/usr/bin:/usr/local/bin \
    REPOSITORY_NAME=crates_vendor__log-0.4.22 \
  bazel-out/k8-opt-exec-8E910DA8-ST-f70ac4ce2ceb/bin/external/rules_rust/util/process_wrapper/process_wrapper --subst 'pwd=${pwd}' -- bazel-out/k8-fastbuild-ST-15cae4aa1a0c/bin/external/rust_linux_x86_64__wasm32-wasi__stable_tools/rust_toolchain/bin/rustc external/crates_vendor__log-0.4.22/src/lib.rs '--crate-name=log' '--crate-type=rlib' '--error-format=human' '--codegen=metadata=-1021366919' '--codegen=extra-filename=-1021366919' '--out-dir=bazel-out/k8-fastbuild-ST-15cae4aa1a0c/bin/external/crates_vendor__log-0.4.22' '--codegen=opt-level=0' '--codegen=debuginfo=0' '--codegen=strip=none' '--remap-path-prefix=${pwd}=' '--emit=dep-info,link' '--color=always' '--target=wasm32-wasi' -L bazel-out/k8-fastbuild-ST-15cae4aa1a0c/bin/external/rust_linux_x86_64__wasm32-wasi__stable_tools/rust_toolchain/lib/rustlib/wasm32-wasi/lib -L bazel-out/k8-fastbuild-ST-15cae4aa1a0c/bin/external/rust_linux_x86_64__wasm32-wasi__stable_tools/rust_toolchain/lib/rustlib/wasm32-wasi/lib/self-contained '--cap-lints=allow' '--edition=2021' '--codegen=instrument-coverage' '--sysroot=bazel-out/k8-fastbuild-ST-15cae4aa1a0c/bin/external/rust_linux_x86_64__wasm32-wasi__stable_tools/rust_toolchain')
# Configuration: 1e27b5d18df1a392cb26e21847ab4e6935a33c925573d52f21f0753a26462cc7
# Execution platform: //bazel/rbe/toolchains:rbe_linux_clang_libcxx_platform
warning: the `wasm32-wasi` target is being renamed to `wasm32-wasip1` and the `wasm32-wasi` target will be removed from nightly in October 2024 and removed from stable Rust in January 2025

error[E0463]: can't find crate for `profiler_builtins`
  |
  = note: the compiler may have been built without the profiler runtime

error: aborting due to 1 previous error

My initial investigation is the global clang profile build will force include '--codegen=instrument-coverage', so that all external builds will also try to build rust while the rustc_flags in rust_library doesn't take effect. Is it possible a problem related to proxy-wasm-rust-sdk? - don't know right now

botengyao avatar Sep 25 '24 18:09 botengyao

cc @mpwarres @phlax @wbpcode

I actually got a workaround to disable the instrument for rust through a patch, waiting on CI to verify.

--- rust/private/rustc.bzl
+++ rust/private/rustc.bzl
@@ -1043,7 +1043,7 @@ def construct_arguments(

     if toolchain.llvm_cov and ctx.configuration.coverage_enabled:
         # https://doc.rust-lang.org/rustc/instrument-coverage.html
-        rustc_flags.add("--codegen=instrument-coverage")
+        pass

     if toolchain._experimental_link_std_dylib:
         rustc_flags.add("--codegen=prefer-dynamic")

botengyao avatar Sep 26 '24 19:09 botengyao