FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: define `NOMINMAX` in common.gypi · nodejs/node@e1bd53c · GitHub

/ node Public

Commit e1bd53c

Browse files
legendecas
authored and
targos
committed
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 4dc8a38 commit e1bd53c

5 files changed

Lines changed: 33 additions & 4 deletions

File tree

‎common.gypi‎

Lines changed: 4 additions & 0 deletions
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‎

Lines changed: 0 additions & 4 deletions
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‎

Lines changed: 5 additions & 0 deletions
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‎

Lines changed: 21 additions & 0 deletions
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‎

Lines changed: 3 additions & 0 deletions
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)
Back | FazBrowse Home | New Git URL