From 6c29ab10a4d6cb91afdee1745bf34458c9d55dd3 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Mon, 1 Aug 2022 11:44:25 -0700 Subject: [PATCH] fix: ensure native modules use the correct config (#35160) * fix: ensure native modules are built with config.gypi This works by patching node.h to check that two defines are set using the equivilant of an XNOR operation. One define "ELECTRON_ENSURE_CONFIG_GYPI" is set via common.gypi which is _already_ used to build native modules and has been since the dawn of time. Therefore this define will be set for all native module compilations targetting the Electron runtime. The second define "USING_ELECTRON_CONFIG_GYPI" is only defined when the gypi argument "using_electron_config_gypi" is set to 1 which is only done so via config.gypi. Only new enough versions of node-gyp correctly use the config.gypi file thus resulting in a compilation error on version of node-gyp that are too old. * chore: fix lint * chore: update patches Co-authored-by: Samuel Attard Co-authored-by: VerteDinde Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> --- BUILD.gn | 1 + patches/node/.patches | 1 + ...compilation_fails_if_not_using_a_new.patch | 70 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 patches/node/build_ensure_native_module_compilation_fails_if_not_using_a_new.patch diff --git a/BUILD.gn b/BUILD.gn index 4e40d2431753a..0d1401ea4e5aa 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -233,6 +233,7 @@ action("electron_js2c") { action("generate_config_gypi") { outputs = [ "$root_gen_dir/config.gypi" ] script = "script/generate-config-gypi.py" + inputs = [ "//third_party/electron_node/configure.py" ] args = rebase_path(outputs) + [ target_cpu ] } diff --git a/patches/node/.patches b/patches/node/.patches index 16fac1923caf4..daf199eed630f 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -44,3 +44,4 @@ 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_ensure_native_module_compilation_fails_if_not_using_a_new.patch diff --git a/patches/node/build_ensure_native_module_compilation_fails_if_not_using_a_new.patch b/patches/node/build_ensure_native_module_compilation_fails_if_not_using_a_new.patch new file mode 100644 index 0000000000000..101e1da9ba1d5 --- /dev/null +++ b/patches/node/build_ensure_native_module_compilation_fails_if_not_using_a_new.patch @@ -0,0 +1,70 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +Date: Wed, 13 Jul 2022 13:56:12 -0700 +Subject: build: ensure native module compilation fails if not using a new + enough builder + +This should not be upstreamed, it is a quality-of-life patch for downstream module builders. + +diff --git a/common.gypi b/common.gypi +index fe9c96722661071d4497e57d5a8d9ae7d6824159..6bf1b3395250d05cad215bd56f6efea9abc7a7aa 100644 +--- a/common.gypi ++++ b/common.gypi +@@ -85,6 +85,8 @@ + + 'v8_enable_reverse_jsargs%': 1, + ++ 'using_electron_config_gypi%': 0, ++ + ##### end V8 defaults ##### + + # When building native modules using 'npm install' with the system npm, +@@ -290,6 +292,7 @@ + 'V8_DEPRECATION_WARNINGS', + 'V8_IMMINENT_DEPRECATION_WARNINGS', + '_GLIBCXX_USE_CXX11_ABI=1', ++ 'ELECTRON_ENSURE_CONFIG_GYPI', + ], + + # Forcibly disable -Werror. We support a wide range of compilers, it's +@@ -393,6 +396,11 @@ + }], + ], + }], ++ ['using_electron_config_gypi == 1', { ++ 'defines': [ ++ 'USING_ELECTRON_CONFIG_GYPI', ++ ], ++ }], + ['v8_enable_pointer_compression == 1', { + 'defines': [ + 'V8_COMPRESS_POINTERS', +diff --git a/configure.py b/configure.py +index fe741d15da821396883dd72b4e0a42c7758bd557..be7bdfbe75aa70b1797ee01f0506ba60ae740e2d 100755 +--- a/configure.py ++++ b/configure.py +@@ -1421,6 +1421,7 @@ def configure_library(lib, output, pkgname=None): + + + def configure_v8(o): ++ o['variables']['using_electron_config_gypi'] = 1 + o['variables']['v8_enable_webassembly'] = 1 + o['variables']['v8_enable_lite_mode'] = 1 if options.v8_lite_mode else 0 + o['variables']['v8_enable_gdbjit'] = 1 if options.gdb else 0 +diff --git a/src/node.h b/src/node.h +index 0b807cb25f9eb52b2100f0e2a7c25344790967cf..a41b09047d2c499a90225651a8324ad83a7712e3 100644 +--- a/src/node.h ++++ b/src/node.h +@@ -22,6 +22,12 @@ + #ifndef SRC_NODE_H_ + #define SRC_NODE_H_ + ++#ifdef ELECTRON_ENSURE_CONFIG_GYPI ++#ifndef USING_ELECTRON_CONFIG_GYPI ++#error "It looks like you are building this native module without using the right config.gypi. This normally means that you need to update electron-rebuild (>=3.2.8) or node-gyp (>=8.4.0) if you're building modules directly." ++#endif ++#endif ++ + #ifdef _WIN32 + # ifndef BUILDING_NODE_EXTENSION + # define NODE_EXTERN __declspec(dllexport)