Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: export OpenSSL UI symbols #27586

Closed

Conversation

sam-github
Copy link
Contributor

@sam-github sam-github commented May 6, 2019

Node.js compiles them, their existence is indicated by OpenSSL header
defines, but they can't be linked to on Windows because their symbols
are not exported. Export them.

Fixes: #27494

See #27494 (comment) for more info.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label May 6, 2019
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform. labels May 6, 2019
@mscdex
Copy link
Contributor

mscdex commented May 6, 2019

I think build might be a better subsystem prefix for the commit message.

Node.js compiles them, their existence is indicated by OpenSSL header
defines, but they can't be linked to on Windows because their symbols
are not exported. Export them.

Fixes: nodejs#27494
@sam-github sam-github force-pushed the export-openssl-ui-symbols branch from 4e40c1d to 666a13d Compare May 6, 2019 21:22
@nodejs-github-bot
Copy link
Collaborator

@mscdex mscdex changed the title tls: export OpenSSL UI symbols build: export OpenSSL UI symbols May 6, 2019
@nkochakian
Copy link

Looks good. This matches the change I made when testing.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sam-github
Copy link
Contributor Author

Landed in b9a3103

@sam-github sam-github closed this May 9, 2019
@sam-github sam-github deleted the export-openssl-ui-symbols branch May 9, 2019 19:22
sam-github added a commit that referenced this pull request May 9, 2019
Node.js compiles them, their existence is indicated by OpenSSL header
defines, but they can't be linked to on Windows because their symbols
are not exported. Export them.

Fixes: #27494

PR-URL: #27586
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request May 10, 2019
Node.js compiles them, their existence is indicated by OpenSSL header
defines, but they can't be linked to on Windows because their symbols
are not exported. Export them.

Fixes: #27494

PR-URL: #27586
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@BridgeAR BridgeAR mentioned this pull request May 21, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenSSL New User Interface functions not exported in the Windows version of Node 10