tools: update inspector_protocol to 0aafd2 #27770

targos · 2019-05-19T08:16:07Z

/cc @nodejs/v8-inspector

I have the following error that I don't know how to fix:

ACTION Generating node protocol sources from protocol json
FAILED: gen/src/node/inspector/protocol/Forward.h gen/src/node/inspector/protocol/Protocol.cpp gen/src/node/inspector/protocol/Protocol.h gen/src/node/inspector/protocol/NodeWorker.cpp gen/src/node/inspector/protocol/NodeWorker.h gen/src/node/inspector/protocol/NodeTracing.cpp gen/src/node/inspector/protocol/NodeTracing.h gen/src/node/inspector/protocol/NodeRuntime.cpp gen/src/node/inspector/protocol/NodeRuntime.h 
cd ../../; python tools/inspector_protocol/code_generator.py --jinja_dir tools/inspector_protocol --output_base out/Release/gen/src/ --config src/inspector/node_protocol_config.json
Traceback (most recent call last):
  File "tools/inspector_protocol/code_generator.py", line 688, in <module>
    main()
  File "tools/inspector_protocol/code_generator.py", line 562, in main
    protocol = Protocol(config)
  File "tools/inspector_protocol/code_generator.py", line 333, in __init__
    self.generate_domains = self.read_protocol_file(config.protocol.path)
  File "tools/inspector_protocol/code_generator.py", line 351, in read_protocol_file
    input_file = open(file_name, "r")
IOError: [Errno 2] No such file or directory: u'src/inspector/node_protocol.json'

nodejs-github-bot · 2019-05-19T08:16:08Z

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3565

bnoordhuis · 2019-05-20T12:22:39Z

You need these changes from 608878c:

diff --git a/tools/inspector_protocol/code_generator.py b/tools/inspector_protocol/code_generator.py
index 1e12343e05..7b555d7478 100755
--- a/tools/inspector_protocol/code_generator.py
+++ b/tools/inspector_protocol/code_generator.py
@@ -36,9 +36,9 @@ module_path, module_filename = os.path.split(os.path.realpath(__file__))
 
 def read_config():
     # pylint: disable=W0703
-    def json_to_object(data, output_base, config_base):
+    def json_to_object(data, output_base):
         def json_object_hook(object_dict):
-            items = [(k, os.path.join(config_base, v) if k == "path" else v) for (k, v) in object_dict.items()]
+            items = [(k, os.path.join(output_base, v) if k == "path" else v) for (k, v) in object_dict.items()]
             items = [(k, os.path.join(output_base, v) if k == "output" else v) for (k, v) in items]
             keys, values = list(zip(*items))
             return collections.namedtuple('X', keys)(*values)
@@ -69,7 +69,6 @@ def read_config():
         jinja_dir = arg_options.jinja_dir
         output_base = arg_options.output_base
         config_file = arg_options.config
-        config_base = os.path.dirname(config_file)
         config_values = arg_options.config_value
     except Exception:
         # Work with python 2 and 3 http://docs.python.org/py3k/howto/pyporting.html
@@ -80,7 +79,7 @@ def read_config():
     try:
         config_json_file = open(config_file, "r")
         config_json_string = config_json_file.read()
-        config_partial = json_to_object(config_json_string, output_base, config_base)
+        config_partial = json_to_object(config_json_string, output_base)
         config_json_file.close()
         defaults = {
             ".use_snake_file_names": False,

Now I get C++ compile errors, I'll dig some more. :-)

/home/bnoordhuis/src/master/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp: In function ‘std::unique_ptr<node::inspector::protocol::Value> node::inspector::protocol::{anonymous}::parseValue(int32_t, node::inspector::protocol::cbor::CBORTokenizer*)’:
/home/bnoordhuis/src/master/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp:230:70: error: ‘fromUTF16’ is not a member of ‘node::inspector::protocol::StringUtil’
       std::unique_ptr<Value> value = StringValue::create(StringUtil::fromUTF16(
                                                                      ^~~~~~~~~
/home/bnoordhuis/src/master/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp:230:70: note: suggested alternative: ‘fromUTF8’
       std::unique_ptr<Value> value = StringValue::create(StringUtil::fromUTF16(
                                                                      ^~~~~~~~~
                                                                      fromUTF8
/home/bnoordhuis/src/master/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp: In function ‘void node::inspector::protocol::{anonymous}::EncodeString(const String&, std::vector<unsigned char>*)’:
/home/bnoordhuis/src/master/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp:471:19: error: ‘CharacterCount’ is not a member of ‘node::inspector::protocol::StringUtil’
   if (StringUtil::CharacterCount(s) == 0) {
                   ^~~~~~~~~~~~~~
/home/bnoordhuis/src/master/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp:473:26: error: ‘CharactersLatin1’ is not a member of ‘node::inspector::protocol::StringUtil’
   } else if (StringUtil::CharactersLatin1(s)) {
                          ^~~~~~~~~~~~~~~~
...

bnoordhuis · 2019-05-20T12:57:01Z

@targos bnoordhuis/io.js@4ac1158716 should give you a compiling build. Tests pass for me locally.

Co-authored-by: Ben Noordhuis <[email protected]>

targos · 2019-05-20T13:14:57Z

Thank you Ben, I included your changes!

nodejs-github-bot · 2019-05-20T13:15:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/23232/

targos · 2019-05-21T08:19:57Z

/cc @eugeneo @ak239

bnoordhuis

(Partially RS)LGTM with some observations:

You check in roll.py but that's not something we use or could use, right?
Likewise, encoding_test.cc and encoding_test_helper.h aren't used or even built.
You check in encoding.cc and encoding.h but they're (re)generated from encoding_cpp.template and encoding_h.template, aren't they?

Aside: interesting that the inspector is adopting CBOR. Not that we currently use it but it's an interesting avenue to explore if we ever run into performance problems with the JSON-based protocol.

targos · 2019-05-21T09:24:45Z

So, what I did is copy the same files the V8 checks in here: https://github.com/v8/v8/tree/master/third_party/inspector_protocol
There indeed seems to be unnecessary things for us but I didn't want to blindly remove files.

Trott · 2019-05-30T15:28:10Z

@targos Are you waiting on @ak239 and/or @eugeneo to weigh in? Or can this land?

targos · 2019-05-30T15:44:56Z

I was waiting for a 2nd LGTM but now it can land 👍

Trott

More than a bit rubber-stampy of me but LGTM

Trott · 2019-06-01T07:29:36Z

Landed in 5aaa7fe

Co-authored-by: Ben Noordhuis <[email protected]> PR-URL: nodejs#27770 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Co-authored-by: Ben Noordhuis <[email protected]> PR-URL: #27770 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Rich Trott <[email protected]>

targos added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. inspector Issues and PRs related to the V8 inspector protocol labels May 19, 2019

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels May 19, 2019

tools: update inspector_protocol to 0aafd2

f1ee906

Co-authored-by: Ben Noordhuis <[email protected]>

targos force-pushed the inspector_protocol branch from 01d8bf1 to f1ee906 Compare May 20, 2019 13:14

targos removed the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label May 20, 2019

bnoordhuis approved these changes May 21, 2019

View reviewed changes

refack mentioned this pull request May 21, 2019

suggested re-work of nodejs/node#27770 targos/node#4

Closed

4 tasks

eugeneo approved these changes May 30, 2019

View reviewed changes

Trott approved these changes Jun 1, 2019

View reviewed changes

Trott closed this Jun 1, 2019

targos mentioned this pull request Jun 3, 2019

v12.4.0 release proposal #28040

Merged

targos deleted the inspector_protocol branch October 17, 2020 12:56

This was referenced Mar 2, 2021

tools: fix compiler warning in inspector_protocol #37573

Merged

deps: update inspector_protocol to c149e90e9ff5bf7 #37574

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: update inspector_protocol to 0aafd2 #27770

tools: update inspector_protocol to 0aafd2 #27770

targos commented May 19, 2019

nodejs-github-bot commented May 19, 2019

bnoordhuis commented May 20, 2019

bnoordhuis commented May 20, 2019 •

edited

Loading

targos commented May 20, 2019

nodejs-github-bot commented May 20, 2019

targos commented May 21, 2019

bnoordhuis left a comment

targos commented May 21, 2019

Trott commented May 30, 2019 •

edited

Loading

targos commented May 30, 2019

Trott left a comment

Trott commented Jun 1, 2019

tools: update inspector_protocol to 0aafd2 #27770

tools: update inspector_protocol to 0aafd2 #27770

Conversation

targos commented May 19, 2019

nodejs-github-bot commented May 19, 2019

bnoordhuis commented May 20, 2019

bnoordhuis commented May 20, 2019 • edited Loading

targos commented May 20, 2019

nodejs-github-bot commented May 20, 2019

targos commented May 21, 2019

bnoordhuis left a comment

Choose a reason for hiding this comment

targos commented May 21, 2019

Trott commented May 30, 2019 • edited Loading

targos commented May 30, 2019

Trott left a comment

Choose a reason for hiding this comment

Trott commented Jun 1, 2019

bnoordhuis commented May 20, 2019 •

edited

Loading

Trott commented May 30, 2019 •

edited

Loading