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

deps: remove unused openssl files #5619

Merged
merged 1 commit into from
Mar 21, 2016

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis added the crypto Issues and PRs related to the crypto subsystem. label Mar 9, 2016
@indutny
Copy link
Member

indutny commented Mar 9, 2016

Do we want to document this?

LGTM

@mscdex mscdex added the openssl Issues and PRs related to the OpenSSL dependency. label Mar 9, 2016
@bnoordhuis
Copy link
Member Author

They're detritus from before we had a real process for upgrading openssl, if I understand your question right.

@rvagg
Copy link
Member

rvagg commented Mar 9, 2016

they don't come from openssl source?

@indutny
Copy link
Member

indutny commented Mar 9, 2016

Ah, ok then.

@rvagg
Copy link
Member

rvagg commented Mar 9, 2016

OK, I've moved dist-indexer to try opensslv.h. It's not a historically reliable file since it's pretty new but I'm using Makefile as a fallback.

nodejs/build@4030c77

So this change lgtm

@shigeki
Copy link
Contributor

shigeki commented Mar 9, 2016

These files are also included in openssl-1.0.2g.tar.gz but not in git repo. They are generated by Configure and Makefile and written in .gitignore and they are not necessary for building node.
Do we use git repo for getting openssl source?
I think that it is better for us to obtain openssl sources from https://www.openssl.org/source/ instead of from git repo because pgp sign and hash are provided.

@jbergstroem
Copy link
Member

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Mar 11, 2016

LGTM

Refs: nodejs#5615
PR-URL: nodejs#5619
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bnoordhuis bnoordhuis closed this Mar 21, 2016
@bnoordhuis bnoordhuis deleted the remove-unused-openssl-files branch March 21, 2016 12:26
@bnoordhuis bnoordhuis merged commit a76cb4d into nodejs:master Mar 21, 2016
@jasnell
Copy link
Member

jasnell commented Mar 21, 2016

@bnoordhuis ... if I'm not mistaken this should be done in v4 also, correct?

Fishrock123 pushed a commit that referenced this pull request Mar 22, 2016
Refs: #5615
PR-URL: #5619
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bnoordhuis
Copy link
Member Author

@jasnell Sorry, yes; forgot to add the label. I think the PR should apply cleanly.

MylesBorins pushed a commit that referenced this pull request Mar 30, 2016
Refs: #5615
PR-URL: #5619
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2016
Refs: #5615
PR-URL: #5619
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants