wasm: enable on Windows.

[PiotrSikora]

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

[repokitteh-read-only]

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).

🐱

Caused by: #15252 was synchronize by PiotrSikora.

see: more, trace.

[PiotrSikora]

@lizan any ideas?

AssertionError: user32.lib is not found in LIB:
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.28.29333\lib\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.18362.0\ucrt\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.19041.0\um\x64
Check if it is installed.

[wrowe]

Hi Piotr... I stood up our windows build image locally to verify... There are a half dozen envvars which bazel foreign cc cmake infers, which are not being inferred in this case (foreign make?). I'm examining why, this morning, to figure out the straight line solution.

[PiotrSikora]

Hi Piotr... I stood up our windows build image locally to verify... There are a half dozen envvars which bazel foreign cc cmake infers, which are not being inferred in this case (foreign make?). I'm examining why, this morning, to figure out the straight line solution.

V8 is using a custom genrule_repository, and nothing is automatically detected here.

For example, ENVOY_ASAN is defined in .bazelrc:

envoy/.bazelrc

Line 50 in 8ac28e2

build:asan --action_env=ENVOY_ASAN=1

before it can be used in wee8.genrule_cmd:

envoy/bazel/external/wee8.genrule_cmd

Lines 47 to 50 in 8ac28e2

	if [[ $${ENVOY_ASAN-} == "1" ]]; then
	WEE8_BUILD_ARGS+=" is_asan=true"
	WEE8_BUILD_ARGS+=" is_lsan=true"
	fi

Do you have a list of the missing env variables that should be included here?

[wrowe]

I'm preparing an independent patch that may let non-clang components build on windows by setting up the INCLUDE, LIB and similar variables as part of our envoy-build-tools docker image. Those are not initialized at present, since bazel and cmake components don't need them pre-declared, they are derived from bazel windows rules_cc.

More info as I have it. I'm setting this up separately so the CI will incrementally show our progress towards this specific PR.

[PiotrSikora]

I'm preparing an independent patch that may let non-clang components build on windows by setting up the INCLUDE, LIB and similar variables as part of our envoy-build-tools docker image. Those are not initialized at present, since bazel and cmake components don't need them pre-declared, they are derived from bazel windows rules_cc.

More info as I have it. I'm setting this up separately so the CI will incrementally show our progress towards this specific PR.

Thanks!

FWIW, we emit other values needed by genrule_cmd using --action_env in .bazelrc, so that might be an easier solution?

[wrowe]

FWIW, we emit other values needed by genrule_cmd using --action_env in .bazelrc

Yes, that's one piece of it. Rebuilding envoy-build-tools with rules to extract any envvar prereqs of this genrule dependency is another.

[wrowe]

Just to refresh this ticket... I don't have it working, yet, but there is some interesting information if you merge main (or at this point, even rebase it) and cherry pick wrowe@4a07ec5

This at least lets us see the state of the env table to hope to decipher how the VS toolchain logic may be failing.

[wrowe]

I'm a bit jammed on the state of trunk trying to build bazel/external/... - can you shed any insight?

[wrowe]

I'm jammed trying to get //bazel/external/... built, any insight @PiotrSikora ?

ERROR: error loading package '': in C:/source/bazel/repositories_extra.bzl: Label '@proxy_wasm_cpp_host//bazel/cargo:crates.bzl' is invalid because 'bazel/cargo' is not a package; perhaps you meant to put the colon here: '@proxy_wasm_cpp_host//:bazel/cargo/crates.bzl'?

[PiotrSikora]

Just to refresh this ticket... I don't have it working, yet, but there is some interesting information if you merge main (or at this point, even rebase it) and cherry pick wrowe@4a07ec5

This at least lets us see the state of the env table to hope to decipher how the VS toolchain logic may be failing.

Done, thanks!

I'm jammed trying to get //bazel/external/... built, any insight @PiotrSikora ?

ERROR: error loading package '': in C:/source/bazel/repositories_extra.bzl: Label '@proxy_wasm_cpp_host//bazel/cargo:crates.bzl' is invalid because 'bazel/cargo' is not a package; perhaps you meant to put the colon here: '@proxy_wasm_cpp_host//:bazel/cargo/crates.bzl'?

But it is a package, and it works for other builds. Perhaps you're using an outdated version of Bazel?

[PiotrSikora]

@wrowe doesn't look like it changed anything. It hits the same assertion as before (#15252 (comment)).

[wrowe]

I might have a slightly out of date toolchain, but this error;

AssertionError: user32.lib is not found in LIB:
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.28.29333\lib\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.18362.0\ucrt\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.19041.0\um\x64

I don't know where 19041 is coming from, should remain 18362. That said...

It is true that user32.lib doesn't exist, it should be in that last um/x64/ directory, but is distributed as
/c/Program Files (x86)/Windows Kits/10/Lib/10.0.18362.0/um/x64/User32.Lib in that filename case.

Looks like a silly case-sensitivity bug in the script (however, the correct case should be used, because
file trees created under a WSL linux path can end up case-sensitive.)

[PiotrSikora]

I don't think it's about case-sensitivity. V8 seems to hardcode expected VS version for consistency, and it looks that the CI is using an outdated Windows 10 SDK 10.0.18362.0 (from 2019) instead of 10.0.19041.0 (from 2020).

I think I managed to revert the requirement, but we should probably upgrade the SDK. What do you think?

[github-actions]

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[wrowe]

waiting does not defeat stalebot. Still waiting on upstream or a workaround to 1 hour timeout

[thhous-msft]

@PiotrSikora @lizan @wrowe I've been working on adding Windows support to the google bazel build that was added over the summer. I got it working, and then a PR in V8 this morning completely broke the changeset. But I got far enough to check a few things. At least on windows, the bazel build took the same amount of time as the gn build, as least locally. This is mostly because the bazel build still builds everything necessary. Which makes me worry it wouldn't speed up the build enough to solve the build system problem.

However, digging through the bazel build makes me realize we could probably actually do it in a different way. At least for now, we only need the wasm code. That is a much smaller subset of code. I think we actually might be better off by manually specifiying the source files needed for wasm, and basically not depend on their build at all. This is the same thing the bazel v8 build does for zlib and icu, and it seems to work well. I'm going to try this out and see if its feasible.

If so, I think that might be a better solution for us to discuss. Otherwise, even with a bazel build we'd likely have to get better build systesm.

[victorgomes]

@thhous-msft If you have a good way to disentangle the wee8 target to the rest of the V8 codebase, I am happy to upstream the change. The only reason wee8 depends of the entire V8 is that no one took the time to separate the dependencies.

Ideally one would want to have this separation expressed in GN as well. Otherwise, we would accidentally introduce dependencies in the future.

[PiotrSikora]

At least on windows, the bazel build took the same amount of time as the gn build, as least locally. This is mostly because the bazel build still builds everything necessary. Which makes me worry it wouldn't speed up the build enough to solve the build system problem.

That's expected, since both build systems use exactly the same compiler to compile exactly the same source code.

However, that's not an issue. The biggest problem we have with V8 is that Bazel sees the GN build as a single task and dispatches it to a single machine on RBE, whereas with native Bazel support, each file would be dispatched as a separate task.

However, digging through the bazel build makes me realize we could probably actually do it in a different way. At least for now, we only need the wasm code. That is a much smaller subset of code. I think we actually might be better off by manually specifiying the source files needed for wasm, and basically not depend on their build at all.

I think you're underestimating how intermingled Wasm is with JavaScript support in V8. Perhaps you're misguided by the small list of files (guarded by is_v8_enable_webassembly flag) that need to be compiled in addition to JavaScript-only version? The reverse is definitely not true.

We've talked about having Wasm-only build in the past (mostly to lower the binary size), so that would be amazing if you want to work on that, but it's definitely a lot more work than selecting a small subset of files to compile.

[thhous-msft]

I started working on the separate wasm build, but yeah I determined that wasn't helpful, everything still had to be built. So back to trying to get the bazel build inside of the V8 repo to work again on Windows.

@victorgomes This was the issue I emailed you about last week. I still haven't found a solution to that. Still trying to see if I can get any more verbose logs. Bazel is unfamiliar to me so the error is cryptic.

Maybe someone else on this issue can help with the error. Below is the error I get, which is very unhelpful, because it doesn't give any help in what is actually failing.

C:\src\v8>bazel build --jobs 48 --local_cpu_resources=48 --local_ram_resources=HOST_RAM*6 --verbose_failures  :wee8
ERROR: file 'noicu/libv8_libshared.lo' is generated by these conflicting actions:
Label: //:noicu/v8_libshared
RuleClass: cc_library rule
Configuration: 6d1022bc39033a614c56e63e3c91dac7894b7f861d08fe613213ffcf550bdb97
Mnemonic: Fail
Action key: 2c6d98c68041059ba1e7d2f4244f302b6282b7026466a13b41f422ee02b51499, 789ea502cf40a5f006c877742dac93365ea2777f0eb9b2648c085687ab8606c4
Progress message: Reporting failed target //:noicu/v8_libshared located at C:/src/v8/BUILD.bazel:3171:11
PrimaryInput: (null)
PrimaryOutput: File:[[<execution_root>]bazel-out/x64_windows-fastbuild/bin]noicu/libv8_libshared.lo
Owner information: ConfiguredTargetKey{label=//:noicu/v8_libshared, config=BuildConfigurationValue.Key[6d1022bc39033a614c56e63e3c91dac7894b7f861d08fe613213ffcf550bdb97]}
MandatoryInputs: are equal
Outputs: are equal
ERROR: Analysis of target '//:wee8' failed; build aborted: for noicu/libv8_libshared.lo, previous action: action 'Reporting failed target //:noicu/v8_libshared located at C:/src/v8/BUILD.bazel:3171:11', attempted action: action 'Reporting failed target //:noicu/v8_libshared located at C:/src/v8/BUILD.bazel:3171:11'

INFO: Elapsed time: 9.311s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 0 targets configured)

[thhous-msft]

I figured out the issue. Its a bug in bazel. I reported the issue to them as bazelbuild/bazel#14237

I can find a workaround on windows with alias's I think. I'll try.

[thhous-msft]

The code for doing this was merged into upstream V8 on Monday. So it seems like we have all the bazel support we need to finish this up. It looks like some custom scripts were used to package the existing v8 build and push it to a server. Who owns that script and would be able to rerun it against current upstream main?

[PiotrSikora]

The code for doing this was merged into upstream V8 on Monday. So it seems like we have all the bazel support we need to finish this up. It looks like some custom scripts were used to package the existing v8 build and push it to a server. Who owns that script and would be able to rerun it against current upstream main?

Done: wee8-9.8.67.tar.gz (sha256: ae1ebafef524e345fb8787bb82155524722bd99b70a650c531cfa52bbb62492a).

You can generate this archive yourself using wee8-archive.sh linked in bazel/repository_locations.bzl.

Note that the master branch is not suitable for production, so while I think that you should open the PR against this version to sort out possible issues with the CI and the custom wee8 archive that's heavily stripped, I don't think it should be merged until v8 v9.8 hits beta in early January.

[thhous-msft]

Thank you! and yeah I know we likely want to wait until beta to merge, but at least now we can test the build.

[thhous-msft]

Apparently bazel doesn't support recursive workspaces. And the v8 build declares 3 dependent folders with new_local_repository. Anyone know a workaround to get those local repositories to work? Otherwise its off to another rewrite of V8's bazel build system.

[thhous-msft]

I did some digging, and there no solution right now to new_local_respository with http downloaded dependencies. You can't get the dependency folder until its too late in the process.

[PiotrSikora]

@thhous-msft Bazel doesn't support recursive WORKSPACEs, but as of now there are only two local repositories in V8:

• zlib - which is already imported as @net_zlib,
• icu - which we don't need,

so this shouldn't be an issue at all, but need to patch BUILD.bazel file on import to adjust them accordingly. It would be extremelly helpful if you could open a work-in-progress PR with your changes, otherwise it's really hard to help you.

btw: I generated a tarball for the branched v9.8:
wee8-9.8.177.2.tar.gz (sha256: 348de05bbf3714864acdd08568f1badd4dae78c557593747816a0f0bd7104752)

[thhous-msft]

There's also a 3rd workspace loaded currently. All the configuration files are loaded from a 3rd workspace.

https://chromium.googlesource.com/v8/v8/+/refs/heads/main/WORKSPACE#19

I figured I could solve the zlib one, but I couldn't figure out how to solve that one. I was doing all this testing outside of envoy, as the envoy build took too long to be useful for testing this. So I don't have a Work in Progress PR for envoy itself. I can try doing one in January.

[PiotrSikora]

There's also a 3rd workspace loaded currently. All the configuration files are loaded from a 3rd workspace.

https://chromium.googlesource.com/v8/v8/+/refs/heads/main/WORKSPACE#19

Ah, I had v9.6 checked out locally and I didn't notice @config, thanks! I don't believe it's needed, and I've sent a CL to remove it: https://chromium-review.googlesource.com/c/v8/v8/+/3331591.

[PiotrSikora]

@wrowe @thhous-msft @lizan @phlax any ideas why Bazel cannot find cmake.exe (when building host tools, AFAICT)?

[PiotrSikora]

I'm abandoning this change. The remaining changes should be minimal, but trying to debug this using CI is not productive.