[v8] Build failure in x64-windows (likely broken by PR #24066)

[hugoam]

Host Environment

• OS: Windows
• Compiler: Visual Studio 2019 Version 16.11

To Reproduce
Steps to reproduce the behavior:
./vcpkg install v8 (with default triplet set to x64-windows)

Failure logs
This will produce an error logged to vcpkg\buildtrees\v8\generate-x64-windows-dbg-out.log which looks like the following:

ERROR at //testing/gmock/BUILD.gn:11:1: Dependency not allowed.
source_set("gmock") {
^--------------------
The item //testing/gmock:gmock
can not depend on //third_party/googletest:gtest_config
because it is not in //third_party/googletest:gtest_config's visibility list: [
  //third_party/googletest:*
]

Additional context
It seems this is broken by PR #24066 (I'm currently building the parent commit 2ac61f8 and it seems fine so far)
Another project has faced a similar issue: envoyproxy/envoy#18365
It seems tied to the recent update of GN, while V8 itself hasn't been updated, and the fix is apparently included in a later V8 commit v8/v8@eac21d5 (https://chromium.googlesource.com/v8/v8.git/+/eac21d572e92a82f5656379bc90f8ecf1ff884fc)

I've tried to fix the port with the following approaches:

1. bumping V8 version of the port to eac21d572e92a82f5656379bc90f8ecf1ff884fc, but this causes too many issues that I don't know how to address, and
2. adding the content of the patch above to the patch of the port, but I then hit another issue which I don't know how to solve either (something about python3)

[JackBoosY]

cc @Kwizatz
It should be a port bug.

[hugoam]

I guess probably @bold84 too as I am now 100% positive PR #24066 broke this.
2ac61f8 builds perfectly fine,

[JackBoosY]

cc @bold84

[hugoam]

If this is not solved soonish, wouldn't it make sense to revert the PR that broke V8? As a reminder, that one: #24066
It doesn't seem like updating one vcpkg port (Skia) is worth it if the cost is to break another port... Keeping this kind of broken state really hinders the interest of using vcpkg in general by a very significant factor

[bold84]

The update to the new v8 version was necessary because the old version wasn't compatible with the new gn. I wasn't aware of such issues with v8 at that time.

@hugoam
The entitlement I sense in the way you write doesn't really motivate me to fix your problem "soonish".
So, if you want a quick solution, use the 2ac61f8 commit, or use overlay ports. What stops you from using that "perfectly fine" parent commit you mentioned? It seems like you actually want to use the newer version and just need someone to fix it for you.

@JackBoosY
I will have a look at it when I'm less busy.

[hugoam]

I'm sorry, it's my mistake and I didn't actually want it to sound that way.
There was a rational thought in there, which was that it might be more desirable for most people using vcpkg if the PR was reverted until we know no ports are broken by it, and one argument is that having broken ports on master that previously worked is an objective drawback that makes vcpkg less usable. But it's an open source project, so sorry about expressing entitlement.

We can use the parent commit, that's how we're currently doing, but our usage of vcpkg is not automated or versioned so we have little control over the fact that someone might get the wrong vcpkg version and lose time trying to figure out why it's not working.

[Kwizatz]

Hi, so, last time I updated v8 I came across some reluctance to let the patches in, there was a heated discussion on whether a fix upstream was or wasn't the "correct" fix for an issue that was present on the version I was updating to, but already fixed on upstream/master.

In the end I had to wait for the version that had the fix to become the current stable at the time and only then update. I was asked to push the patches upstream rather than keeping them here, which I thought made sense, but I expected the chrome developers to show little interest in the patches... and so they did. forward to a couple of weeks back, I tried updating the patches and posted them upstream, but I need to start a discussion with the chrome developers on why they would be interested in merging the patches, honestly, I am pessimistic, the reasoning being that they would have to set up a CI environment to check that the patches don't break, even though there is one already set up here...

Anyway, I digress, here are the patches for v8 master branch a couple of weeks old, the size of the patches I think has come down, and they may work with the latest stable version, you may want to see if they work for you and you'll be doing me a big favor by uploading a PR here 😁

Chromium BUILD
Chromium ICU
v8

Also, if a mayor project or port is making use of the v8 port I contributed it would really help if they pushed upstream to merge the patches.

[Kwizatz]

Hi @JackBoosY

So I am working on this but found that the python3 package downloaded by vcpkg_find_acquire_program(PYTHON3) does not contain a python3 symlink to python.exe, as such, GN does not find python3. Should I file an Issue?

Thanks!

[JackBoosY]

Hi @JackBoosY

So I am working on this but found that the python3 package downloaded by vcpkg_find_acquire_program(PYTHON3) does not contain a python3 symlink to python.exe, as such, GN does not find python3. Should I file an Issue?

Thanks!

@Kwizatz In #24817?

[Kwizatz]

Hi @JackBoosY
So I am working on this but found that the python3 package downloaded by vcpkg_find_acquire_program(PYTHON3) does not contain a python3 symlink to python.exe, as such, GN does not find python3. Should I file an Issue?
Thanks!

@Kwizatz In #24817?

I added a workaround, copy the python.exe as python3.exe over the buildtrees directory, so it may not be necessary. It would be good to have the vcpkg_find_acquire_program function make the copy at the extraction location, but really the embeddable package should contain the executable/symlink to begin with.

[JackBoosY]

I added a workaround, copy the python.exe as python3.exe over the buildtrees directory, so it may not be necessary. It would be good to have the vcpkg_find_acquire_program function make the copy at the extraction location, but really the embeddable package should contain the executable/symlink to begin with.

No no, please don't do that.
We should completely fix the issue instead of avoid it.

[Kwizatz]

No no, please don't do that. We should completely fix the issue instead of avoid it.

Ok, its just that I think that the issue here is at the Python package, in other words, its an oversight at how python.org creates their embeddable bundle.

[ZRR666]

Cloning into 'build'...
fatal: unable to access 'https://chromium.googlesource.com/chromium/src/build.git/': SSL certificate problem: self signed certificate

[ZRR666]

ERROR at //build/config/win/BUILD.gn:299:27: Script returned non-zero exit code.
vcvars_toolchain_data = exec_script("../../toolchain/win/setup_toolchain.py",
^----------
Current dir: C:/src/vcpkg/buildtrees/v8/x86-windows-dbg/
Command: C:/src/vcpkg/downloads/tools/python/python-3.10.5-x86/python.exe C:/src/vcpkg/buildtrees/v8/src/a883683717-e774d37ef1.clean/build/toolchain/win/setup_toolchain.py "C:\Program Files (x86)/Microsoft Visual Studio/2017/Professional" "C:\Program Files (x86)\Windows Kits\10" "C:\WINDOWS\Sysnative;C:\WINDOWS\SysWOW64;Arm64Unused" win x86 none
Returned 1.
See //build/config/BUILDCONFIG.gn:344:5: which caused the file to be included.
"//build/config/win:default_cfg_compiler",
^----------------------------------------

[over-shine]

I meet the same problem with latest vcpkg. How to make it?

[JackBoosY]

cc @LilyWangLL

[piusayowale]

For anyone who might come across this issue in future, just as I did, this is a fix

v8/v8@eac21d5

[futscdav]

For anyone who might come across this issue in future, just as I did, this is a fix

v8/v8@eac21d5

Thanks, this was really helpful. Had to run with --editable and locally fix the files. v8 also no longer builds because vcpkg downloads python 3.10, the jinja2 fetched submodule invokes the no longer supported from collections import Mapping and vcpkg doesn't seem to respect --editable for submodules. I was able to get a build to succeed by manually patching the file between the time it's refetched and actually used, which is an awful hack, but works. Maybe there's a way to layer a patch on top of the fetch that I'm not aware of.

Is the v8 package support not maintained? Seems like this has been a known issue for almost a year, and the commit referenced by @piusayowale is from Sept 2021.

[Kwizatz]

Is the v8 package support not maintained? Seems like this has been a known issue for almost a year, and the commit referenced by @piusayowale is from Sept 2021.

Unfortunatelly no it is not being mantained. The last unmerged update I did had a clash with the Windows SDK version installed on the CI servers, the build requieres a specific version which is not the latest and does not match the versions on the VCPKG side. That was pretty much it, it did build on my system, but without the tests passing it just couldn't be merged.

If anyone is interested the commits are on this branch:

https://github.com/Kwizatz/vcpkg/tree/v8

Anyway, later things just got worse, I wasn't just keeping the VCPKG version up to date but the msys2 version as well as keeping it building on Linux using gcc, but gcc 12 and 13 has broken a lot of things, both compatibility with clang flags as well as some bugs on false positives and a broken idiom when mixing gcc attribute() with C++ [[attribute]].

So, in short, at the moment there's too many things that need to be fixed outside of my reach, and even when everything settles who knows how long before it all breaks all over again. Upstream had no interest in accepting the patches in part because these issues would mean having someone keeping track and supporting yet another compiler 🤷‍♂️.

[github-actions]

This is an automated message. Per our repo policy, stale issues get closed if there has been no activity in the past 180 days. The issue will be automatically closed in 14 days. If you wish to keep this issue open, please add a new comment.