Skip to content

Revert "build,test: add duplicate symbol test"#1828

Merged
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:revert-2761afb
Jul 16, 2019
Merged

Revert "build,test: add duplicate symbol test"#1828
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:revert-2761afb

Conversation

@bnoordhuis
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis commented Jul 16, 2019

This reverts commit 2761afb.

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.

Refs: nodejs/node#28647 (comment)

cc @gabrielschulhof

CI: https://ci.nodejs.org/job/nodegyp-test-pull-request/144/

@rvagg
Copy link
Copy Markdown
Member

rvagg commented Jul 16, 2019

Will get a patch release out as soon as this lands.

We have a semver-major queued so I'll have to leave that one out #1818.

This reverts commit 2761afb.

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: #1828
Refs: nodejs/node#28647 (comment)
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bnoordhuis bnoordhuis merged commit c9a86fe into nodejs:master Jul 16, 2019
@bnoordhuis bnoordhuis deleted the revert-2761afb branch July 16, 2019 17:20
rvagg pushed a commit that referenced this pull request Jul 17, 2019
This reverts commit 2761afb.

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: #1828
Refs: nodejs/node#28647 (comment)
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@rvagg
Copy link
Copy Markdown
Member

rvagg commented Jul 17, 2019

this commit became 0878db3 when I reordered for the v5.0.3 release as per #1829

bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Jan 12, 2020
Upcoming changes to node-gyp will turn on `-fvisibility=hidden` on
macOS. Ensure that public symbols that are dlsym'd have default
visibility.

Refs: nodejs#28647
Refs: nodejs/node-gyp#1828
Trott pushed a commit to Trott/io.js that referenced this pull request Jan 18, 2020
Upcoming changes to node-gyp will turn on `-fvisibility=hidden` on
macOS. Ensure that public symbols that are dlsym'd have default
visibility.

Refs: nodejs#28647
Refs: nodejs/node-gyp#1828

PR-URL: nodejs#28717
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere pushed a commit to nodejs/node that referenced this pull request Feb 17, 2020
Upcoming changes to node-gyp will turn on `-fvisibility=hidden` on
macOS. Ensure that public symbols that are dlsym'd have default
visibility.

Refs: #28647
Refs: nodejs/node-gyp#1828

PR-URL: #28717
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere pushed a commit to nodejs/node that referenced this pull request Mar 14, 2020
Upcoming changes to node-gyp will turn on `-fvisibility=hidden` on
macOS. Ensure that public symbols that are dlsym'd have default
visibility.

Refs: #28647
Refs: nodejs/node-gyp#1828

PR-URL: #28717
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere pushed a commit to nodejs/node that referenced this pull request Mar 17, 2020
Upcoming changes to node-gyp will turn on `-fvisibility=hidden` on
macOS. Ensure that public symbols that are dlsym'd have default
visibility.

Refs: #28647
Refs: nodejs/node-gyp#1828

PR-URL: #28717
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants