From 45bd6ff842832aa2ca5977ed4e4b71a18f06237a Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 15 Jul 2022 16:57:29 -0700 Subject: [PATCH] fix: do not define _LIBCPP_ABI_NAMESPACE=Cr for all native modules (#34932) This define is only needed when linking against Chromiums libc++ which we currently do not ship / expose the symbols of. We probably should make those symbols visible and actually ensure that electron-rebuild et. al link against our libc++ instead of the system libc++ but for now this fixes compilation issues on macOS where the default system clang links to the system libc++ which does not (obviously) use the Chromium ABI namespace. For our nan tests which do link against Chromiums libc++ we define the ABI namespace in the spec runner. --- patches/node/.patches | 1 - ...mespace_as_cr_to_align_with_chromium.patch | 21 ------------------- script/nan-spec-runner.js | 1 + 3 files changed, 1 insertion(+), 22 deletions(-) delete mode 100644 patches/node/build_define_libcpp_abi_namespace_as_cr_to_align_with_chromium.patch diff --git a/patches/node/.patches b/patches/node/.patches index af17227eaf3b9..16fac1923caf4 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -44,4 +44,3 @@ src_update_importmoduledynamically.patch fix_add_v8_enable_reverse_jsargs_defines_in_common_gypi.patch json_parse_errors_made_user-friendly.patch build_ensure_v8_pointer_compression_sandbox_is_enabled_on_64bit.patch -build_define_libcpp_abi_namespace_as_cr_to_align_with_chromium.patch diff --git a/patches/node/build_define_libcpp_abi_namespace_as_cr_to_align_with_chromium.patch b/patches/node/build_define_libcpp_abi_namespace_as_cr_to_align_with_chromium.patch deleted file mode 100644 index 4935c302868f4..0000000000000 --- a/patches/node/build_define_libcpp_abi_namespace_as_cr_to_align_with_chromium.patch +++ /dev/null @@ -1,21 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Samuel Attard -Date: Mon, 6 Jun 2022 14:46:40 -0700 -Subject: build: define _LIBCPP_ABI_NAMESPACE as Cr to align with chromium - -Without this define native modules will be built trying to link to _LIBCPP_ABI_NAMESPACE which is the default name, chromium overrides this to Cr for PDB size reasons but they override it on all platforms. Setting this define allows native modules to actually work. This should not be upstreamed as it is Electron specific. - -Refs: https://chromium-review.googlesource.com/c/chromium/src/+/3655638 - -diff --git a/common.gypi b/common.gypi -index e20092d15d5f71f3e90a2ce655d660a8fa1e1385..242ff08b581c143018046618e539bec12ac566f1 100644 ---- a/common.gypi -+++ b/common.gypi -@@ -293,6 +293,7 @@ - 'V8_DEPRECATION_WARNINGS', - 'V8_IMMINENT_DEPRECATION_WARNINGS', - '_GLIBCXX_USE_CXX11_ABI=1', -+ '_LIBCPP_ABI_NAMESPACE=Cr', - ], - - # Forcibly disable -Werror. We support a wide range of compilers, it's diff --git a/script/nan-spec-runner.js b/script/nan-spec-runner.js index 5672af3322fa0..0f1c6a7ff0ec6 100644 --- a/script/nan-spec-runner.js +++ b/script/nan-spec-runner.js @@ -62,6 +62,7 @@ async function main () { `-isystem"${path.resolve(BASE, 'buildtools', 'third_party', 'libc++', 'trunk', 'include')}"`, `-isystem"${path.resolve(BASE, 'buildtools', 'third_party', 'libc++abi', 'trunk', 'include')}"`, '-fPIC', + '-D_LIBCPP_ABI_NAMESPACE=Cr', ...platformFlags ].join(' ');