Skip to content

Commit

Permalink
build: fix inspector dependency resolution
Browse files Browse the repository at this point in the history
It was reported that parallel builds on Windows sometimes error because
of missing intermediate files.

On closer inspection I noticed that some files are copied from src/ to
the intermediate build directory in a way where they don't participate
in dependency resolution. Put another way, the build system doesn't
know to wait for the copy to complete because we don't tell it to.

Fix that by not copying around files but instead making the script that
processes them a little smarter about where to find them and where to
store the results.

PR-URL: #27026
Fixes: #27025
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
  • Loading branch information
bnoordhuis authored and BethGriggs committed Apr 9, 2019
1 parent ca7c4f4 commit 19a30f3
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 19 deletions.
20 changes: 5 additions & 15 deletions src/inspector/node_inspector.gypi
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
'variables': {
'protocol_tool_path': '../../tools/inspector_protocol',
'node_inspector_path': '../../src/inspector',
'node_inspector_generated_sources': [
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/Forward.h',
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/Protocol.cpp',
Expand Down Expand Up @@ -67,23 +66,14 @@
'<(SHARED_INTERMEDIATE_DIR)',
'<(SHARED_INTERMEDIATE_DIR)/src', # for inspector
],
'copies': [
{
'files': [
'<(node_inspector_path)/node_protocol_config.json',
'<(node_inspector_path)/node_protocol.pdl'
],
'destination': '<(SHARED_INTERMEDIATE_DIR)',
}
],
'actions': [
{
'action_name': 'convert_node_protocol_to_json',
'inputs': [
'<(SHARED_INTERMEDIATE_DIR)/node_protocol.pdl',
'node_protocol.pdl',
],
'outputs': [
'<(SHARED_INTERMEDIATE_DIR)/node_protocol.json',
'<(SHARED_INTERMEDIATE_DIR)/src/node_protocol.json',
],
'action': [
'python',
Expand All @@ -95,8 +85,8 @@
{
'action_name': 'node_protocol_generated_sources',
'inputs': [
'<(SHARED_INTERMEDIATE_DIR)/node_protocol_config.json',
'<(SHARED_INTERMEDIATE_DIR)/node_protocol.json',
'node_protocol_config.json',
'<(SHARED_INTERMEDIATE_DIR)/src/node_protocol.json',
'<@(node_protocol_files)',
],
'outputs': [
Expand All @@ -108,7 +98,7 @@
'tools/inspector_protocol/code_generator.py',
'--jinja_dir', '<@(protocol_tool_path)/..',
'--output_base', '<(SHARED_INTERMEDIATE_DIR)/src/',
'--config', '<(SHARED_INTERMEDIATE_DIR)/node_protocol_config.json',
'--config', 'src/inspector/node_protocol_config.json',
],
'message': 'Generating node protocol sources from protocol json',
},
Expand Down
7 changes: 3 additions & 4 deletions tools/inspector_protocol/code_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@

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)
Expand Down Expand Up @@ -71,7 +71,6 @@ def init_defaults(config_tuple, path, defaults):
if not config_file:
raise Exception("Config file name must be specified")
config_file = config_file.decode('utf8')
config_base = os.path.dirname(config_file)
config_values = arg_options.config_value
if not config_values:
config_values = []
Expand All @@ -84,7 +83,7 @@ def init_defaults(config_tuple, path, defaults):
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,
Expand Down

1 comment on commit 19a30f3

@richardlau
Copy link
Member

@richardlau richardlau commented on 19a30f3 Apr 10, 2019

Choose a reason for hiding this comment

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

@BethGriggs 19a30f3 appears to be missing the change to L109 in src/inspector/node_inspector.gypi from 608878c which is causing the v11.14.0-proposal build to fail.

Please sign in to comment.