-
Notifications
You must be signed in to change notification settings - Fork 7.3k
deps: copy all openssl header files to include dir #25582
Conversation
On upgrading openssl, all symlinks in pulic header files are replaced with nested include files. The issue was raised that installing them leads to lost its references to real header files. To avoid this, all public header files are copied into the `deps/openssl/openssl/include/openssl/` directory. As a result, we have duplicated header files under `deps/openssl/openssl/` but copied files are refereed in build as specified to include path in openssl.gyp. Fixes: nodejs#9269
@shigeki Why not removing the openssl header files that are the target of the symlinks (or targets of the #include directives in node's codebase) and keep only |
@misterdjules The current master of openssl has already remove them in openssl/openssl@dee502b. You can see that it also needs to change |
@shigeki I see what you mean, thank you for the clarification 👍 My concern is more that by keeping both set of files in our repository, if the OpenSSL project makes changes to the files that are symlink targets, we won't pickup these changes unless we explicitly watch for that. Is there a chance that openssl/openssl@dee502b make it to the 1.0.1 branch at some point? |
@misterdjules I wouldn't bet on it. In order to avoid missing changes of header files in the future, I revised the upgrading docs of https://github.com/nodejs/io.js/blob/master/deps/openssl/doc/UPGRADING.md#3-replace-openssl-header-files-in-depsopensslopensslincludeopenssl. #!/bin/bash
for h in *.h
do
a=`readlink $h`
echo $a
mv $h ${h}.org
cp $a .
done |
@shigeki Would it be better to add in That way we do not float another patch on OpenSSL, and we do not run the risk of forgetting to run this step manually during future upgrades of OpenSSL. That's just a suggestion, you know the OpenSSL code base and the way we integrate it in this repository much better than I do. |
@misterdjules Replacing all symlink of header files with real header files is necessary with/without executing |
@shigeki Right, I'm not questioning removing symlinks, I understand we need that on Windows. What I'm suggesting is that we could replace a manual, error prone, process to copy header files with an automated one in If that's not an option, then we'll need to mention the additional step of copying header files in the one bundled in official releases in the OpenSSL upgrade guide. |
This has landed in v0.10 with 8277822. |
On upgrading openssl, all symlinks in pulic header files are replaced with nested include files. The issue was raised that installing them leads to lost its references to real header files.
To avoid this, all public header files are copied into the
deps/openssl/openssl/include/openssl/
directory.As a result, we have duplicated header files under
deps/openssl/openssl/
but copied files are refereed in build as specified to include path in openssl.gyp.Fixes: #9269
I made a test to install node in my Linux server and confirmed that all real headers of openssl exist in
/usr/local/include/node/openssl/