From b9ab8bad5fcd3ff50f158cd0b1412dc6ce667492 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 18 Feb 2022 16:42:40 +0100 Subject: [PATCH] src: enable wrapping Napi namespace with custom namespace Enable support for a `NAPI_CPP_CUSTOM_NAMESPACE` that would wrap the `Napi` namespace in a custom namespace. This can be helpful when there are multiple parts of a compiled shared library or executable that depend on `node-addon-api`, but different versions of it. In order to avoid symbol conflicts in these cases, namespacing can be used to make sure that the linker would not merge definitions that the compiler potentially generates for the node-addon-api methods. --- napi-inl.h | 8 ++++++ napi.h | 16 +++++++++++ package.json | 1 - test/binding.gyp | 6 +++++ test/common/index.js | 6 +++-- test/common/test_helper.h | 10 +++++++ test/error_terminating_environment.js | 1 + unit-test/binding.gyp | 39 ++++++++++++++++----------- 8 files changed, 68 insertions(+), 19 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 64d4f5aa0..13391187b 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -18,6 +18,10 @@ namespace Napi { +#ifdef NAPI_CPP_CUSTOM_NAMESPACE +namespace NAPI_CPP_CUSTOM_NAMESPACE { +#endif + // Helpers to handle functions exposed from C++. namespace details { @@ -6213,6 +6217,10 @@ bool Env::CleanupHook::IsEmpty() const { } #endif // NAPI_VERSION > 2 +#ifdef NAPI_CPP_CUSTOM_NAMESPACE +} // namespace NAPI_CPP_CUSTOM_NAMESPACE +#endif + } // namespace Napi #endif // SRC_NAPI_INL_H_ diff --git a/napi.h b/napi.h index 429e6a622..c65b8e852 100644 --- a/napi.h +++ b/napi.h @@ -140,6 +140,18 @@ static_assert(sizeof(char16_t) == sizeof(wchar_t), "Size mismatch between char16 //////////////////////////////////////////////////////////////////////////////// namespace Napi { +#ifdef NAPI_CPP_CUSTOM_NAMESPACE +// NAPI_CPP_CUSTOM_NAMESPACE can be #define'd per-addon to avoid symbol +// conflicts between different instances of node-addon-api + +// First dummy definition of the namespace to make sure that Napi::(name) still +// refers to the right things inside this file. +namespace NAPI_CPP_CUSTOM_NAMESPACE {} +using namespace NAPI_CPP_CUSTOM_NAMESPACE; + +namespace NAPI_CPP_CUSTOM_NAMESPACE { +#endif + // Forward declarations class Env; class Value; @@ -2977,6 +2989,10 @@ namespace Napi { }; #endif // NAPI_VERSION > 5 +#ifdef NAPI_CPP_CUSTOM_NAMESPACE + } // namespace NAPI_CPP_CUSTOM_NAMESPACE +#endif + } // namespace Napi // Inline implementations of all the above class methods are included here. diff --git a/package.json b/package.json index fb74a1c0f..68c5c778d 100644 --- a/package.json +++ b/package.json @@ -323,7 +323,6 @@ "name": "WenheLI", "url": "https://github.com/WenheLI" }, - { "name": "Yohei Kishimoto", "url": "https://github.com/morokosi" diff --git a/test/binding.gyp b/test/binding.gyp index a0470bde5..b6aca5710 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -114,5 +114,11 @@ 'sources': ['>@(build_sources_swallowexcept)'], 'defines': ['NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS'] }, + { + 'target_name': 'binding_custom_namespace', + 'includes': ['../noexcept.gypi'], + 'sources': ['>@(build_sources)'], + 'defines': ['NAPI_CPP_CUSTOM_NAMESPACE=cstm'] + }, ], } diff --git a/test/common/index.js b/test/common/index.js index 90d02b108..f28121549 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -81,7 +81,8 @@ exports.runTest = async function (test, buildType, buildPathRoot = process.env.B const bindings = [ path.join(buildPathRoot, `../build/${buildType}/binding.node`), path.join(buildPathRoot, `../build/${buildType}/binding_noexcept.node`), - path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`) + path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`), + path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`) ].map(it => require.resolve(it)); for (const item of bindings) { @@ -96,7 +97,8 @@ exports.runTestWithBindingPath = async function (test, buildType, buildPathRoot const bindings = [ path.join(buildPathRoot, `../build/${buildType}/binding.node`), path.join(buildPathRoot, `../build/${buildType}/binding_noexcept.node`), - path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`) + path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`), + path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`) ].map(it => require.resolve(it)); for (const item of bindings) { diff --git a/test/common/test_helper.h b/test/common/test_helper.h index 1b321a19e..88ab1e8e0 100644 --- a/test/common/test_helper.h +++ b/test/common/test_helper.h @@ -3,6 +3,12 @@ namespace Napi { +// Needs this here since the MaybeUnwrap() functions need to be in the +// same namespace as their arguments for C++ argument-dependent lookup +#ifdef NAPI_CPP_CUSTOM_NAMESPACE +namespace NAPI_CPP_CUSTOM_NAMESPACE { +#endif + // Use this when a variable or parameter is unused in order to explicitly // silence a compiler warning about that. template @@ -58,4 +64,8 @@ inline bool MaybeUnwrapTo(MaybeOrValue maybe, T* out) { #endif } +#ifdef NAPI_CPP_CUSTOM_NAMESPACE +} // namespace NAPI_CPP_CUSTOM_NAMESPACE +#endif + } // namespace Napi diff --git a/test/error_terminating_environment.js b/test/error_terminating_environment.js index 63b42259e..37ead4e22 100644 --- a/test/error_terminating_environment.js +++ b/test/error_terminating_environment.js @@ -70,6 +70,7 @@ test(`./build/${buildType}/binding.node`, true); test(`./build/${buildType}/binding_noexcept.node`, true); test(`./build/${buildType}/binding_swallowexcept.node`, false); test(`./build/${buildType}/binding_swallowexcept_noexcept.node`, false); +test(`./build/${buildType}/binding_custom_namespace.node`, true); function test(bindingPath, process_should_abort) { const number_of_test_cases = 5; diff --git a/unit-test/binding.gyp b/unit-test/binding.gyp index 8d4a92cc9..f701c9296 100644 --- a/unit-test/binding.gyp +++ b/unit-test/binding.gyp @@ -11,23 +11,23 @@ }, 'targets': [ { - "target_name": "generateBindingCC", - "type": "none", - "actions": [ { - "action_name": "generateBindingCC", - "message": "Generating binding cc file", - "outputs": ["generated/binding.cc"], - "conditions": [ - [ "'true'=='true'", { - "inputs": [""], - "action": [ - "node", - "generate-binding-cc.js", + "target_name": "generateBindingCC", + "type": "none", + "actions": [ { + "action_name": "generateBindingCC", + "message": "Generating binding cc file", + "outputs": ["generated/binding.cc"], + "conditions": [ + [ "'true'=='true'", { + "inputs": [""], + "action": [ + "node", + "generate-binding-cc.js", "@(build_sources)'], + 'defines': ['NAPI_CPP_CUSTOM_NAMESPACE=cstm'], + 'dependencies': [ 'generateBindingCC' ] + }, ], }