-
Notifications
You must be signed in to change notification settings - Fork 30k
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
fix install source path for openssl headers #1354
Conversation
This part is broken for a very long time. We noticed the problem while using jxcore native interface with embedded openssl. I've also sent a pull request to node.js repo. The problem may affect a native addon using builtin openssl.
/cc @indutny @shigeki @bnoordhuis thanks @obastemur |
This has a problem because diff --git a/tools/install.py b/tools/install.py
index 094cadb..2d00cf0 100755
--- a/tools/install.py
+++ b/tools/install.py
@@ -173,8 +173,8 @@ def files(action):
subdir_files('deps/uv/include', 'include/node/', action)
if 'false' == variables.get('node_shared_openssl'):
- action(['deps/openssl/config/opensslconf.h'], 'include/node/openssl/')
subdir_files('deps/openssl/openssl/include/openssl', 'include/node/openssl/', action)
+ action(['deps/openssl/config/opensslconf.h'], 'include/node/openssl/')
if 'false' == variables.get('node_shared_v8'):
subdir_files('deps/v8/include', 'include/node/', action) @obastemur Could you fix your patch? |
@shigeki thanks. it's even better now. |
Thanks. Just landed but I found a mistake in the commit message. |
@shigeki though I'm not 100% sure and rvagg probably knows better, what I've heard people do is they amend the commit with the fix and force push. Unsure if that's right though. |
This part is broken for a very long time. We noticed the problem while using jxcore native interface with embedded openssl. I've also sent a pull request to node.js repo. The problem may affect a native addon using builtin openssl. `opensslconf.h` is overwritten with `deps/openssl/conf/opensslconf.h` PR-URL: #1354 Reviewed-By: Shigeki Ohtsu <[email protected]>
@brendanashworth Thanks. I've just amend the previous commit and pushed a new one in ec7fbf2. |
Probably worth talking this through on #1355, the general rule I've adopted is that a force-push to fix a mistake you've just made is OK as long as you're still at the HEAD, if something else has got in on top then that's just bad luck and you should move on. I'm the wrong person to make up the rule here though, my git-fu is relatively week compared to others in here. |
This part is broken for a very long time. We noticed the problem while using jxcore native interface with embedded openssl. I've also sent a pull request to node.js repo. The problem may affect a native addon using builtin openssl.