-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Conversation
Simplify the BSD list by defining OPENSSL_BSD if using a matching BSD platform. Add NetBSD to the list and update documentation.
Shouldn't this be submitted upstream instead? |
The comments inside this file imply it's maintained by node? Or is that wrong? |
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. |
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. |
There was a problem hiding this 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.
my node commit: a6fe49b9bdedef513f67009f4de665492d2d3012 upstream PR: nodejs/node#14313
@bnoordhuis I guess this could land? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rubber stamp lgtm
Landed in 0b0c2ec |
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>
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>
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>
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>
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>
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>
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>
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.
make -j4 test
(UNIX)