inspector: bring in inspector_protocol as a direct dependency #76
Conversation
* Split "gn gen" and "ninja" invocations into two separate actions * Make the ninja action use console pool so that progress is shown * Set DEPOT_TOOLS_WIN_TOOLCHAIN=0 by default (to avoid non-googlers being asked for credentials) * Propagate depot_tools path to "gn gen" step (it runs vs_toolchain.py) * Add flags for clang-cl, setting gn args, sub-ninja -j and -l parameters
* Fixed several hardcoded paths to use <(STATIC_LIB_PREFIX) &
<(STATIC_LIB_SUFFIX)
* Added GENERATOR=='ninja' conditionals to /WHOLEARCHIVE flags (msbuild
puts .lib files under separate directories).
* Add missing link_settings for v8_monolith
* Set use_sysroot=false only for non-windows builds.
To build using clang-cl:
python configure --build-v8-with-gn --use-clang-cl --ninja
It was breaking when .gypi strings had quotes in them. e.g.: 'foo': 'bar="baz"'
It needed to be passed via "make_global_settings" rather than environment variable.
It was breaking during install when .gypi strings had quotes in them. e.g.: 'foo': 'bar="baz"'
Precursor to removing deprecated APIs on the v8 side @ https://chromium-review.googlesource.com/c/v8/v8/+/1045310
This is done in preparation for landing https://chromium-review.googlesource.com/c/v8/v8/+/1126099 on the V8 side, which extends the existing PromiseRejectEvent mechanism with new hooks for reject/resolve after a Promise was previously resolved already. Refs: nodejs/promises-debugging#8 Design: https://goo.gl/2stLUY
As per Node.js docs, vm.Script instance is not bound to any context. However, this test was expecting otherwise and depended on implementation details which are going to change. Refs: https://chromium-review.googlesource.com/c/v8/v8/+/1013581
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 6.9. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
V8 improved break locations for the node --inspect-brk -e case.
In v8 we want to remove the lldbinit file (https://crrev.com/c/1127892). In order to not brake node.js, we need to remove it here first.
This patch adds [inspector_protocol](https://www.google.com/url?q=https://chromium.googlesource.com/deps/inspector_protocol/) as a direct dependency to node.js. Inspector Protocol is used in node.js tracing, and this is required to be able to roll protocol independently from v8. inspector_protocol dependency comes with two more libs: - jinja2 - markupsafe
|
@hashseed can you please take a look? |
|
Can I get some more context? Is the copy of inspector protocol in V8 still required when rolling into V8, e.g. when running |
|
It looks like right now V8 and node.js use the same inspector_protocol version so when we need to land some protocol changes to V8 we need to roll it in a way that would not break Node.js that makes everything harder. Having two different copies will make rolling inspector_protocol changes much easier since we can roll them separately. |
|
I see. That means that we no longer have to copy inspector when running |
|
I am not sure that I understand what inspector did you mention. I believe we still need both one inside V8 and one inside node. One from V8 is used by inspector inside V8, one from node is used by node inspector_agent.cc. |
|
I see. So we are not sharing the code between Node and V8, right? Upstream PR would be great, yes. Once that lands I can just sync with upstream. |
|
@hashseed The node.js part has landed: nodejs#21975 Should I do anything else to land https://chromium-review.googlesource.com/c/v8/v8/+/1149321 ? |
hashseed
force-pushed
the
vee-eight-lkgr
branch
3 times, most recently
from
01b19b3
to
0c05ebd
Compare
|
I think this can be closed? |
This patch adds
inspector_protocol
as a direct dependency to node.js. Inspector Protocol is used in node.js
tracing, and this is required to be able to roll protocol independently
from v8.
This is necessary to roll protocol to v8: v8 patch
inspector_protocol dependency comes with two more libs:
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes