From 5d91780108646efd34afc332ac05c17319e4d3e4 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 18 Apr 2017 08:20:56 +0200 Subject: [PATCH 1/6] build: fix ninja build failure When working on commit 6a09a69 ("build: enable cctest to use generated objects") I did not take into account building with ninja: $ ./configure $ tools/gyp_node.py -f ninja $ ninja -C out/Release $ ln -fs out/Release/node node When ninja generated the ninja build files, src files that are relative to the src directory will be named with a dot instead of a path separator, for example: out/Release/obj/src/node/node.o would instead become: out/Release/obj/src/node.node.o This commit adds an additional variable for the typ of object separator used for this case. Also, the LIBS specified in the 'libraries' section are not being included in the --start-group --end-group section which means that these libraries will not be searched causing issue with linkers where the order matters. Fixes: https://github.com/nodejs/node/issues/12448 --- common.gypi | 19 +++++----- node.gyp | 50 +++++++++++++++----------- tools/gyp/pylib/gyp/generator/ninja.py | 4 +-- 3 files changed, 39 insertions(+), 34 deletions(-) diff --git a/common.gypi b/common.gypi index a97e12ae38c7f1..ea08e80365b5c6 100644 --- a/common.gypi +++ b/common.gypi @@ -35,6 +35,13 @@ 'icu_use_data_file_flag%': 0, 'conditions': [ + ['GENERATOR=="ninja"', { + 'OBJ_DIR': '<(PRODUCT_DIR)/obj', + 'V8_BASE': '<(PRODUCT_DIR)/obj/deps/v8/src/libv8_base.a', + }, { + 'OBJ_DIR%': '<(PRODUCT_DIR)/obj.target', + 'V8_BASE%': '<(PRODUCT_DIR)/obj.target/deps/v8/src/libv8_base.a', + }], ['OS == "win"', { 'os_posix': 0, 'v8_postmortem_support%': 'false', @@ -45,18 +52,8 @@ 'v8_postmortem_support%': 'true', }], ['OS== "mac"', { - 'OBJ_DIR': '<(PRODUCT_DIR)/obj.target', + 'OBJ_DIR%': '<(PRODUCT_DIR)/obj.target', 'V8_BASE': '<(PRODUCT_DIR)/libv8_base.a', - }, { - 'conditions': [ - ['GENERATOR=="ninja"', { - 'OBJ_DIR': '<(PRODUCT_DIR)/obj', - 'V8_BASE': '<(PRODUCT_DIR)/obj/deps/v8/src/libv8_base.a', - }, { - 'OBJ_DIR%': '<(PRODUCT_DIR)/obj.target', - 'V8_BASE%': '<(PRODUCT_DIR)/obj.target/deps/v8/src/libv8_base.a', - }], - ], }], ['openssl_fips != ""', { 'OPENSSL_PRODUCT': 'libcrypto.a', diff --git a/node.gyp b/node.gyp index d7a282134ee286..5dbc205b82f57c 100644 --- a/node.gyp +++ b/node.gyp @@ -574,7 +574,14 @@ 'OBJ_GEN_PATH': '<(OBJ_DIR)/node/gen', 'OBJ_TRACING_PATH': '<(OBJ_DIR)/node/src/tracing', 'OBJ_SUFFIX': 'o', + 'OBJ_SEPARATOR': '/', 'conditions': [ + ['GENERATOR=="ninja"', { + 'OBJ_PATH': '<(OBJ_DIR)/src/', + 'OBJ_GEN_PATH': '<(OBJ_DIR)/gen/', + 'OBJ_TRACING_PATH': '<(OBJ_DIR)/src/tracing/', + 'OBJ_SEPARATOR': 'node.', + }], ['OS=="win"', { 'OBJ_PATH': '<(OBJ_DIR)/node', 'OBJ_GEN_PATH': '<(OBJ_DIR)/node', @@ -603,24 +610,25 @@ ], 'libraries': [ - '<(OBJ_GEN_PATH)/node_javascript.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_debug_options.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/async-wrap.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/env.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_buffer.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_i18n.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_url.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/util.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/string_bytes.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/string_search.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/stream_base.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_constants.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_revert.<(OBJ_SUFFIX)', - '<(OBJ_TRACING_PATH)/agent.<(OBJ_SUFFIX)', - '<(OBJ_TRACING_PATH)/node_trace_buffer.<(OBJ_SUFFIX)', - '<(OBJ_TRACING_PATH)/node_trace_writer.<(OBJ_SUFFIX)', - '<(OBJ_TRACING_PATH)/trace_event.<(OBJ_SUFFIX)', + '<(OBJ_GEN_PATH)<(OBJ_SEPARATOR)node_javascript.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_debug_options.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)async-wrap.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)env.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_buffer.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_i18n.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_url.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)debug-agent.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)util.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)string_bytes.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)string_search.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)stream_base.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_constants.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_revert.<(OBJ_SUFFIX)', + '<(OBJ_TRACING_PATH)<(OBJ_SEPARATOR)agent.<(OBJ_SUFFIX)', + '<(OBJ_TRACING_PATH)<(OBJ_SEPARATOR)node_trace_buffer.<(OBJ_SUFFIX)', + '<(OBJ_TRACING_PATH)<(OBJ_SEPARATOR)node_trace_writer.<(OBJ_SUFFIX)', + '<(OBJ_TRACING_PATH)<(OBJ_SEPARATOR)trace_event.<(OBJ_SUFFIX)', ], 'defines': [ @@ -683,9 +691,9 @@ 'copies': [{ 'destination': '<(OBJ_DIR)/cctest/src', 'files': [ - '<(OBJ_PATH)/node_dtrace_ustack.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_dtrace_provider.<(OBJ_SUFFIX)', - '<(OBJ_PATH)/node_dtrace.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_dtrace_ustack.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_dtrace_provider.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)node_dtrace.<(OBJ_SUFFIX)', ]}, ], }], diff --git a/tools/gyp/pylib/gyp/generator/ninja.py b/tools/gyp/pylib/gyp/generator/ninja.py index 0555a4a90d3885..1f67c945005028 100644 --- a/tools/gyp/pylib/gyp/generator/ninja.py +++ b/tools/gyp/pylib/gyp/generator/ninja.py @@ -2148,13 +2148,13 @@ def GenerateOutputForConfig(target_list, target_dicts, data, params, restat=True, command=mtime_preserving_solink_base % {'suffix': '@$link_file_list'}, rspfile='$link_file_list', - rspfile_content='-Wl,--start-group $in -Wl,--end-group $solibs $libs', + rspfile_content='-Wl,--start-group $in $solibs $libs -Wl,--end-group', pool='link_pool') master_ninja.rule( 'link', description='LINK $out', command=('$ld $ldflags -o $out ' - '-Wl,--start-group $in -Wl,--end-group $solibs $libs'), + '-Wl,--start-group $in $solibs $libs -Wl,--end-group'), pool='link_pool') elif flavor == 'win': master_ninja.rule( From e65f70d193d1e4fafb57c463a9c3d51f94ae6c18 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 19 Apr 2017 18:12:42 +0200 Subject: [PATCH 2/6] make win settings optional --- node.gyp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/node.gyp b/node.gyp index 5dbc205b82f57c..b7d4926b158241 100644 --- a/node.gyp +++ b/node.gyp @@ -583,9 +583,9 @@ 'OBJ_SEPARATOR': 'node.', }], ['OS=="win"', { - 'OBJ_PATH': '<(OBJ_DIR)/node', - 'OBJ_GEN_PATH': '<(OBJ_DIR)/node', - 'OBJ_TRACING_PATH': '<(OBJ_DIR)/node', + 'OBJ_PATH%': '<(OBJ_DIR)/node', + 'OBJ_GEN_PATH%': '<(OBJ_DIR)/node', + 'OBJ_TRACING_PATH%': '<(OBJ_DIR)/node', 'OBJ_SUFFIX': 'obj', }], ['OS=="aix"', { From 9a87428633292b86405c1a74115480843ce1aca2 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 19 Apr 2017 18:25:16 +0200 Subject: [PATCH 3/6] add check for .obj file extensions Currently the files specified in libraries in node.gyp cctest target are getting a '.lib' extension on windows when generated with ninja. This commit adds a check to see if a file has a '.obj' extension and in that case no '.lib' extension will be added. --- tools/gyp/pylib/gyp/msvs_emulation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/gyp/pylib/gyp/msvs_emulation.py b/tools/gyp/pylib/gyp/msvs_emulation.py index e4a85a96e6c536..14daaec4c7fabc 100644 --- a/tools/gyp/pylib/gyp/msvs_emulation.py +++ b/tools/gyp/pylib/gyp/msvs_emulation.py @@ -269,7 +269,8 @@ def ConvertVSMacros(self, s, base_to_build=None, config=None): def AdjustLibraries(self, libraries): """Strip -l from library if it's specified with that.""" libs = [lib[2:] if lib.startswith('-l') else lib for lib in libraries] - return [lib + '.lib' if not lib.endswith('.lib') else lib for lib in libs] + return [lib + '.lib' if not lib.endswith('.lib') \ + and not lib.endswith('.obj') else lib for lib in libs] def _GetAndMunge(self, field, path, default, prefix, append, map): """Retrieve a value from |field| at |path| or return |default|. If From a543ed038964bec71d1f8d8c71f4e05e705bcfce Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 20 Apr 2017 07:20:46 +0200 Subject: [PATCH 4/6] prefix OBJ_SEPARATOR with slash for ninja gen --- node.gyp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/node.gyp b/node.gyp index b7d4926b158241..293f8d6d4ae2e5 100644 --- a/node.gyp +++ b/node.gyp @@ -577,10 +577,10 @@ 'OBJ_SEPARATOR': '/', 'conditions': [ ['GENERATOR=="ninja"', { - 'OBJ_PATH': '<(OBJ_DIR)/src/', - 'OBJ_GEN_PATH': '<(OBJ_DIR)/gen/', - 'OBJ_TRACING_PATH': '<(OBJ_DIR)/src/tracing/', - 'OBJ_SEPARATOR': 'node.', + 'OBJ_PATH': '<(OBJ_DIR)/src', + 'OBJ_GEN_PATH': '<(OBJ_DIR)/gen', + 'OBJ_TRACING_PATH': '<(OBJ_DIR)/src/tracing', + 'OBJ_SEPARATOR': '/node.', }], ['OS=="win"', { 'OBJ_PATH%': '<(OBJ_DIR)/node', From 1f9b5729d22327be0429b867560cb87af546cddf Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 20 Apr 2017 08:10:20 +0200 Subject: [PATCH 5/6] make if/else out of the generator statement Currently the check for if ninja is being used is a normal if statement as are the following os checks (win and aix). But the win and aix ones should only be evaluated if the build is not generated by ninja. This commit turns this logic into an if ninja else statement. --- node.gyp | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/node.gyp b/node.gyp index 293f8d6d4ae2e5..0229dfff8bac3e 100644 --- a/node.gyp +++ b/node.gyp @@ -576,24 +576,29 @@ 'OBJ_SUFFIX': 'o', 'OBJ_SEPARATOR': '/', 'conditions': [ + ['OS=="win"', { + 'OBJ_SUFFIX': 'obj', + }], ['GENERATOR=="ninja"', { 'OBJ_PATH': '<(OBJ_DIR)/src', 'OBJ_GEN_PATH': '<(OBJ_DIR)/gen', 'OBJ_TRACING_PATH': '<(OBJ_DIR)/src/tracing', 'OBJ_SEPARATOR': '/node.', - }], - ['OS=="win"', { - 'OBJ_PATH%': '<(OBJ_DIR)/node', - 'OBJ_GEN_PATH%': '<(OBJ_DIR)/node', - 'OBJ_TRACING_PATH%': '<(OBJ_DIR)/node', - 'OBJ_SUFFIX': 'obj', - }], - ['OS=="aix"', { - 'OBJ_PATH': '<(OBJ_DIR)/node_base/src', - 'OBJ_GEN_PATH': '<(OBJ_DIR)/node_base/gen', - 'OBJ_TRACING_PATH': '<(OBJ_DIR)/node_base/src/tracing', - }], - ], + }, { + 'conditions': [ + ['OS=="win"', { + 'OBJ_PATH': '<(OBJ_DIR)/node', + 'OBJ_GEN_PATH': '<(OBJ_DIR)/node', + 'OBJ_TRACING_PATH': '<(OBJ_DIR)/node', + }], + ['OS=="aix"', { + 'OBJ_PATH': '<(OBJ_DIR)/node_base/src', + 'OBJ_GEN_PATH': '<(OBJ_DIR)/node_base/gen', + 'OBJ_TRACING_PATH': '<(OBJ_DIR)/node_base/src/tracing', + }], + ]} + ] + ], }, 'includes': [ From 36e54a70abea1618e5fe39c332fa4312f913ab80 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 11 May 2017 10:50:33 +0200 Subject: [PATCH 6/6] remove debug-agent from cctest target This file has been removed in upstream/master but was still included in the cctest target causing an error. --- node.gyp | 1 - 1 file changed, 1 deletion(-) diff --git a/node.gyp b/node.gyp index 0229dfff8bac3e..f50aea3475598d 100644 --- a/node.gyp +++ b/node.gyp @@ -623,7 +623,6 @@ '<(OBJ_PATH)<(OBJ_SEPARATOR)node_buffer.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)node_i18n.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)node_url.<(OBJ_SUFFIX)', - '<(OBJ_PATH)<(OBJ_SEPARATOR)debug-agent.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)util.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)string_bytes.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)string_search.<(OBJ_SUFFIX)',