Skip to content

Conversation

@targos
Copy link
Member

/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' 

@targostargos 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
Copy link
Collaborator

@nodejs-github-botnodejs-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
@bnoordhuis
Copy link
Member

You need these changes from 608878c:

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

Details
/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
Copy link
Member

bnoordhuis commented May 20, 2019

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

@targos
Copy link
MemberAuthor

Thank you Ben, I included your changes!

@targostargosforce-pushed the inspector_protocol branch from 01d8bf1 to f1ee906CompareMay 20, 2019 13:14
@nodejs-github-bot
Copy link
Collaborator

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

/cc @eugeneo@ak239

Copy link
Member

@bnoordhuisbnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Partially RS)LGTM with some observations:

  1. You check in roll.py but that's not something we use or could use, right?

  2. Likewise, encoding_test.cc and encoding_test_helper.h aren't used or even built.

  3. 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
Copy link
MemberAuthor

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
Copy link
Member

Trott commented May 30, 2019

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

@targos
Copy link
MemberAuthor

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

Copy link
Member

@TrottTrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More than a bit rubber-stampy of me but LGTM

@Trott
Copy link
Member

Trott commented Jun 1, 2019

Landed in 5aaa7fe

@TrottTrott closed this Jun 1, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Jun 1, 2019
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]>
targos added a commit that referenced this pull request Jun 1, 2019
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]>
@targostargos mentioned this pull request Jun 3, 2019
@targostargos deleted the inspector_protocol branch October 17, 2020 12:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.inspectorIssues and PRs related to the V8 inspector protocollib / srcIssues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@targos@nodejs-github-bot@bnoordhuis@Trott@eugeneo