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: add NetBSD support to opensslconf.h #14313

Closed
wants to merge 1 commit into from

Conversation

rsmarples
Copy link
Contributor

Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.
@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Jul 16, 2017
@mscdex
Copy link
Contributor

mscdex commented Jul 16, 2017

Shouldn't this be submitted upstream instead?

@rsmarples
Copy link
Contributor Author

The comments inside this file imply it's maintained by node? Or is that wrong?
I don't see this file in openssl upstream.

@bnoordhuis
Copy link
Member

Upstream openssl generates it at build time. We have a pregenerated version for convenience.

The define leaks out though because opensslconf.h is included in many places. That includes native add-ons that build against the bundled openssl.

@rsmarples
Copy link
Contributor Author

It seems that upstream just have a BSD-generic64 configure target - they don't have any specific test for FreeBSD or OpenBSD that's missing for NetBSD to generate opensslconf.h as far as I can tell.

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Okay, seems fine if this is all that's needed. Thanks.

As to my comment on the define leaking out, I realize it's not materially different from the existing OPENSSL_LINUX define so objection withdrawn.

rsmarples added a commit to rsmarples/nodegit that referenced this pull request Aug 5, 2017
my node commit: a6fe49b9bdedef513f67009f4de665492d2d3012
upstream PR: nodejs/node#14313
@BridgeAR
Copy link
Member

@bnoordhuis I guess this could land?

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

rubber stamp lgtm

@BridgeAR
Copy link
Member

Landed in 0b0c2ec

@BridgeAR BridgeAR closed this Aug 30, 2017
BridgeAR pushed a commit that referenced this pull request Aug 30, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: #14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Aug 31, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: nodejs#14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 5, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: nodejs/node#14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: #14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: #14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rsmarples rsmarples deleted the openssl-netbsd branch September 12, 2017 16:47
MylesBorins pushed a commit that referenced this pull request Sep 20, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: #14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
MylesBorins pushed a commit that referenced this pull request Sep 26, 2017
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: #14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants