-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
node: Do not use shared abseil-cpp
#25582
Conversation
Signed-off-by: Austin Lane <[email protected]>
Hi @vidplace7 Question. The source code distributed by node.js includes abseil-cpp.
It appears to be used in the current package.
Please tell us why this modification is necessary for this package. |
@nxhack a shared Without this patch, if the shared |
Understood. |
It seems that the host build of node.js fails when abseil-cpp is installed. We will consider a workaround. |
It's a very dirty patch, but it worked! diff --git a/lang/node/patches/204-v8_gyp.patch b/lang/node/patches/204-v8_gyp.patch
index 8496a7c71..0a12e67b8 100644
--- a/lang/node/patches/204-v8_gyp.patch
+++ b/lang/node/patches/204-v8_gyp.patch
@@ -28,7 +28,7 @@
'sources': [
'<(V8_ROOT)/src/init/setup-isolate-full.cc',
],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
}, # v8_init
{
# This target is used to work around a GCC issue that causes the
@@ -40,7 +40,7 @@
'include_dirs': [
'<(SHARED_INTERMEDIATE_DIR)',
'<(generate_bytecode_output_root)',
-+ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
++ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
],
'sources': [
'<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "\\"v8_initializers.*?sources = ")',
@@ -48,7 +48,7 @@
'toolsets': ['host', 'target'],
'direct_dependent_settings': {
'sources': ['<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "v8_compiler_sources = ")'],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'conditions': [
['v8_target_arch=="ia32"', {
'sources': [
@@ -57,7 +57,7 @@
'type': 'static_library',
'toolsets': ['host', 'target'],
+ 'libraries!':[ '-lcrypto', '-lssl', '-lz', '-lhttp_parser', '-luv', '-lnghttp2', '-lcares' ],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'dependencies': [
'generate_bytecode_builtins_list',
'run_torque',
@@ -65,7 +65,7 @@
'run_torque',
'v8_maybe_icu',
],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'conditions': [
['(is_component_build and not v8_optimized_debug and v8_enable_fast_mksnapshot) or v8_enable_turbofan==0', {
'dependencies': [
@@ -99,7 +99,7 @@
'type': 'executable',
+ 'libraries!':[ '-lcrypto', '-lssl', '-lz', '-lhttp_parser', '-luv', '-lnghttp2', '-lcares' ],
+ 'library_dirs':[ '../../../../staging_dir/hostpkg/share/icu/current/lib' ],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'dependencies': [
'v8_base_without_compiler',
'v8_compiler_for_mksnapshot',
@@ -144,7 +144,7 @@
'include_dirs': [
'<(V8_ROOT)/third_party/zlib',
'<(V8_ROOT)/third_party/zlib/google',
-+ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
++ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
],
},
'defines': [ 'ZLIB_IMPLEMENTATION' ],
@@ -152,7 +152,21 @@
'include_dirs': [
'<(V8_ROOT)/third_party/zlib',
'<(V8_ROOT)/third_party/zlib/google',
-+ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
++ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
],
'sources': [
'<(V8_ROOT)/third_party/zlib/adler32.c',
+@@ -2229,10 +2254,13 @@
+ 'ABSEIL_ROOT': '../../deps/v8/third_party/abseil-cpp',
+ },
+ 'direct_dependent_settings': {
++ 'include_dirs!': [ '<!@(echo "$STAGING_DIR"/usr/include)' ],
+ 'include_dirs': [
+ '<(ABSEIL_ROOT)',
++ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
+ ],
+ },
++ 'include_dirs!': [ '<!@(echo "$STAGING_DIR"/usr/include)' ],
+ 'include_dirs': [
+ '<(ABSEIL_ROOT)',
+ ], I will resume testing. |
This PR seems doesn't work but patch above by @nxhack is working. |
I have confirmed that it builds and runs on aarch64, arm, i386, x86_64 with my patch applied. |
Hmm, can you please try with glibc on x86_64? |
I am verifying a simplified version of the above patch. musl does not give me any problems at all, but glibc gives me the following error. I will consider a countermeasure.
|
With this patch, glibc seems to be ok. diff --git a/lang/node/patches/204-v8_gyp.patch b/lang/node/patches/204-v8_gyp.patch
index 8496a7c71..5dba48a4f 100644
--- a/lang/node/patches/204-v8_gyp.patch
+++ b/lang/node/patches/204-v8_gyp.patch
@@ -28,7 +28,7 @@
'sources': [
'<(V8_ROOT)/src/init/setup-isolate-full.cc',
],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
}, # v8_init
{
# This target is used to work around a GCC issue that causes the
@@ -40,7 +40,7 @@
'include_dirs': [
'<(SHARED_INTERMEDIATE_DIR)',
'<(generate_bytecode_output_root)',
-+ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
++ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
],
'sources': [
'<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "\\"v8_initializers.*?sources = ")',
@@ -48,7 +48,7 @@
'toolsets': ['host', 'target'],
'direct_dependent_settings': {
'sources': ['<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "v8_compiler_sources = ")'],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'conditions': [
['v8_target_arch=="ia32"', {
'sources': [
@@ -57,7 +57,7 @@
'type': 'static_library',
'toolsets': ['host', 'target'],
+ 'libraries!':[ '-lcrypto', '-lssl', '-lz', '-lhttp_parser', '-luv', '-lnghttp2', '-lcares' ],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'dependencies': [
'generate_bytecode_builtins_list',
'run_torque',
@@ -65,7 +65,7 @@
'run_torque',
'v8_maybe_icu',
],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'conditions': [
['(is_component_build and not v8_optimized_debug and v8_enable_fast_mksnapshot) or v8_enable_turbofan==0', {
'dependencies': [
@@ -99,7 +99,7 @@
'type': 'executable',
+ 'libraries!':[ '-lcrypto', '-lssl', '-lz', '-lhttp_parser', '-luv', '-lnghttp2', '-lcares' ],
+ 'library_dirs':[ '../../../../staging_dir/hostpkg/share/icu/current/lib' ],
-+ 'include_dirs': [ '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
++ 'include_dirs': [ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)' ],
'dependencies': [
'v8_base_without_compiler',
'v8_compiler_for_mksnapshot',
@@ -144,7 +144,7 @@
'include_dirs': [
'<(V8_ROOT)/third_party/zlib',
'<(V8_ROOT)/third_party/zlib/google',
-+ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
++ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
],
},
'defines': [ 'ZLIB_IMPLEMENTATION' ],
@@ -152,7 +152,15 @@
'include_dirs': [
'<(V8_ROOT)/third_party/zlib',
'<(V8_ROOT)/third_party/zlib/google',
-+ '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
++ '../../deps/v8/third_party/abseil-cpp', '<!@(echo "$STAGING_DIR"/usr/../usr/include)',
],
'sources': [
'<(V8_ROOT)/third_party/zlib/adler32.c',
+@@ -2228,6 +2253,7 @@
+ 'variables': {
+ 'ABSEIL_ROOT': '../../deps/v8/third_party/abseil-cpp',
+ },
++ 'include_dirs!': [ '<!@(echo "$STAGING_DIR"/usr/include)' ],
+ 'direct_dependent_settings': {
+ 'include_dirs': [
+ '<(ABSEIL_ROOT)', |
@nxhack |
I have applied my recent patch and confirmed it works on aarch64, arm, i386, x86_64 and x86_64+glibc. |
@commodo This change should be reverted, @nxhack's patch #25582 (comment) should be submitted instead. Sorry for the confusion. |
ok; please open a PR for this; |
This reverts commit 9e70d3c. As mentioned here: openwrt#25582 (comment) Signed-off-by: Alexandru Ardelean <[email protected]>
This reverts commit 9e70d3c. As mentioned here: openwrt#25582 (comment) Signed-off-by: Alexandru Ardelean <[email protected]>
This reverts commit 9e70d3c. As mentioned here: #25582 (comment) Signed-off-by: Alexandru Ardelean <[email protected]>
Please do the honour for submitting the right PR. |
Okay. I will work on it. |
remake this commit openwrt#25582 Fix nodejs compile errors after the introduction of abseil-cpp in openwrt#25565 Signed-off-by: Hirokazu MORIKAWA <[email protected]>
remake this commit #25582 Fix nodejs compile errors after the introduction of abseil-cpp in #25565 Signed-off-by: Hirokazu MORIKAWA <[email protected]>
Maintainer: @nxhack
Compile tested: OpenWRT One master/snapshot
Description:
Fix nodejs compile errors after the introduction of
abseil-cpp
in #25565Extends the existing gyp_tools patch to force v8 to use the bundled (chromium) abseil-cpp.
Resolves #25483
Thanks to @vortexilation for pointing out this issue.