From 0878db3b81a79ead9c62f0562c9db143726d607d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 16 Jul 2019 10:22:38 +0200 Subject: [PATCH] Revert "build,test: add duplicate symbol test" This reverts commit 2761afbf730f7001675bbdbfa4a9e4821243adcb. Building with `-fvisibility=hidden` breaks some of Node's add-on tests and therefore likely also affects third-party add-ons. This change was landed in a patch release so I'm opting to revert it until the next major release. PR-URL: https://github.com/nodejs/node-gyp/pull/1828 Refs: https://github.com/nodejs/node/pull/28647#issuecomment-511715968 Reviewed-By: Richard Lau Reviewed-By: Rod Vagg --- addon.gypi | 4 -- .../node_modules/duplicate_symbols/binding.cc | 10 ----- .../duplicate_symbols/binding.gyp | 19 ---------- test/node_modules/duplicate_symbols/common.h | 37 ------------------- test/node_modules/duplicate_symbols/extra.cc | 6 --- test/node_modules/duplicate_symbols/index.js | 5 --- .../duplicate_symbols/package.json | 14 ------- test/test-addon.js | 24 ------------ 8 files changed, 119 deletions(-) delete mode 100644 test/node_modules/duplicate_symbols/binding.cc delete mode 100644 test/node_modules/duplicate_symbols/binding.gyp delete mode 100644 test/node_modules/duplicate_symbols/common.h delete mode 100644 test/node_modules/duplicate_symbols/extra.cc delete mode 100644 test/node_modules/duplicate_symbols/index.js delete mode 100644 test/node_modules/duplicate_symbols/package.json diff --git a/addon.gypi b/addon.gypi index 3cadaab78b..6462f539ff 100644 --- a/addon.gypi +++ b/addon.gypi @@ -90,14 +90,10 @@ 'conditions': [ [ 'OS=="mac"', { - 'cflags': [ - '-fvisibility=hidden' - ], 'defines': [ '_DARWIN_USE_64_BIT_INODE=1' ], 'xcode_settings': { - 'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES', # -fvisibility=hidden 'DYLIB_INSTALL_NAME_BASE': '@rpath' }, }], diff --git a/test/node_modules/duplicate_symbols/binding.cc b/test/node_modules/duplicate_symbols/binding.cc deleted file mode 100644 index a0999b76dc..0000000000 --- a/test/node_modules/duplicate_symbols/binding.cc +++ /dev/null @@ -1,10 +0,0 @@ -#include -#include "common.h" - -void Init(v8::Local exports) { - Nan::Set(exports, Nan::New("pointerCheck").ToLocalChecked(), - Nan::GetFunction( - Nan::New(Something::PointerCheck)).ToLocalChecked()); -} - -NODE_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/node_modules/duplicate_symbols/binding.gyp b/test/node_modules/duplicate_symbols/binding.gyp deleted file mode 100644 index 46f1c8cba4..0000000000 --- a/test/node_modules/duplicate_symbols/binding.gyp +++ /dev/null @@ -1,19 +0,0 @@ -{ - "target_defaults": { - "include_dirs": [ - " - -class Something { - public: - static void PointerCheck(const Nan::FunctionCallbackInfo& info); -}; - -// Removing the inline keyword below will result in the addon failing to link -// on OSX because of a duplicate symbol. -inline void -Something::PointerCheck(const Nan::FunctionCallbackInfo& info) { - v8::Local v8result; - - if (info.Length() > 0) { - // If an argument was passed in, it is a pointer to the `PointerCheck` - // method from the other addon. So, we compare it to the value of the - // pointer to the `PointerCheck` method in this addon, and return - // `"equal"` if they are equal, and `"not equal"` otherwise". - - const char* result = - (reinterpret_cast(Something::PointerCheck) == - info[0].As()->Value()) ? - "equal" : "not equal"; - v8result = Nan::New(result).ToLocalChecked(); - } else { - // If no argument was passed in, we wrap the pointer to the `PointerCheck` - // method in this addon into a `v8::External` and pass it into JavaScript. - v8result = Nan::New( - reinterpret_cast(Something::PointerCheck)); - } - info.GetReturnValue().Set(v8result); -} - -#endif // DUPLICATE_SYMBOLS_COMMON_H_ diff --git a/test/node_modules/duplicate_symbols/extra.cc b/test/node_modules/duplicate_symbols/extra.cc deleted file mode 100644 index 0c9d5dbebd..0000000000 --- a/test/node_modules/duplicate_symbols/extra.cc +++ /dev/null @@ -1,6 +0,0 @@ -#include "common.h" - -// It is important that common.h be included from two different translation -// units, because doing so can create duplicate symbols in some instances. If -// it does so and fails to build because of it, that is considered a test -// failure. diff --git a/test/node_modules/duplicate_symbols/index.js b/test/node_modules/duplicate_symbols/index.js deleted file mode 100644 index 09dbed81b7..0000000000 --- a/test/node_modules/duplicate_symbols/index.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict' -module.exports = { - pointerCheck1: require('bindings')('binding1').pointerCheck, - pointerCheck2: require('bindings')('binding2').pointerCheck -}; diff --git a/test/node_modules/duplicate_symbols/package.json b/test/node_modules/duplicate_symbols/package.json deleted file mode 100644 index 2a193f230f..0000000000 --- a/test/node_modules/duplicate_symbols/package.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "name": "duplicate_symbols", - "version": "0.0.0", - "description": "Duplicate Symbols Test", - "main": "index.js", - "private": true, - "dependencies": { - "bindings": "~1.2.1", - "nan": "^2.14.0" - }, - "scripts": { - "test": "node index.js" - } -} diff --git a/test/test-addon.js b/test/test-addon.js index 49e30164c3..f97215c0a2 100644 --- a/test/test-addon.js +++ b/test/test-addon.js @@ -18,15 +18,6 @@ function runHello (hostProcess) { return execFileSync(hostProcess, [ '-e', testCode ], { cwd: __dirname }).toString() } -function runDuplicateBindings () { - const hostProcess = process.execPath - var testCode = - 'console.log((function(bindings) {' + - 'return bindings.pointerCheck1(bindings.pointerCheck2());' + - "})(require('duplicate_symbols')))" - return execFileSync(hostProcess, [ '-e', testCode ], { cwd: __dirname }).toString() -} - function getEncoding () { var code = 'import locale;print(locale.getdefaultlocale()[1])' return execFileSync('python', [ '-c', code ]).toString().trim() @@ -60,21 +51,6 @@ test('build simple addon', function (t) { proc.stderr.setEncoding('utf-8') }) -test('make sure addon symbols do not overlap', function (t) { - t.plan(3) - - var addonPath = path.resolve(__dirname, 'node_modules', 'duplicate_symbols') - // Set the loglevel otherwise the output disappears when run via 'npm test' - var cmd = [nodeGyp, 'rebuild', '-C', addonPath, '--loglevel=verbose'] - execFile(process.execPath, cmd, function (err, stdout, stderr) { - var logLines = stderr.trim().split(/\r?\n/) - var lastLine = logLines[logLines.length - 1] - t.strictEqual(err, null) - t.strictEqual(lastLine, 'gyp info ok', 'should end in ok') - t.strictEqual(runDuplicateBindings().trim(), 'not equal') - }) -}) - test('build simple addon in path with non-ascii characters', function (t) { t.plan(1)