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 a97e12ae38c7..ea08e80365b5 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 d7a282134ee2..5dbc205b82f5 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 0555a4a90d38..1f67c9450050 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 5dbc205b82f5..b7d4926b1582 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 e4a85a96e6c5..14daaec4c7fa 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 b7d4926b1582..293f8d6d4ae2 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 293f8d6d4ae2..0229dfff8bac 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 0229dfff8bac..f50aea347559 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)',