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
base: master
from

Conversation

Projects
None yet
7 participants
@rsmarples
Contributor

rsmarples commented Jul 16, 2017

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

build: add NetBSD support to opensslconf.h
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.
@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex

mscdex Jul 16, 2017

Contributor

Shouldn't this be submitted upstream instead?

Contributor

mscdex commented Jul 16, 2017

Shouldn't this be submitted upstream instead?

@rsmarples

This comment has been minimized.

Show comment
Hide comment
@rsmarples

rsmarples Jul 16, 2017

Contributor

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

Contributor

rsmarples commented Jul 16, 2017

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

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Jul 16, 2017

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.

Member

bnoordhuis commented Jul 16, 2017

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

This comment has been minimized.

Show comment
Hide comment
@rsmarples

rsmarples Jul 16, 2017

Contributor

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.

Contributor

rsmarples commented Jul 16, 2017

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.

@bnoordhuis

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

Fix building OpenSSL on NetBSD.
my node commit: a6fe49b9bdedef513f67009f4de665492d2d3012
upstream PR: nodejs/node#14313
@BridgeAR

This comment has been minimized.

Show comment
Hide comment
@BridgeAR

BridgeAR Aug 28, 2017

Member

@bnoordhuis I guess this could land?

Member

BridgeAR commented Aug 28, 2017

@bnoordhuis I guess this could land?

@jasnell

rubber stamp lgtm

@BridgeAR

This comment has been minimized.

Show comment
Hide comment
@BridgeAR

BridgeAR Aug 30, 2017

Member

Landed in 0b0c2ec

Member

BridgeAR commented Aug 30, 2017

Landed in 0b0c2ec

@BridgeAR BridgeAR closed this Aug 30, 2017

BridgeAR added a commit that referenced this pull request Aug 30, 2017

build: add NetBSD support to opensslconf.h
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 added a commit to cjihrig/node-1 that referenced this pull request Aug 31, 2017

build: add NetBSD support to opensslconf.h
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 added a commit to addaleax/ayo that referenced this pull request Sep 5, 2017

build: add NetBSD support to opensslconf.h
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 added a commit that referenced this pull request Sep 10, 2017

build: add NetBSD support to opensslconf.h
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 referenced this pull request Sep 10, 2017

Merged

v8.5.0 proposal #15308

MylesBorins added a commit that referenced this pull request Sep 12, 2017

build: add NetBSD support to opensslconf.h
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 rsmarples:openssl-netbsd branch Sep 12, 2017

MylesBorins added a commit that referenced this pull request Sep 20, 2017

build: add NetBSD support to opensslconf.h
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 referenced this pull request Sep 20, 2017

Merged

v6.11.4 proposal #15506

MylesBorins added a commit that referenced this pull request Sep 26, 2017

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