fixes/refactoring for building against LibreSSL#7586
fixes/refactoring for building against LibreSSL#7586jonasschnelli wants to merge 3 commits intobitcoin:masterfrom
Conversation
|
Added a commit that fixes a logical text-issue when compiling with LibreSSL. |
|
In main.cpp we explicitly cede for LibreSSL, do we want a similar construction here? (or even: factor it out somehow so it exists only in one place, so that we don't have this inconsistency next time) |
|
Yes. Let me factor it out. But not sure about extra treating LibreSSL, IMO we should just use I could imaging allow compiling against different OpenSSL alternatives (Apple also has it's own IIRC). |
|
IIRC support for (or we have the |
|
It's not LibreSSL's mistake. They forked at a specific version of OpenSSL that they guarantee API compatibility with. That version uses SSLEAY_VERSION. So LibreSSL has to keep that in order to keep the API compatibility promise. The change in OpenSSL is after the fork. |
|
Added a commit that refactors the SSL detection. |
That's true but they do report a newer OpenSSL version in the |
|
@jonasschnelli just a note, CentOS is not a LibreSSL distribution. I built and run LibreSSL on it, as the OpenSSL it has is old and has poor ECC support. |
Aside, as it is no reason to have worse support for LibreSSL, but as of 0.12, ECC support in OpenSSL is no longer a requirement for Bitcoin Core. |
|
@laanwj yeah, but while I can have both openssl and libressl installed at the same time I can't have the header files to both installed at the same time, at least not with both installed in /usr/include |
| <widget class="QLabel" name="label_14"> | ||
| <property name="text"> | ||
| <string>Using OpenSSL version</string> | ||
| <string>Using SSL version</string> |
There was a problem hiding this comment.
Maybe add library to make clear it's not the protocol?
|
Concept ACK |
| #elif defined(SSLEAY_VERSION_NUMBER) || defined(OPENSSL_VERSION_NUMBER) | ||
| return SSLeay_version(SSLEAY_VERSION); | ||
| #else | ||
| return NULL; |
There was a problem hiding this comment.
what about returning _("unknown") and further simplify the code?
There was a problem hiding this comment.
Soon, there could be an option to run without SSL (after rand and aes implementation).
Then we might want to skip/hide the SSL log info?
|
Debug log: before: after: I propose to change the text to "Using SSL library" |
|
|
||
| std::string CopyrightHolders(const std::string& strPrefix); | ||
|
|
||
| /* returns the current SSL product and version info */ |
There was a problem hiding this comment.
Looks like this file is completely documented, can you please extend the comments? E.g. like this:
/**
* Return the product name with the version number of the SSL library being used.
* @note Bitcoin Core supports OpenSSL and LibreSSL.
*/There was a problem hiding this comment.
Thanks. Will use you comment.
|
Since its no longer part of consensus does it make sense to log it? Bitcoind doesn't log the boost version or the QT version. |
Github-Pull: bitcoin#7586 Rebased-From: 16605c6
|
Agree with @gmaxwell. This way we could get rid of the hacky logic and also tidy up the GUI a bit, keeping in mind there are other ways to detect the library version, if this is ever desired. |
|
I think it could be useful in the debug log. But I agree it can be removed in the UI. |
|
Closing (#7605 merged) |
Alternative solution for #7585. Re-Enables LibreSSL compatibility.
Commit count and credit goes to @AliceWonderMiscreations