-
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
Cleanup old ssl code #28085
Cleanup old ssl code #28085
Conversation
Is this true when using system copies of OpenSSL? I don't remember if there was ever a decision about this. |
It was discussed, I can dig the conversation up if I have to (perhaps in the TSC PR on the openssl strategy? or the tls1.3 PR? or the tls1.3 "help wanted" issue?), but if you try you will find that node |
And by "does not compile" I mean "did not compile even before this PR" (just to be clear). |
@mscdex FYI, errors in node master for build of node_crypto.cc against openssl 1.1.0: https://gist.github.com/sam-github/75f33f84b39e822f6e06e87c16e83ab3 |
@mscdex Did I address your concerns? Can you review, please? |
Versions of OpenSSL lower than 1.1.1 are no longer supported, so remove ifdefs for previous versions.
Workaround added in d9b9229 is no longer needed, since OpenSSL versions lower than 1.1.1 are unsupported.
ee9a65e
to
cd8cf1a
Compare
Versions of OpenSSL lower than 1.1.1 are no longer supported, so remove ifdefs for previous versions. PR-URL: #28085 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Workaround added in d9b9229 is no longer needed, since OpenSSL versions lower than 1.1.1 are unsupported. PR-URL: #28085 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax Confirmed: should not land on 10.x and 8.x, they support external openssl |
Versions of OpenSSL lower than 1.1.1 are no longer supported, so remove ifdefs for previous versions. PR-URL: #28085 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Workaround added in d9b9229 is no longer needed, since OpenSSL versions lower than 1.1.1 are unsupported. PR-URL: #28085 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
and
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes