-
Notifications
You must be signed in to change notification settings - Fork 651
Workaround for three known issues in OpenSSL for Android. #814
Conversation
SYSTEM=android | ||
ARCH=${ANDROID_SSL_ARCH} | ||
${configure_command}) | ||
# Using the documented method is currently broken on Clang |
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.
I don't understand how this links related to the fact that now we are using ./Configure
instead of ./config
...
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 is because ./config propogates this bug.
If you change this to ./config you will experience the current failure in Travis with -no-canonical-prefixes or any other -no-* CFLAG
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.
I just changed ./Configure back to ./config without changing anything else I get:
***** Unsupported options: no-canonical-prefixes
Using ./Configure, I see -no-canonical-prefixes correctly placed in the arguments to gcc/clang.
There is no other way this script can be written. I am not sure what you are wanting here.
This one kept in case if somebody still maintaining Hunter fork with taka-no-me toolchain support and want to receive latest updates. Anyway it's not related to the current patch, please submit this separately.
You're using the same code with the same links to documenation as in #807. #807 was closed, why do you expect me to merge this one?
Any link to documentation with this statement? This is main requirement for me. All your arguments have "works for me" type, it's not appropriate. |
|
Okay, looks good. Please update |
Done |
Although it actually doesn't affect any result, should be using the ANDROID_SSL_ARCH instead of CMAKE_ANDROID_ARCH to determine 32/64 since I work it out anyway. |
The OpenSSL Configure script runs a s/^-no-/no- script that ruins our CFLAGS input when input from ./config.
On this line: https://github.com/openssl/openssl/blob/OpenSSL_1_1_0-stable/Configure#L560
Tracked here: Unsupported option: -no-canonical-prefixes on Android openssl/openssl#3493
The android-* targets use -mandroid that doesn't work on clang.
To be fixed in future versions with this pull: Build for Android with Clang openssl/openssl#2229
./config can only set up x86 and armv7 targets. It ignores armv8, mips32, mips64 and x86_64.
Tracked here: [rt.openssl.org #4536] Android arm-v8/arm64/aarch64 Support openssl/openssl#2490
As a solution, use our CFLAGS on top of a generic linux target and use the main Configure script.
As far as I know, Configure should be used when you are bringing your own CFLAGS and config should be used to automatically set up sane flags.
For Android, the latter is not working very well.
With this change, Android now compiles for all versions of OpenSSL on GCC and Clang.