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

src: add HAVE_OPENSSL directive to openssl_config #11734

Closed

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Mar 7, 2017

Currently when building with the following configuration options:
$ ./configure --without-ssl && make

The following link error is reported:

Undefined symbols for architecture x86_64:
"node::openssl_config", referenced from:
node::Init(int*, char const**, int*, char const***) in node.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see
invocation)

Adding an HAVE_OPENSSL directive around this code allows the build to
pass.

PR-URL: #11618
Reviewed-By: Anna Henningsen [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Colin Ihrig [email protected]
Reviewed-By: Ben Noordhuis [email protected]
Reviewed-By: Sam Roberts [email protected]

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

src

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v7.x labels Mar 7, 2017
@sam-github
Copy link
Contributor

@gibfahn @gdams @nodejs/build what would it take to get a build job added, linux only would be ok, to build and test without openssl? There is a steady stream of fixes coming in for code and tests that don't work properly when node isn't built with openssl support, it would be good to catch these problems during PR not after the fact.

@gibfahn
Copy link
Member

gibfahn commented Mar 7, 2017

@sam-github if we were only building/testing on one platform it shouldn't be too hard, and if we run it on one of our quicker platforms it shouldn't slow down the CI run (our quickest job builds and tests in under 3 minutes).

I'll raise an issue on the build repo to track adding the job.

EDIT: nodejs/build#643

Currently when building with the following configuration options:
$ ./configure --without-ssl && make

The following link error is reported:

Undefined symbols for architecture x86_64:
  "node::openssl_config", referenced from:
      node::Init(int*, char const**, int*, char const***) in node.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see
invocation)

Adding an HAVE_OPENSSL directive around this code allows the build to
pass.

PR-URL: nodejs#11618
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@danbev danbev force-pushed the add-have_openssl-directive branch from bfc6d25 to 6ecb1cb Compare May 30, 2017 05:35
@danbev
Copy link
Contributor Author

danbev commented May 30, 2017

@Trott
Copy link
Member

Trott commented Aug 10, 2017

I'm going to close this because it targets v7.x and there's no more releases planned for that release line. If I'm mistaken here about something somehow, of course, just re-open and apologies in advance.

@Trott Trott closed this Aug 10, 2017
@danbev danbev deleted the add-have_openssl-directive branch November 16, 2017 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants