Bump v8 to version 11.2.34

[jlb6740]

Hi .. @lizan @PiotrSikora Hi .. I'd like to bump v8 used here to a more recent version. Ultimately this is to support the version used in envoy. Two things concerning this:

(1) In envoy there is a patch file located here: which is beefer version of the one found here: .. basically subsuming what is found in the v8.patch in this repository. So first, should these files be consistent? Second, how do I test that the patch applies OK here when building. I wasn't sure which target to build to test the new v8 but I choose this one this set of commands at the root:

bazel clean --expunge
bazel build  --worker_verbose --verbose_explanations=true --verbose_failures --sandbox_debug --explain=build.out.txt //:v8_lib

The build completed and clearly seem to bring in the new v8. However, when I sabotaged the v8.patch by adding garbage to it, the steps above still worked. Clearly the v8.patch file was not applied while building v8.

• Also a related question .. does the build command above build the v8 library or the wee8 library (which I understand is target option when compiling v8)?

(2) The build files in envoy have pointers to a version of v8 found here:

https://github.com/envoyproxy/envoy/blob/9d9940b49f642fcf483ac8ff387ecb521dcf94fc/bazel/repository_locations.bzl#L1065-L1078

in repositories_locations.bzl point to the same version of v8 as listed here:

proxy-wasm-cpp-host/bazel/repositories.bzl

Lines 126 to 135 in 0f5a0e7

	maybe(
	git_repository,
	name = "v8",
	# 10.7.193.13
	commit = "6c8b357a84847a479cd329478522feefc1c3195a",
	remote = "https://chromium.googlesource.com/v8/v8",
	shallow_since = "1664374400 +0000",
	patches = ["@proxy_wasm_cpp_host//bazel/external:v8.patch"],
	patch_args = ["-p1"],
	)

So, setting aside that I don't understand why the envoy build needs to point to a different repository with the same code for v8 that is already brought in by proxy-wasm-cpp-host, based on some comments from @lizan about dependencies being flattened, I do assume these to sources need to be the same version of v8. So I tried to do a wget on the location here:

urls = ["https://storage.googleapis.com/envoyproxy-wee8/v8-{version}.tar.gz"],

substituting the version number (11.2.34) this patch is trying to bump to and the file was not found. Of course the old commit version (10.7.193.13) is found. How do I update this location with the version of v8 consistent to the one now pointed to in this patch?

So my primary questions are:
(1) How do I test that v8.patch is applied properly when just building proxy-wasm-cpp-host.
(2) Which v8.patch file is used by envoy (the one in the envoy repo or the one in this repo)
(3) Should these two v8.patch files be identical?
(4) How do I update https://storage.googleapis.com/ to include v8-11.2.34.tar.gz

I guess I have another question related to this patch. I'd like to include a v8 patch file for proxy-wasm-cpp-host (or envoy) that is targeting build flags in v8 friendly to debugging and perf analysis. I assume that needs to be done with a patch file. How do I add another v8-perf.patch file that is only used when specified with some command line flag when building.

nearly identically but basically subsumes what is here in this repository:

[PiotrSikora]

(1) In envoy there is a patch file located here: which is beefer version of the one found here: .. basically subsuming what is found in the v8.patch in this repository.
So first, should these files be consistent?

No, some of those changes are Envoy-specific (e.g. integration with it's build system and existing dependencies).

Second, how do I test that the patch applies OK here when building. I wasn't sure which target to build to test the new v8 but I choose this one this set of commands at the root:

bazel clean --expunge
bazel build  --worker_verbose --verbose_explanations=true --verbose_failures --sandbox_debug --explain=build.out.txt //:v8_lib

The build completed and clearly seem to bring in the new v8. However, when I sabotaged the v8.patch by adding garbage to it, the steps above still worked. Clearly the v8.patch file was not applied while building v8.

If it builds, then it applied fine. The patch command / format ignores garbage that's outside of the marked diff, so it's possible that your sabotaged changes are not even processed.

• Also a related question .. does the build command above build the v8 library or the wee8 library (which I understand is target option when compiling v8)?

It's the @v8//:wee8 target (grep for it).

(2) The build files in envoy have pointers to a version of v8 found here:

https://github.com/envoyproxy/envoy/blob/9d9940b49f642fcf483ac8ff387ecb521dcf94fc/bazel/repository_locations.bzl#L1065-L1078

in repositories_locations.bzl point to the same version of v8 as listed here:

proxy-wasm-cpp-host/bazel/repositories.bzl

Lines 126 to 135 in 0f5a0e7

	maybe(
	git_repository,
	name = "v8",
	# 10.7.193.13
	commit = "6c8b357a84847a479cd329478522feefc1c3195a",
	remote = "https://chromium.googlesource.com/v8/v8",
	shallow_since = "1664374400 +0000",
	patches = ["@proxy_wasm_cpp_host//bazel/external:v8.patch"],
	patch_args = ["-p1"],
	)

So, setting aside that I don't understand why the envoy build needs to point to a different repository with the same code for v8 that is already brought in by proxy-wasm-cpp-host, based on some comments from @lizan about dependencies being flattened, I do assume these to sources need to be the same version of v8. So I tried to do a wget on the location here:

urls = ["https://storage.googleapis.com/envoyproxy-wee8/v8-{version}.tar.gz"],

substituting the version number (11.2.34) this patch is trying to bump to and the file was not found. Of course the old commit version (10.7.193.13) is found.

Envoy wants tarballs and googlesource.com doesn't support deterministic tarballs, so it has to be created and stored externally. See how it's build here: https://storage.googleapis.com/envoyproxy-wee8/wee8-fetch-deps.sh, it's only an automated dependency extraction and tarball creation.

The comment about flattened checkout was true with old gclient/gn builds, but that's no longer the case after we've switched to V8's native Bazel support.

How do I update this location with the version of v8 consistent to the one now pointed to in this patch?

Only @mpwarres can update envoyproxy GCS bucket.

So my primary questions are:
(1) How do I test that v8.patch is applied properly when just building proxy-wasm-cpp-host.

If it builds, then it applied fine.

(2) Which v8.patch file is used by envoy (the one in the envoy repo or the one in this repo)

The one in Envoy.

(3) Should these two v8.patch files be identical?

No (see above).

(4) How do I update https://storage.googleapis.com/ to include v8-11.2.34.tar.gz

Ask @mpwarres.

I guess I have another question related to this patch. I'd like to include a v8 patch file for proxy-wasm-cpp-host (or envoy) that is targeting build flags in v8 friendly to debugging and perf analysis. I assume that needs to be done with a patch file. How do I add another v8-perf.patch file that is only used when specified with some command line flag when building.

You could add another V8 target (e.g. --wasm=v8perf with different patch). But why cannot this be done at runtime via config flags or something?

nearly identically but basically subsumes what is here in this repository:

(missing link)

[PiotrSikora]

You need to update //base/trace_event/common below to a version that matches V8 as well.

See https://storage.googleapis.com/envoyproxy-wee8/wee8-fetch-deps.sh (or just run it) to see how it's extracted.

[PiotrSikora]

This is a wrong version to update to.

The main branch of V8 isn't stable, and you should use only versions used in linux/stable or linux/beta builds of Chrome. See v8_version column here: https://omahaproxy.appspot.com.

[jlb6740]

@PiotrSikora So I attempted to update the patch to either linux/stable (10.9.194.10) and linux/beta (11.0.226.13) but unfortunately both fail with the error:

external/v8/src/heap/base/asm/x64/save_registers_asm.cc:5:10: fatal error: src/heap/base/stack.h: No such file or directory
5 | #include <src/heap/base/stack.h>
| ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

This file in fact does exists but because the bazel build uses a build command that includes the needed directory path with:

-iquote external/v8

instead of

-I external/v8

this file is ignored and not seen. The save_registers_asm.cc includes the file as <src/heap/base/stack.h> instead of "src/heap/base/stack.h". I am not sure an acceptable course of action to rectify this but the issue is not seen obviously in the current version of v8 used in proxy-wasm-cpp-host head nor in the version of v8 that I initially tried to push. That said, apparently the patch that introduced save_registers_asm.cc was failing in some cases and so the fix here http://crrev.com/c/4171639 removed the offending files. So I will hold off on this patch until this fix is in beta channel.

[PiotrSikora]

@PiotrSikora So I attempted to update the patch to either linux/stable (10.9.194.10) and linux/beta (11.0.226.13) but unfortunately both fail with the error:

external/v8/src/heap/base/asm/x64/save_registers_asm.cc:5:10: fatal error: src/heap/base/stack.h: No such file or directory
5 | #include <src/heap/base/stack.h>
| ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

This file in fact does exists but because the bazel build uses a build command that includes the needed directory path with:

-iquote external/v8

instead of

-I external/v8

this file is ignored and not seen. The save_registers_asm.cc includes the file as <src/heap/base/stack.h> instead of "src/heap/base/stack.h". I am not sure an acceptable course of action to rectify this but the issue is not seen obviously in the current version of v8 used in proxy-wasm-cpp-host head nor in the version of v8 that I initially tried to push.

You can add diff replacing <src/heap/base/stack.h> with "src/heap/base/stack.h" to v8.patch. V8 uses quote-includes for local files, so the former is clearly wrong, but the V8's native build system (gn) doesn't break the build because of that.

That said, apparently the patch that introduced save_registers_asm.cc was failing in some cases and so the fix here http://crrev.com/c/4171639 removed the offending files. So I will hold off on this patch until this fix is in beta channel.

That's probably 2-3 months away, so not worth waiting for, IMHO.