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

(v7.x backport) src,test: --without ssl fixes #12332

Closed
wants to merge 3 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 11, 2017

These three commit are backports required to allow the build and test to be run when configured with --without-ssl.

I thought it made sense to include them all in one pull request, but let me know if it is preferred to have each one in a separate PR and I'll just point out where there is a dependency (should only be danbev@fe17e9b which is required to be landed first as the will be a link error otherwise)

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

src, test

danbev added 3 commits April 11, 2017 07:43
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 <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Currently when node is build --without-ssl and the test are run,
there are a number of failing test due to tests expecting crypto
support to be available. This commit fixes fixes the failure and
instead skips the tests that expect crypto to be available.

PR-URL: nodejs#11631
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
I'm currently seeing a timeout error for test-signal-handler.js on
macosx when using the following configuration:

./configure --debug --without-ssl && make -j8 test

--without-ssl implies that there will be no inspector but the signal
SIGUSR1 is blocked in PlatformInit just the same. But in this case
never unblocked which is causing the signal to never be delivered to
the handlers in test-signal-handler.js and it loops until it times out.

Not sure if this is the best way of fixing this but hopefully more eyes
on this will help.

PR-URL: nodejs#12266
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Apr 11, 2017
@mscdex mscdex added inspector Issues and PRs related to the V8 inspector protocol openssl Issues and PRs related to the OpenSSL dependency. labels Apr 11, 2017
@sam-github
Copy link
Contributor

I think having them in one PR is fine for now, since its still undecided exactly how/if commit metadata should be rewritten for backports when they land.

@Trott
Copy link
Member

Trott commented Jul 15, 2017

Unfortunately, there are no more 7.x releases planned. Sorry this was never landed! I'm going to close it now.

@Trott Trott closed this Jul 15, 2017
@refack refack added the embedding Issues and PRs related to embedding Node.js in another project. label Aug 17, 2017
@danbev danbev deleted the v7.x-without-ssl-fixes branch February 28, 2018 07:27
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++. embedding Issues and PRs related to embedding Node.js in another project. inspector Issues and PRs related to the V8 inspector protocol openssl Issues and PRs related to the OpenSSL dependency. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants