From 8b2011a818a3ad324f4b44cc811fea8e56e2f15d Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Sat, 4 May 2024 14:55:06 +0100 Subject: [PATCH] build: define `NOMINMAX` in common.gypi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V8 and Node.js had defined `NOMINMAX` on Windows for a long time. In recent changes, V8 added `std::numeric_limits::min` usages in its header files which caused addons without `NOMINMAX` defines failed to compile. Define `NOMINMAX` in common.gypi so that addons can be compiled with the latest V8 header files. PR-URL: https://github.com/nodejs/node/pull/52794 Fixes: https://github.com/nodejs/nan/issues/968 Refs: https://github.com/nodejs/gyp-next/pull/244 Reviewed-By: Michaƫl Zasso Reviewed-By: Mohammed Keyvanzadeh Reviewed-By: Yagiz Nizipli Reviewed-By: Luigi Pinca Reviewed-By: Jiawen Geng --- common.gypi | 4 ++++ node.gypi | 4 ---- test/addons/hello-world/binding.gyp | 5 +++++ test/addons/hello-world/binding2.cc | 21 +++++++++++++++++++++ test/addons/hello-world/test.js | 3 +++ 5 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 test/addons/hello-world/binding2.cc diff --git a/common.gypi b/common.gypi index d6f024ab93a365..694f15ebd23c61 100644 --- a/common.gypi +++ b/common.gypi @@ -458,6 +458,10 @@ '_HAS_EXCEPTIONS=0', 'BUILDING_V8_SHARED=1', 'BUILDING_UV_SHARED=1', + # Stop from defining macros that conflict with + # std::min() and std::max(). We don't use (much) + # but we still inherit it from uv.h. + 'NOMINMAX', ], }], [ 'OS in "linux freebsd openbsd solaris aix os400"', { diff --git a/node.gypi b/node.gypi index 95133818dff7c7..e3c125fb9ec7f8 100644 --- a/node.gypi +++ b/node.gypi @@ -63,10 +63,6 @@ 'FD_SETSIZE=1024', # we need to use node's preferred "win32" rather than gyp's preferred "win" 'NODE_PLATFORM="win32"', - # Stop from defining macros that conflict with - # std::min() and std::max(). We don't use (much) - # but we still inherit it from uv.h. - 'NOMINMAX', '_UNICODE=1', ], 'msvs_precompiled_header': 'tools/msvs/pch/node_pch.h', diff --git a/test/addons/hello-world/binding.gyp b/test/addons/hello-world/binding.gyp index 55fbe7050f18e4..a445af3d1cd18e 100644 --- a/test/addons/hello-world/binding.gyp +++ b/test/addons/hello-world/binding.gyp @@ -4,6 +4,11 @@ 'target_name': 'binding', 'sources': [ 'binding.cc' ], 'includes': ['../common.gypi'], + }, + { + 'target_name': 'binding2', + 'sources': [ 'binding2.cc' ], + 'includes': ['../common.gypi'], } ] } diff --git a/test/addons/hello-world/binding2.cc b/test/addons/hello-world/binding2.cc new file mode 100644 index 00000000000000..b94aecb3c76b26 --- /dev/null +++ b/test/addons/hello-world/binding2.cc @@ -0,0 +1,21 @@ +// Include uv.h and v8.h ahead of node.h to verify that node.h doesn't need to +// be included first. Disable clang-format as it will sort the include lists. +// clang-format off +#include +#include +#include +// clang-format on + +static void Method(const v8::FunctionCallbackInfo& args) { + v8::Isolate* isolate = args.GetIsolate(); + args.GetReturnValue().Set( + v8::String::NewFromUtf8(isolate, "world").ToLocalChecked()); +} + +static void InitModule(v8::Local exports, + v8::Local module, + v8::Local context) { + NODE_SET_METHOD(exports, "hello", Method); +} + +NODE_MODULE(NODE_GYP_MODULE_NAME, InitModule) diff --git a/test/addons/hello-world/test.js b/test/addons/hello-world/test.js index d1d67d34f688c9..79f03f44be857a 100644 --- a/test/addons/hello-world/test.js +++ b/test/addons/hello-world/test.js @@ -6,6 +6,9 @@ const binding = require(bindingPath); assert.strictEqual(binding.hello(), 'world'); console.log('binding.hello() =', binding.hello()); +const binding2 = require(require.resolve(`./build/${common.buildType}/binding2`)); +assert.strictEqual(binding2.hello(), 'world'); + // Test multiple loading of the same module. delete require.cache[bindingPath]; const rebinding = require(bindingPath);