wasm: migrate to V8's native Bazel rules. #19275

PiotrSikora · 2021-12-13T21:45:28Z

Signed-off-by: Piotr Sikora piotrsikora@google.com

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

repokitteh-read-only · 2021-12-13T21:45:31Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #19275 was opened by PiotrSikora.

see: more, trace.

repokitteh-read-only · 2021-12-13T21:45:35Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @phlax

🐱

Caused by: #19275 was opened by PiotrSikora.

see: more, trace.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora · 2021-12-15T05:32:50Z

WIP as a few things are still failing (and were working fine using the existing gn / genrule_cmd build):

Coverage fails either due to relocation R_X86_64_PC32 out of range (when V8 is linked statically with linkstatic=1) or due to an undefined symbol: v8_Default_embedded_blob_code_ (when V8 is linked dynamically with linkstatic=0). The former works for me locally, so it might be an issue with the CI (although, I'm using the same version of LLVM). The latter sounds like an issue with V8's Bazel integration.
MSan tests (not covered by the CI) are reporting reading uninitialized values. Note that the existing gn build is using v8_target_cpu=arm64 when building on x86_64, so I'm not sure why the resulting binary works at all, since we're not running it under a simulator. When I change the existing gn build to target host CPU, then MSan tests also fail, so I'm willing to let this one go.

Please note that this migration involves a dozen or so patches to V8's Bazel integration, but I think merging this is acceptable once the individual patches are merged into V8.

cc @victorgomes @thhous-msft

PiotrSikora · 2021-12-15T06:13:07Z

Also, Docker multiarch is failing due to running out of the disk space. @phlax any ideas how to fix that?

phlax · 2021-12-15T12:11:31Z

Docker multiarch is failing due to running out of the disk space. @phlax any ideas how to fix that?

We download ~1GB of artefacts before running the docker build. Im guessing we can delete those files once they have been loaded. Not sure if that will be enough, but worth a try. I can raise a PR to do it and you can pick the commit to see if it helps

https://dev.azure.com/cncf/envoy/_build/results?buildId=96709&view=logs&j=2229703b-209a-5c70-43aa-3eacf674f12a&t=5618bc45-8382-5c2b-9d5d-3e45721ec0f0&l=18

phlax · 2021-12-15T12:37:42Z

docker PR is here #19294

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora · 2021-12-16T07:17:19Z

/retest

repokitteh-read-only · 2021-12-16T07:17:22Z

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #19275 (comment) was created by @PiotrSikora.

see: more, trace.

phlax · 2021-12-16T07:56:15Z

@PiotrSikora checking CI further it seems the size of the binaries and dwp files increase significantly with this PR

$ tar zvtf ~/Downloads/thispr/envoy-contrib_binary.tar.gz 
drwxr-xr-x envoybuild/envoygroup 0 2021-12-16 04:47 build_envoy-contrib_release/
-r-xr-xr-x envoybuild/envoygroup 2644450792 2021-12-16 04:47 build_envoy-contrib_release/envoy.dwp
-r-xr-xr-x envoybuild/envoygroup  525520416 2021-12-16 04:47 build_envoy-contrib_release/envoy
-r-xr-xr-x envoybuild/envoygroup       8936 2021-12-16 04:47 build_envoy-contrib_release/su-exec
drwxr-xr-x envoybuild/envoygroup          0 2021-12-16 04:47 build_envoy-contrib_release_stripped/
-rwxr-xr-x envoybuild/envoygroup   65236544 2021-12-16 04:47 build_envoy-contrib_release_stripped/envoy
$ tar zvtf ~/Downloads/envoymain/envoy-contrib_binary.tar.gz 
drwxr-xr-x envoybuild/envoygroup 0 2021-12-16 05:30 build_envoy-contrib_release/
-r-xr-xr-x envoybuild/envoygroup 1552118064 2021-12-16 05:30 build_envoy-contrib_release/envoy.dwp
-r-xr-xr-x envoybuild/envoygroup  463377288 2021-12-16 05:30 build_envoy-contrib_release/envoy
-r-xr-xr-x envoybuild/envoygroup       8936 2021-12-16 05:30 build_envoy-contrib_release/su-exec
drwxr-xr-x envoybuild/envoygroup          0 2021-12-16 05:30 build_envoy-contrib_release_stripped/
-rwxr-xr-x envoybuild/envoygroup   63976896 2021-12-16 05:30 build_envoy-contrib_release_stripped/envoy

the PR i raised should help in terms of CI, but wondering about the size change

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora · 2021-12-16T08:44:13Z

@phlax yeah, both builds are quite different (including the default options and flags), so I'm trying to figure out what's the issue, but it's getting a bit tricky since the coverage works for me locally using the same rules and RBE toolchain.

Thanks for looking! The size difference gave me an idea... Let's see if the snapshot compression helps.

This reverts commit 10a263a. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora · 2022-02-05T08:42:54Z

removing alpine is still an option, but im not sure it would make that big a difference. We can also try pruning intermediate build images between builds i think

Could you create a draft PR, so that I could cherry-pick either of those changes to see if it helps?

phlax · 2022-02-07T07:21:13Z

there is a PR to remove the alpine build queued to land on main here #19837

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

phlax · 2022-02-07T12:20:12Z

@PiotrSikora doesnt seem to have helped - ill fork your Pr and see if i can debug and figure out a way to prune between builds

phlax · 2022-02-07T12:42:26Z

added #19845

phlax · 2022-02-07T13:52:47Z

from initial debugging it doesnt seem like there is too much worth pruning between builds for a couple of reasons - one is that there is hardly any space in the first place, also the builds are building the same images, just with different envoy artefacts, so there is nothing really prunable there

i have added a commit to remove the images that (i think) come preinstalled in azp, lets see if that works/helps

phlax · 2022-02-07T16:33:59Z

i think #19848 should resolve the disk space issue

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora · 2022-02-08T08:35:07Z

/retest

repokitteh-read-only · 2022-02-08T08:35:10Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #19275 (comment) was created by @PiotrSikora.

see: more, trace.

PiotrSikora · 2022-02-08T09:46:18Z

Finally! #19848 (with temurin-*-jdk purged per @lizan's comment) allows for the succesful Docker multiarch build.

We're good to go once it's merged! Thanks a lot for helping to make a room for this, @phlax @lizan.

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

moderation · 2022-02-08T23:21:45Z

bazel/repository_locations.bzl

+    com_googlesource_chromium_zlib = dict(
+        project_name = "Chromium's zlib",
+        project_desc = "Chromium’s fork of zlib with compression utils",
+        project_url = "https://chromium.googlesource.com/chromium/src/third_party/zlib/",
+        # NOTE: Update together with v8 and com_googlesource_chromium_base_trace_event_common.
+        # Use version and sha256 from https://storage.googleapis.com/envoyproxy-wee8/v8-<v8_version>-deps.sha256.
+        version = "fc5cfd78a357d5bb7735a58f383634faaafe706a",
+        # Static snapshot created using https://storage.googleapis.com/envoyproxy-wee8/wee8-fetch-deps.sh.
+        sha256 = "695c73750cf6472fc6c926e43952262206f1475157377364142bdbb84a1a5a83",
+        urls = ["https://storage.googleapis.com/envoyproxy-wee8/chromium-zlib-{version}.tar.gz"],
+        use_category = ["dataplane_ext"],
+        extensions = ["envoy.wasm.runtime.v8"],
+        release_date = "2022-01-12",
+        cpe = "N/A",
+    ),


Any concerns with this being the 3rd zlib dependency we bring into Envoy /cc @envoyproxy/dependency-shepherds

Note that we're not using this as a zlib library, we're linking against whatever //external:zlib points at.

Unfortuantely, V8 uses tiny C++ shim library that's included in their fork (the "zlib compression utils"), which we have to use, but similarly, that's linked against //external:zlib in this build.

Lastly, this is still an improvement over the existing genrule_repository() that we use for V8, which links against Chromium's zlib fork.

PiotrSikora · 2022-02-09T00:15:46Z

/retest

repokitteh-read-only · 2022-02-09T00:15:49Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #19275 (comment) was created by @PiotrSikora.

see: more, trace.

moderation · 2022-02-09T01:27:10Z

/lgtm deps

Fixes envoyproxy#15145. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Josh Perry <josh.perry@mx.com>

Fixes envoyproxy#15145. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Ryan Northey <ryan@synca.io>

Fixes #15145. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Ryan Northey <ryan@synca.io>

PiotrSikora added 2 commits December 10, 2021 14:50

wasm: update V8 to v9.7.106.17.

dcf1d3e

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

wasm: migrate to V8's native Bazel support.

c411adc

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Dec 13, 2021

repokitteh-read-only bot assigned phlax Dec 13, 2021

PiotrSikora added 6 commits December 13, 2021 15:38

review: fix presubmit.

193fa96

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: fix build on Linux/aarch64.

8ab7c90

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: add link for the aarch64 fix.

23b3b8e

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: fix coverage.

b7232dd

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: fix build with GCC.

c0ecc28

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: regenerate patch.

590d817

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

phlax mentioned this pull request Dec 15, 2021

docker: Delete CI build artefacts after loading #19294

Merged

PiotrSikora force-pushed the v8_v9.7-with-bazel branch from cfb6bde to 590d817 Compare December 16, 2021 03:53

review: force -O1 in fastbuild.

71a803e

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: enable snapshot compression.

10a263a

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

lizan self-assigned this Jan 3, 2022

PiotrSikora added 5 commits January 5, 2022 15:22

Revert "review: enable snapshot compression."

aced9bc

This reverts commit 10a263a. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: update V8 to v9.9.62.

d4bbfc0

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: try build on Windows.

feedc69

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Merge remote-tracking branch 'origin/main' into PiotrSikora/v8_v9.7-w…

0a8ce19

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: use locally installed toolchain on Windows.

cfab867

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora added 2 commits February 6, 2022 23:58

Merge remote-tracking branch 'origin/main' into PiotrSikora/v8_v9.7-w…

5abeb04

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: cherry-pick envoyproxy#19837 for testing.

6126953

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

phlax mentioned this pull request Feb 7, 2022

Revert "ci: Dont remove packages in ci jobs (#17146)" #19848

Merged

PiotrSikora added 4 commits February 7, 2022 21:15

Merge remote-tracking branch 'origin/main' into PiotrSikora/v8_v9.7-w…

7afcdb7

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: cherry-pick envoyproxy#19848 for testing.

fd2b513

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: purge temurin-*-jdk.

25f84db

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: Kick CI.

c71c30c

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Merge remote-tracking branch 'origin/main' into PiotrSikora/v8_v9.7-w…

6459e57

…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>

lizan approved these changes Feb 8, 2022

View reviewed changes

moderation reviewed Feb 8, 2022

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Feb 9, 2022

lizan merged commit 3be5738 into envoyproxy:main Feb 9, 2022

joshperry pushed a commit to joshperry/envoy that referenced this pull request Feb 13, 2022

wasm: migrate to V8's native Bazel rules. (envoyproxy#19275)

25c61f5

Fixes envoyproxy#15145. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Josh Perry <josh.perry@mx.com>

phlax pushed a commit to phlax/envoy that referenced this pull request Nov 15, 2022

wasm: migrate to V8's native Bazel rules. (envoyproxy#19275)

b836329

Fixes envoyproxy#15145. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Ryan Northey <ryan@synca.io>

phlax pushed a commit that referenced this pull request Nov 18, 2022

wasm: migrate to V8's native Bazel rules. (#19275)

9e7b8eb

Fixes #15145. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Ryan Northey <ryan@synca.io>

jbohanon mentioned this pull request Apr 20, 2023

[1.21] April 4 CVEs solo-io/envoy-fork#2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm: migrate to V8's native Bazel rules. #19275

wasm: migrate to V8's native Bazel rules. #19275

PiotrSikora commented Dec 13, 2021 •

edited

Loading

repokitteh-read-only bot commented Dec 13, 2021

repokitteh-read-only bot commented Dec 13, 2021

PiotrSikora commented Dec 15, 2021

PiotrSikora commented Dec 15, 2021

phlax commented Dec 15, 2021

phlax commented Dec 15, 2021

PiotrSikora commented Dec 16, 2021

repokitteh-read-only bot commented Dec 16, 2021

phlax commented Dec 16, 2021

PiotrSikora commented Dec 16, 2021 •

edited

Loading

PiotrSikora commented Feb 5, 2022

phlax commented Feb 7, 2022

phlax commented Feb 7, 2022

phlax commented Feb 7, 2022

phlax commented Feb 7, 2022 •

edited

Loading

phlax commented Feb 7, 2022

PiotrSikora commented Feb 8, 2022

repokitteh-read-only bot commented Feb 8, 2022

PiotrSikora commented Feb 8, 2022

moderation Feb 8, 2022

PiotrSikora Feb 9, 2022

PiotrSikora commented Feb 9, 2022

repokitteh-read-only bot commented Feb 9, 2022

moderation commented Feb 9, 2022

wasm: migrate to V8's native Bazel rules. #19275

wasm: migrate to V8's native Bazel rules. #19275

Conversation

PiotrSikora commented Dec 13, 2021 • edited Loading

repokitteh-read-only bot commented Dec 13, 2021

repokitteh-read-only bot commented Dec 13, 2021

PiotrSikora commented Dec 15, 2021

PiotrSikora commented Dec 15, 2021

phlax commented Dec 15, 2021

phlax commented Dec 15, 2021

PiotrSikora commented Dec 16, 2021

repokitteh-read-only bot commented Dec 16, 2021

phlax commented Dec 16, 2021

PiotrSikora commented Dec 16, 2021 • edited Loading

PiotrSikora commented Feb 5, 2022

phlax commented Feb 7, 2022

phlax commented Feb 7, 2022

phlax commented Feb 7, 2022

phlax commented Feb 7, 2022 • edited Loading

phlax commented Feb 7, 2022

PiotrSikora commented Feb 8, 2022

repokitteh-read-only bot commented Feb 8, 2022

PiotrSikora commented Feb 8, 2022

moderation Feb 8, 2022

Choose a reason for hiding this comment

PiotrSikora Feb 9, 2022

Choose a reason for hiding this comment

PiotrSikora commented Feb 9, 2022

repokitteh-read-only bot commented Feb 9, 2022

moderation commented Feb 9, 2022

PiotrSikora commented Dec 13, 2021 •

edited

Loading

PiotrSikora commented Dec 16, 2021 •

edited

Loading

phlax commented Feb 7, 2022 •

edited

Loading