Skip to content

Commit 8b2011a

Browse files
authored
build: define NOMINMAX in common.gypi
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: #52794 Fixes: nodejs/nan#968 Refs: nodejs/gyp-next#244 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
1 parent dd5b68d commit 8b2011a

File tree

5 files changed

+33
-4
lines changed

5 files changed

+33
-4
lines changed

common.gypi

+4
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,10 @@
458458
'_HAS_EXCEPTIONS=0',
459459
'BUILDING_V8_SHARED=1',
460460
'BUILDING_UV_SHARED=1',
461+
# Stop <windows.h> from defining macros that conflict with
462+
# std::min() and std::max(). We don't use <windows.h> (much)
463+
# but we still inherit it from uv.h.
464+
'NOMINMAX',
461465
],
462466
}],
463467
[ 'OS in "linux freebsd openbsd solaris aix os400"', {

node.gypi

-4
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@
6363
'FD_SETSIZE=1024',
6464
# we need to use node's preferred "win32" rather than gyp's preferred "win"
6565
'NODE_PLATFORM="win32"',
66-
# Stop <windows.h> from defining macros that conflict with
67-
# std::min() and std::max(). We don't use <windows.h> (much)
68-
# but we still inherit it from uv.h.
69-
'NOMINMAX',
7066
'_UNICODE=1',
7167
],
7268
'msvs_precompiled_header': 'tools/msvs/pch/node_pch.h',

test/addons/hello-world/binding.gyp

+5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44
'target_name': 'binding',
55
'sources': [ 'binding.cc' ],
66
'includes': ['../common.gypi'],
7+
},
8+
{
9+
'target_name': 'binding2',
10+
'sources': [ 'binding2.cc' ],
11+
'includes': ['../common.gypi'],
712
}
813
]
914
}

test/addons/hello-world/binding2.cc

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Include uv.h and v8.h ahead of node.h to verify that node.h doesn't need to
2+
// be included first. Disable clang-format as it will sort the include lists.
3+
// clang-format off
4+
#include <uv.h>
5+
#include <v8.h>
6+
#include <node.h>
7+
// clang-format on
8+
9+
static void Method(const v8::FunctionCallbackInfo<v8::Value>& args) {
10+
v8::Isolate* isolate = args.GetIsolate();
11+
args.GetReturnValue().Set(
12+
v8::String::NewFromUtf8(isolate, "world").ToLocalChecked());
13+
}
14+
15+
static void InitModule(v8::Local<v8::Object> exports,
16+
v8::Local<v8::Value> module,
17+
v8::Local<v8::Context> context) {
18+
NODE_SET_METHOD(exports, "hello", Method);
19+
}
20+
21+
NODE_MODULE(NODE_GYP_MODULE_NAME, InitModule)

test/addons/hello-world/test.js

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ const binding = require(bindingPath);
66
assert.strictEqual(binding.hello(), 'world');
77
console.log('binding.hello() =', binding.hello());
88

9+
const binding2 = require(require.resolve(`./build/${common.buildType}/binding2`));
10+
assert.strictEqual(binding2.hello(), 'world');
11+
912
// Test multiple loading of the same module.
1013
delete require.cache[bindingPath];
1114
const rebinding = require(bindingPath);

0 commit comments

Comments
 (0)