-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
wasm: migrate to V8's native Bazel rules. #19275
Conversation
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to |
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
WIP as a few things are still failing (and were working fine using the existing
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. |
Also, 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 |
docker PR is here #19294 |
cfb6bde
to
590d817
Compare
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
/retest |
Retrying Azure Pipelines: |
@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>
@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>
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>
Could you create a draft PR, so that I could cherry-pick either of those changes to see if it helps? |
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>
@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 |
added #19845 |
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 |
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>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
/retest |
Retrying Azure Pipelines: |
…ith-bazel Signed-off-by: Piotr Sikora <piotrsikora@google.com>
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", | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any concerns with this being the 3rd zlib dependency we bring into Envoy /cc @envoyproxy/dependency-shepherds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
/retest |
Retrying Azure Pipelines: |
/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>
Fixes #15145.
Signed-off-by: Piotr Sikora piotrsikora@google.com