Conversation
|
Build fails on Windows (VS2015): https://ci.nodejs.org/job/node-compile-windows/6598/ |
|
It's a bit buried in the sea of warnings but the build error is this: Seems something goes wrong in the generate_inspector_protocol_sources phase. |
|
cc @ofrobots ^ |
|
I ran out of day-time and couldn't look at this today. I will try again tomorrow. /cc @eugeneo. |
I believe that already happened =). |
|
/cc @dgozman Maybe it's a bug in V8's gypfiles? |
This matches the number in nodejs/node#10992.
This matches the number in nodejs/node#10992.
|
5.7 has been released with faster async/await and PromiseHook API. Would it make sense to jump into it ? |
|
@YurySolovyov 5.7 is still in beta phase. We will update to it when Chrome 57 is stable (should be mid-March) |
This matches the number in nodejs/node#10992.
|
ping @nodejs/v8 for the Windows build issue. Basically the problem is that |
|
@targos Can you check if this patch helps? diff --git a/deps/v8/gypfiles/toolchain.gypi b/deps/v8/gypfiles/toolchain.gypi
index 95eb1d9..5105fff 100644
--- a/deps/v8/gypfiles/toolchain.gypi
+++ b/deps/v8/gypfiles/toolchain.gypi
@@ -989,8 +989,6 @@
# present in VS 2003 and earlier.
'msvs_disabled_warnings': [4351],
'msvs_configuration_attributes': {
- 'OutputDirectory': '<(DEPTH)\\build\\$(ConfigurationName)',
- 'IntermediateDirectory': '$(OutDir)\\obj\\$(ProjectName)',
'CharacterSet': '1',
},
}],If we don't want to float a patch, we should be able to override it from common.gypi like this: diff --git a/common.gypi b/common.gypi
index d87205e..7fd9efc 100644
--- a/common.gypi
+++ b/common.gypi
@@ -148,6 +148,10 @@
}
}]
],
+ 'msvs_configuration_attributes': {
+ 'IntermediateDirectory': '$(ConfigurationName)\\obj\\$(ProjectName)',
+ 'OutputDirectory': '$(SolutionDir)$(ConfigurationName)',
+ },
'msvs_settings': {
'VCCLCompilerTool': {
'Optimization': 3, # /Ox, full optimization(Caveat emptor: needs to be duplicated in the Debug configuration.) |
|
@bnoordhuis thanks for getting around to it sooner than me. I did a quick test on a windows box with that patch and it seems to work. /cc @ak239: can you think of a better upstream fix? |
|
@bnoordhuis I confirm it works on my computer too. What prevents us from upstreaming this fix? |
|
Are third_party being rolled as expected? https://github.com/v8/v8/blob/5.7-lkgr/third_party/inspector_protocol/lib/Values_h.template was updated 4 months ago and looks like this CL still has an old version. |
|
Sorry, I've mistaken 5.6 and 5.7. |
Original commit message:
[build] Fix gyp files for building inspector
This patch fixes compilation of V8 with inspector on Windows as well as
cross-compilation of the V8 inspector.
BUG=
Refs: nodejs#10992
Review-Url: https://codereview.chromium.org/2705423003
Cr-Commit-Position: refs/heads/master@{nodejs#43533}
PR-URL: nodejs#11752
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Original commit message:
[build] Fix gyp files for building inspector
This patch fixes compilation of V8 with inspector on Windows as well as
cross-compilation of the V8 inspector.
BUG=
Refs: nodejs#10992
Review-Url: https://codereview.chromium.org/2705423003
Cr-Commit-Position: refs/heads/master@{nodejs#43533}
Original commit message:
[build] Fix gyp files for building inspector
This patch fixes compilation of V8 with inspector on Windows as well as
cross-compilation of the V8 inspector.
BUG=
Refs: nodejs#10992
Review-Url: https://codereview.chromium.org/2705423003
Cr-Commit-Position: refs/heads/master@{nodejs#43533}
This matches the number in nodejs/node#10992.
This matches the number in nodejs/node#10992.
This matches the number in nodejs/node#10992.
Depends on #9618.V8 5.6 should become stable next week.It stable now!I'm opening the PR now to track eventual issues with CI.
CI: https://ci.nodejs.org/job/node-test-commit/7473/?auto_refresh=true
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
v8