-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 OpenSSL binding C preprocesor hack failing on absence of headers #5973
Conversation
Oops, forgot to mention this resolves #4676. |
Yeah this is still going to break things. Because of openssl and libressl's breaking API changes, assuming the oldest possible version on failure just won't work. It'll fail to link. Actually, why are we doing this at all. We should just revert the original PR back to the original well-tested version detection that worked for openssl. We then shell out to the preprocessor only for detecting libressl, and if we can't shell out or the headers don't exist or something similar we just assume openssl instead of breaking everything |
And in parallel we should bug libressl upstream to do something actually sane, and explain our (and rust's) issues here. |
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.
It fixes the issue for me. Probably not the definitive solution, but still an improvement. Thank you! 👍
But that's what the original version detection did in the first place, check for 1.0.2 and 1.1.0 if it can (using pkg-config) and assume it's an older version otherwise. If it fails to link with this solution, it would have failed with that too.
I agree but I wouldn't hold my breath on it :) |
But we no-longer check |
I think the current solution is fine and every hickup we had with it was due to trivial implementation errors but I am not against the idea of trying |
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.
Let's merge this now but revert if we hear any more issues with this stuff.
(@sdogruyol noticed this) After this commit, on Arch Linux: $ make clean std_spec
Using /usr/bin/llvm-config [version=6.0.0]
rm -rf .build
rm -rf ./docs
rm -rf src/llvm/ext/llvm_ext.o
rm -rf src/ext/sigfault.o src/ext/libcrystal.a
g++ -c -o src/llvm/ext/llvm_ext.o src/llvm/ext/llvm_ext.cc -I/usr/include -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -O3 -DNDEBUG -fno-exceptions -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
cc -fPIC -c -o src/ext/sigfault.o src/ext/sigfault.c
ar -rcs src/ext/libcrystal.a src/ext/sigfault.o
./bin/crystal build -o .build/std_spec spec/std_spec.cr
_main.o: In function `__crystal_main':
/home/blaxpirit/repos/crystal/src/openssl/lib_ssl.cr:218: undefined reference to `SSL_library_init'
/home/blaxpirit/repos/crystal/src/openssl/lib_ssl.cr:218: undefined reference to `SSL_load_error_strings'
/home/blaxpirit/repos/crystal/src/openssl/lib_ssl.cr:218: undefined reference to `OPENSSL_add_all_algorithms_noconf'
/home/blaxpirit/repos/crystal/src/openssl/lib_ssl.cr:218: undefined reference to `ERR_load_crypto_strings'
_main.o: In function `~procProc(Pointer(Void), Nil)@src/openssl/ssl/hostname_validation.cr:66':
/home/blaxpirit/repos/crystal/src/raise.cr:202: undefined reference to `sk_free'
_main.o: In function `~proc2Proc(Pointer(Void), Nil)@src/openssl/ssl/hostname_validation.cr:66':
/home/blaxpirit/repos/crystal/src/raise.cr:202: undefined reference to `sk_free'
_main.o: In function `~proc3Proc(Pointer(Void), Nil)@src/openssl/ssl/hostname_validation.cr:66':
/home/blaxpirit/repos/crystal/src/raise.cr:202: undefined reference to `sk_free'
_main.o: In function `~proc4Proc(Pointer(Void), Nil)@src/openssl/ssl/hostname_validation.cr:66':
/home/blaxpirit/repos/crystal/src/raise.cr:202: undefined reference to `sk_free'
_main.o: In function `~proc5Proc(Pointer(Void), Nil)@src/openssl/ssl/hostname_validation.cr:66':
/home/blaxpirit/repos/crystal/src/raise.cr:202: undefined reference to `sk_free'
_main.o:/home/blaxpirit/repos/crystal/src/raise.cr:202: more undefined references to `sk_free' follow
O-penS-S-L-5858D-igest.o: In function `new_evp_mt_ctx':
/home/blaxpirit/repos/crystal/src/openssl/digest/digest.cr:23: undefined reference to `EVP_MD_CTX_create'
O-penS-S-L-5858D-igest.o: In function `finalize':
/home/blaxpirit/repos/crystal/src/openssl/digest/digest.cr:38: undefined reference to `EVP_MD_CTX_destroy'
O-penS-S-L-5858D-igest.o: In function `clone':
/home/blaxpirit/repos/crystal/src/openssl/digest/digest.cr:42: undefined reference to `EVP_MD_CTX_create'
/home/blaxpirit/repos/crystal/src/openssl/digest/digest.cr:44: undefined reference to `EVP_MD_CTX_destroy'
O-penS-S-L-5858S-S-L-5858C-ontext.o: In function `default_method':
/home/blaxpirit/repos/crystal/src/openssl/ssl/context.cr:4: undefined reference to `SSLv23_method'
O-penS-S-L-5858S-S-L-5858H-ostnameV-alidation.o: In function `matches_subject_alternative_name':
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:34: undefined reference to `sk_num'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:35: undefined reference to `sk_value'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
/home/blaxpirit/repos/crystal/src/openssl/ssl/hostname_validation.cr:66: undefined reference to `sk_pop_free'
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc "${@}" -o '/home/blaxpirit/repos/crystal/.build/std_spec' -rdynamic -lyaml -lreadline `command -v pkg-config > /dev/null && pkg-config --libs --silence-errors libssl || printf %s '-lssl -lcrypto'` `command -v pkg-config > /dev/null && pkg-config --libs --silence-errors libcrypto || printf %s ' -lcrypto'` /home/blaxpirit/repos/crystal/src/llvm/ext/llvm_ext.o `/usr/bin/llvm-config --libs --system-libs --ldflags 2> /dev/null` -lstdc++ -lgmp -lz -lxml2 -lpcre -lm -lgc -lpthread /home/blaxpirit/repos/crystal/src/ext/libcrystal.a -levent -lrt -ldl -L/usr/lib -L/usr/local/lib`
make: *** [Makefile:112: .build/std_spec] Error 1
$ pacman -Q | grep ^openssl
openssl 1.1.0.h-1
openssl-1.0 1.0.2.o-1 Before this commit the same command passes. |
That's a link to this very pull request :p |
Gotta love recursion 😄 |
My fault again, I found an error in the second commit where I redirect |
…rystal-lang#5973) * Fixed openssl/lib_crypto to not fail when openssl headers are not present on the system * Fixed openssl/lib_crypto to not output errors if the C preprocessor hack fails
The C preprocessor hack to determine OpenSSL & LibreSSL version numbers set them to 0 on failure now. Also I redirected stderr to stdoutput and redirected that to
/dev/null
to silence errors on the second commit but I'm not sure if a Unix compatible shell is guaranteed, we can omit that if you like.